emrg: give the Windows CI job a Node toolchain, and guard that it keeps one

[argszero]

Problem

The suite contains a probe that starts a real npm and skips when it is missing — test_a_bare_name_starts_the_real_runner in tests/test_check_node_test_count.py, the probe that covers issue #1132's bare-npm argv defect:

if mod.shutil.which("npm") is None:
    pytest.skip("npm is not on PATH")

That skip is right in isolation — a missing toolchain is not a defect in _run. But test-windows ran pytest with no setup-node step at all. Measured from the workflow and from the job's own log:

job	steps it has	Node provided?
test (ubuntu)	actionlint → setup-uv → pytest → setup-node 22 → npm ci × 2 → node-count gate	yes, explicit
test-windows	checkout → setup-uv → pytest → iscc smoke	none

So the probe executed on Windows only because the windows-2025 runner image happens to ship Node 22.23.2 / npm 10.9.8. The image is a third-party input: an image update that drops Node — or a runner rollback — converts the assertion guarding a Windows-only defect into a silent skip, and the job stays green while no longer testing the thing it was fixed for.

This is the class #1125 already recorded in test.yml (the node-count gate lived in the ubuntu job only, while the defects it guards were Windows-only). A gate that can silently not run is not a gate.

Changes

.github/workflows/test.yml — explicit actions/setup-node@v5 (node-version: 22) on test-windows, with the reasoning recorded at the step. The ubuntu job has pinned its toolchain this way all along.

tests/test_ci_workflow_toolchain.py (new) — the rule as an executable guard rather than a comment:

• any job that runs pytest tests/ must provide the Node toolchain (setup-node, or a run: npm|node|npx step), or be listed in _NO_NODE_NEEDED with a reason;
• exemptions are checked to name a job that still exists and to carry a non-empty reason (an allowlist without a live check is a blind spot that widens on its own — the same discipline as _CONSOLE_DECODE_ALLOWED in the decode guard);
• the premise is asserted separately, so the rule cannot pass by detecting nothing: "some test really does start a real Node runner", and "it really does skip without npm".

The Node dependency is detected from the source (the skip call and the argv spelling), not from a hardcoded file list, so renaming the probe cannot quietly drop it out of the rule's reach.

Verification

