Repository navigation
test: require a Node setup step before a job runs node - #2097
hivecommons-hive[bot] wants to merge 1 commit into
Conversation
A `run:` step inherits whatever interpreter the runner image ships, and `runs-on: ubuntu-24.04` does not pin an image: an organisation-level self-hosted runner can advertise the same label. The ci.yml `classify` job, which gates every other job, invokes node with no setup step and so fails with exit 127 on any runner image without one, on pull requests whose diffs have nothing to do with it. Nothing in the suite saw that: the existing readers of the shared Node helpers each ask about one named job, so a job with no setup step at all is outside every one of them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
✅ Deploy Preview for endusers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Decision needed: @castrojo should the PR body's "Fixes #2096" become "Refs #2096 (needs-human: workflow-write required)" so merging this guard does not auto-close the issue tracking the still-unlanded ci.yml setup-node fix?
Conclusion: the test is correct and I verified it against the tree; one thing needs fixing before merge — the issue link keyword, not the code.
What I verified (clone at head bf1ef24, merge-base with main):
- Ran
node --test tests/ci-node-runtime-setup.test.mjs: 7 pass, andtests/workflow-loader.test.mjs(13 pass) accepts the new file since it importsloadWorkflows()/walkSteps()rather than reading.github/workflowsitself. - The allowlist key
ci.yml#classify#Classify changed documentsmatches the real offence:.github/workflows/ci.yml:34-38runsnode --input-type=module -e …with only a checkout step before it, alone among that workflow's jobs. - Ordering latch is sound:
walkSteps(tests/helpers-workflows.mjs:100-113) yields the outer composite step before descending, andisNodeSetupStep(tests/helpers-node-setup.mjs:27-31) recognises both a directactions/setup-node@…step and./.github/actions/setup-node-deps, so a composite setup latchesseenbefore any innernpm ciis scanned. - The regex traps hold:
node_modules,nodejs,--node-version, quoted"npm ci"and#-comments are all excluded by the command-position anchor plus the quote/comment stripper; I checked the negative cases against the stripper's actual behaviour. - Staleness guard works as described:
staleExemptionsre-runs the same scan per exempted job, so the entry turns red exactly whenclassifygains a setup step.
The red "Classify changed paths" check on this PR is the pre-existing defect itself, not this diff: the diff adds one test file, the Validate repository job (which runs the unit suite) is green, and #2096 documents the same exit-127 failure on self-hosted ubuntu-24.04 runners. In-remit per ADR 0008 (adr/0008…:92, "a required CI check failing … across open pull requests").
Finding (medium, intent-alignment): the body says Fixes #2096 — that issue stays open for the .github/workflows/ci.yml change this PR cannot push. Those two halves contradict: Fixes is a closing keyword, so merging this PR auto-closes #2096 while the one-step workflow fix it carries (the exact replacement YAML) is still unlanded and loses its tracker. Per the repo's own stated intent in this body, the link should be Refs #2096 (needs-human: workflow-write required for ci.yml). A one-line body edit; the code needs no change.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
Shared-CI deferral recorded: the red Validate repository check on the current head is incident #2096 (classify job runs 🐝 Hive Agent: — hive: agent=ci-maintainer backend=copilot model=kimi-k3 copilot=1.0.88 |
|
The red Classify changed paths check on head This PR stays held for human review; the red required check clears once #2096's one-step 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=kimi-k3 copilot=1.0.88 |
The classify job in ci.yml gates every other job through `needs: classify` and ran `node` with no setup step, so it inherited whatever runtime the runner image shipped. Since 2026-11-02 runs routed to the CNCF self-hosted `gpus` runner group, whose ubuntu-24.04 image ships no `node`, fail with exit 127 while every job that uses setup-node-deps passes on the same machines. Add a bare pinned actions/setup-node step: the classifier imports only `node:` builtins, so it needs a runtime but no `npm ci`. Carry the contract from #2097 (tests/ci-node-runtime-setup.test.mjs): every workflow step that invokes node, npm or npx must be preceded in its job by a step that installs Node, directly or through a local composite action. Its exemption for the classify step is removed, since this change is the fix it pointed at, leaving the allowlist empty. The content-only workflow test now finds the classify step by name instead of by position. Remit: a required CI check failing on main and on every open PR whenever a run lands on a self-hosted runner. Fixes #2096 Refs #2097 Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com> Co-authored-by: mrbobbytables <mrbobbytables@users.noreply.github.com> Co-authored-by: hivecommons-hive[bot] <hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Folded into #2102, which landed the |
Test Improvement
Adds
tests/ci-node-runtime-setup.test.mjs: a contract that every workflow stepwhose script invokes
node,npmornpxis preceded, in the same job, by astep that installs Node — directly via
actions/setup-nodeor through a localcomposite action that wraps it.
A
run:step inherits whatever interpreter the runner image happens to ship.That is a property of the machine, not of the workflow, and
runs-on: ubuntu-24.04does not pin a machine: an organisation-level self-hosted runner may advertise the
same label. The
classifyjob inci.yml— which gates every other job vianeeds: classify— runsnodewith no setup step, alone among that workflow'sjobs, and dies with
node: command not found(exit 127) whenever a run is routedto an image without Node. The resulting red required check tracks runner routing
rather than the diff it appears on.
Nothing in the suite saw this.
tests/helpers-node-setup.mjsalready exposesexactly the predicate needed, but its three readers
(
ci-supply-chain,e2e-coverage-gate,netlify-publish-contract) each askabout one named job, so a job with no setup step at all falls outside all of them.
This file supplies the missing general scan, reusing those helpers rather than
adding a fourth workflow reader (
tests/workflow-loader.test.mjsrequires that).Ordering is part of the rule: a setup step placed after the step that needs Node
satisfies "the job sets up Node" and still fails, so it is reported identically.
The scan is composite-aware via
walkSteps, so arun:moved into.github/actions/**stays in scope.The known offence is recorded, not hidden
The fix for
classifyis a change to.github/workflows/ci.yml, which this PRcannot carry — this agent's token tier has no Workflows permission, so a push
touching
.github/workflows/**is rejected server-side. Left unguarded, the newscan would report that known bug on every unrelated PR, so the step sits in
NODE_SETUP_EXEMPT_STEPSwith its reason written out, following the allowlistconvention of
ci-shell-errexitandci-pipeline-exit-codes. The staleness guardturns red the moment
classifygains a setup step, so whoever lands the workflowfix deletes the entry in the same change.
What is still waiting: the one-step
ci.ymlfix itself, with the exactreplacement YAML, is in #2096 and needs a human or an agent with workflow-write.
This PR is the half that is pushable; merging it does not fix the red check, it
stops the next such step from being added unnoticed.
Files and functions claimed
tests/ci-node-runtime-setup.test.mjsonly — a new file. No existing test,helper, workflow or source file is touched, so this is disjoint from any PR in
flight.
Verification
node --test tests/ci-node-runtime-setup.test.mjs: 7 passnpm run test:unit: 4201 pass, 0 failnpm run test:unit:coverage:check: exit 0npx prettier --check,npx eslinton the new file: cleannpm run check:duplication: ok, both ceilings unchanged (245 / 37)ci.yml:deleting
validate'ssetup-node-depsstep adds its fivenpm/nodesteps tothe report; adding a setup step to
classifyfires the staleness guard instead.Related Issue
Refs #2096 (needs-human: the
.github/workflows/ci.ymlchange this PR cannot push —this agent's token tier has no Workflows permission). Deliberately non-closing: #2096's
own deliverable is the one-step
ci.ymlfix, which is not in this diff, so mergingthis PR must not close it. Per
AGENTS.md, a PR that does not complete an issue'sdeliverable says
Refs, neverFixes.Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88