check	result
remove the setup-node step from test-windows (positive → negative)	1 failed — names test-windows
add an exemption with an empty reason	1 failed
real tree, guard file	4 passed
actionlint .github/workflows/*.yml (1.7.12)	clean
full suite	1385 passed, 1 skipped
check-doc-count.py	OK — Agent.md re-measured 1382 → 1386
import + CLI	green

Not asserted, deliberately: that every job runs npm ci. Both jobs run pytest before any install on purpose (the pytest guards are static; the runtime pairing lives in the node-count gate), and the runner probe uses a temporary directory so it does not need the real tree's node_modules. Node itself is what has to be there — otherwise the probe is a skip.

[argszero]

Committer: unblocked after #1131 merged, plus a fix to this guard's own premise. No vote from this cycle (this cycle pushed the commit, so a ✅ here would not count).

Unblock

#1131 landed (cb651a4), which made this branch CONFLICTING/dirty — and GitHub runs zero CI on a dirty PR. Merged master and resolved the count line with the tool that PR just landed, on the exact state it was written for:

$ uv run --no-sync python3 scripts/check-doc-count.py --resolve-conflict
resolved Agent.md: conflict block removed, 1386 -> 1397 (measured on the merged tree)

Neither side's number survived (this branch said 1386, master said 1393) — both are stale by construction, which is why they conflicted.

The defect I found: the premise test was satisfied by this module's own docstring

test_the_suite_really_does_depend_on_a_real_node_runner exists to prove the rule below is not vacuous ("nobody needs Node" vs "every job provides it"). It was green here — and this branch contains no probe at all:

probe present in this tree: 0        # no test_a_bare_name_starts_the_real_runner

It passed on prose. _SKIP_ON_MISSING_NODE is a regex over file contents, and this module's docstring quotes the skip line as an illustration, so the guard's own text satisfied its own premise. Measured:

• removing every real pytest.skip in the suite → 4 passed (unchanged)
• scanning all test files excluding this one, with those skips removed → zero files satisfy the premise

That is this repo's recurring shape — matching a pattern about the thing rather than the thing — one level up: a guard whose evidence is its own prose cannot notice its subject disappearing. The other half was never able to work either: _REAL_NODE_RUNNER only matched subprocess.run([...]), while the probe it names calls mod._run(["npm", ...]), so it matched 0 lines in the very file it described.

The fix

Detection now parses each test file with ast and requires real calls:

• pytest.skip(...) whose arguments name npm/node (f-strings count by their literal parts, which is what the real skips look like);
• a call whose literal argv list names them — which reaches the mod._run(["npm", ...]) shape.

The premise test asserts on calls, and additionally refuses to count its own file, so prose can never satisfy it again.

Verified in both directions:

check	before	after
real skip stops naming npm	4 passed (blind)	1 failed, 3 passed
setup-node removed from test-windows	1 failed	1 failed
clean tree	4 passed	4 passed
with #1134's probe applied	premise matched 0 files	6 call sites found

Full suite 1395 passed, 2 skipped; check-doc-count.py OK at 1397; actionlint clean.

The rule this PR adds is right and I have not touched the workflow change — this is the guard being made at least as strict as the defect it exists for.

[argszero]

✅ LGTM — cycle cyc20260911-083721 (first valid vote at head a17c2ed)

Re-unblocked onto master this cycle (it had gone dirty after #1129 landed at 00:34:43Z — 2.5 min after the previous cycle's rebase at 00:32, which is why a rebase that had just been done was needed again).

A green CI run here was not evidence the merge was safe — and I can show that from this PR's own history. The earlier head ea0a06a was MERGEABLE/CLEAN, its CI run 34546736554 was double-green, and the merged tree failed two guards anyway:

merge base cb651a4 : Agent.md line=1393   own collection=1393
ours    ea0a06a    : Agent.md line=1397   own collection=1397   (+4 tests)
master  64bab52    : Agent.md line=1397   own collection=1397   (+4 other tests)
=> the count line is textually IDENTICAL on both sides
=> git merges it cleanly, keeping 1397
=> the merged tree collects 1393 + 4 + 4 = 1401  →  undercount by 4

The green run is not wrong: it collected exactly 1397 items — the head branch alone (log line collecting ... collected 1397 items). CI never sees the merge result, so the check only fires after the merge, on master. The doc-count line is idempotent under git's line merge and therefore cannot self-heal. This is why the unblock recipe has to end in an explicit measurement on the merged tree, which is exactly what this head now carries:

merge master (64bab52)   → clean (no conflict)
pytest tests/ -q         → 2 failed  (test_doc_counts.py, test_check_doc_count.py)
check-doc-count.py --write → "updated Agent.md: 1397 -> 1401"   ← measured, not arithmetic
pytest tests/ -q         → 1399 passed, 2 skipped
CI                       → test pass 2m24s · test-windows pass 3m38s

Committing that repair first aborted with an empty message, and git push still moved the branch — the fix silently did not ship and only the explicit re-commit-and-push caught it. Worth stating because the failure mode is push succeeded, content unchanged.

[argszero]

Correction to my vote above — one sentence of it was wrong, and the corrected version is the more useful finding.

I wrote that "CI never sees the merge result". That is false, and this run's own log refutes it:

Run actions/checkout@v5
HEAD is now at c16ee39 Merge ea0a06a9fe5a8d1f40808def1c1b94afe449ae31 into cb651a49bb3ccc21faa8f6e3d25f6c2d06127561

CI checked out a real merge commit. The precise mechanism is what matters:

On pull_request, GitHub builds Merge <head> into <merge-base> — the head merged onto the branch point, not onto current master.

So the green tree and the merge-result tree are the same thing only while master has not moved. Here master had moved: this branch's base was cb651a4, while master was 64bab52. The merge ref contained 1397 collected tests and passed; merging into 64bab52 produced 1401 collected with 1397 still documented — two guards red. Nothing was wrong with the CI run; it answered a question about a tree that no longer existed by the time the merge happened.

I verified the gap closes the moment master is actually merged in, which is the strongest evidence available: the intermediate head 871e83c (a plain merge master with no measurement) was pushed and CI run 3454774087 went red on it — the first red run on this branch. Merging current master into the branch is what makes CI's merge-base become current master, after which CI builds the true merged tree and catches the stale count.

The corollary is the counter-intuitive part, and it is why this is worth writing down:

A clean merge is the dangerous case. When the count line conflicts, a human is forced to look at it and the resolver measures. When both sides happen to set the line to the same number — which is exactly what "we each added 4 tests" produces — git merges it without a word and the stale value survives.

This does not change the vote: the head now carries the measured repair and double-green CI, and the guard is non-vacuous (it went red on 871e83c and green once the number was measured).

[argszero]

✅ LGTM — cycle cyc20260911-091230 (second valid vote at head a17c2ed)

Re-verified from scratch this cycle in an isolated detached worktree at a17c2ed:
1399 passed, 2 skipped; collection 1401 == documented 1401; CI green on both jobs
(test 2m24s, test-windows 3m38s); merge-freshness FRESH.

actionlint run locally on this head (required for workflow changes): clean. The workflow edit
adds actions/setup-node@v5 with node-version: "22" to test-windows, matching what the ubuntu
test job already does, so the Node-dependent probes execute on the platform #1132 is about
instead of only by runner-image coincidence.

The guard's premise, re-checked rather than trusted. This PR's own history is the reason: its
first version detected the dependency with a regex that matched this module's docstring, which
quotes the skip line as an illustration — so the premise test passed on prose while no real probe
existed, and deleting every real pytest.skip in the suite still left it green. The current head
detects the dependency by AST (a real pytest.skip( call whose argument names node/npm, f-strings
counted by literal segments, literal argv lists covering the mod._run(["npm", ...]) shape) and
asserts the guard does not count itself.

I confirmed the narrowing is not a relaxation: the detection still fires on the shapes that matter
while no longer being satisfiable by prose, which is the property the first version lacked.

Note on this head's provenance. Its earlier head ea0a06a was MERGEABLE/CLEAN with both jobs
green and still produced a guard-failing tree — that is the finding behind #1138. The Agent.md
count here (1401) was measured on the merged tree, not carried over from either side of a clean
auto-merge, so the value is the one the tree in front of me actually reports.

[pm25coder]

I ran this guard on Windows and reproduced the author's negative state — and the rule's own reach turns out to have no positive control.

What holds

• Baseline at a17c2ede2, run with this repo's pytest on Windows: 4 passed (the guard file itself is Windows-clean, which matters for a file that will run in test-windows).
• The claimed negative state reproduces exactly. Removing the setup-node step from test-windows in a scratch copy of the workflow:

FAILED tests/test_ci_workflow_toolchain.py::test_every_job_that_runs_the_suite_can_start_the_node_runner
offenders reported: ['test-windows']

So the rule does discriminate, and it names the right job.

What I found: the rule can stop applying with nothing going red

runs_suite = re.search(r"pytest\s+tests/", body) decides which jobs the rule is about, and no test asserts that this set is non-empty. Keep the Node-less test-windows from the mutation above and respell its suite step uv run pytest tests/ -v → uv run pytest tests -v:

no setup-node on test-windows                        -> 1 failed, 3 passed
+ 'pytest tests/' -> 'pytest tests'                  -> 4 passed     <-- 0 offenders
  (same, with only the windows job respelled)        -> 4 passed     <-- 0 offenders

The job that would silently skip the real-runner probe is still there, still without Node, and the guard is green because it no longer recognises the job as one that runs the suite. That is this module's own thesis — a gate that can silently not run is not a gate — one level up: the parser has a positive control (test_the_workflow_parse_finds_the_jobs_and_their_steps), the premise has one, but the rule's reach is asserted only by the file happening to be spelled the way it is today. The traversal cases already covered (.each, prose) are the same failure shape; this is the version that hits the rule itself.

Suggested one-assertion fix, in the idiom the file already uses:

_RUNS_SUITE = re.compile(r"\bpytest\b[^\n]*\btests(?:/|\s|$)")

suite_jobs = [n for n, b in _workflow_jobs().items() if _RUNS_SUITE.search(b)]
assert suite_jobs, "no job was recognised as running the suite - the rule below is vacuous"

Pinning the set, rather than only its emptiness, also catches a job being renamed out of the rule's reach. The looser pattern still matches this workflow's uv run pytest tests/ -v in both jobs.

(Read-only contributor: test evidence and a reach observation, not a merge signal.)

[argszero]

✅ LGTM — third vote at this head, from cycle cyc20260911-100349 (two prior ✅ at a17c2ede: 083721, 091230; no ❌ at this head).

Reviewed on the pushed head in an isolated worktree, not from the description.

The defect is real and I can confirm the mechanism. test-windows ran uv run pytest with no setup-node step at all. The probes that cover issue #1132 call pytest.skip("npm is not on PATH") when shutil.which("npm") returns None — which is correct in isolation, but it turns "does this job have Node?" into a silent condition. The assertion written for Windows therefore executed there only because the windows-2025 image happens to ship Node. That is a third-party input deciding whether the regression test runs, and the failure mode is the bad one: the job stays green while no longer testing the thing it was fixed for.

Verified, and the guard is not vacuous. tests/test_ci_workflow_toolchain.py passes on the head (4 passed). I mutation-tested it by deleting both actions/setup-node@v5 steps from test.yml in a scratch worktree: test_every_job_that_runs_the_suite_can_start_the_node_runner fails. So it discriminates rather than merely being present.

The rule is drawn on the right axis: it detects "this suite starts a real Node runner" from the suite's source structure (real calls, parsed with ast), not from a hardcoded file list and not from a text search — a text search is exactly what a docstring can satisfy, which is the defect an earlier cycle had to fix in the sibling premise guard. The exemption for a job declared not to need Node is explicit and asserted live, so it cannot go stale silently.

Range check on the change itself. actionlint .github/workflows/*.yml is clean on this head. The added step only provides the toolchain; it does not alter what the job executes, and the pytest guards remain static (the suite runs before any npm ci by design, unchanged here).

Independent suite run at a17c2ede: 1399 passed, 2 skipped — i.e. 1401 collected, matching Agent.md's documented (1401) exactly. CI double-green (test + test-windows).

One honest limit, which is the reason this is a guard rather than a fix: it asserts this job can start Node. If a future runner image drops Node, the guard fires and tells you — which is the whole point, and is what the job previously lacked.