From 107eb8c1d9c35b8b43351a7529050fd06aa2c5e9 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 23 Aug 2026 21:21:21 -0400 Subject: [PATCH] feat(#3753): run docs guards on the PR that changes the docs they read (#3787) A PR whose diff is entirely under docs/ runs zero tests, so a guard whose INPUT is shipped prose cannot protect the PR lane of the diffs it exists to check. Its only firing opportunity is after merge, on the shared branch -- which is how next went red on dacae9273 while the PR that caused it (#3746) was green on every check. The docs-lint job in .github/workflows/docs-required.yml -- an ALREADY-REQUIRED context -- now selects and runs the docs guards that read the specific docs files the PR changed. scripts/docs-guard-registry.cjs test file -> the docs paths it reads (63) scripts/select-docs-guards.cjs pure (changedPaths, registry) -> test files scripts/lint-docs-guard-registration.cjs drift guard, wired into lint:ci scripts/ci-test-scope.cjs is NOT touched -- `git diff origin/next --` on it is empty -- so #764's saving stands and its 21 pinning tests are untouched. Selection: exact path; trailing-slash directory prefix (boundary-checked -- docs/adrenaline.md does NOT match docs/adr/, which a naive startsWith gets wrong); and '*' for the 6 entries that walk docs/ generally or read a computed path. Unknown maps to '*' -- guessing narrow is how a guard silently stops running. Measured: a typo fix selects 6 of 63; docs/AGENTS.md selects 12; docs/COMMANDS.md selects 18. Four things this got wrong first, each found by an independent reviewer or by probe, and each having been asserted safe in a comment: 1. The registry started as a RULE in ci-test-scope.cjs's RULES, on the theory that classify()'s !codeChanged normalization made it inert. True for docs-ONLY diffs; false for MIXED docs+code diffs, where codeChanged is true and the normalization never runs: node scripts/ci-test-scope.cjs --files "docs/a.md src/semver.cts" with the RULE: 25 targeted_tests origin/next: 3 targeted_tests Category error: RULES is the scoped lane's input; a docs-guard registry is a lane manifest for a consumer that never calls classify(). Extracted; pinned by value. 2. The second attempt was a dedicated workflow with paths: [docs/**]. Such a workflow never reports on a non-docs PR, so it can never be a required context without hanging every non-docs PR -- and a non-required check does not block a merge, so the guard would have been advisory and #3753 unfixed. docs-required.yml already has no paths: filter, already supplies the required docs-lint context, already computes docs_changed, and already ran one docs guard gated on it. Generalizing that step needs no ruleset edit at all. 3. The registry and the drift lint were built from ONE path-segment heuristic, so both were blind identically -- and blind at the guard that motivated the issue. The reader-call regex required a character BEFORE its keyword, so a callee named exactly read( / load( / parse( / doc( / file( / content( could never match; and only an INLINE path.join(ROOT,'docs','X.md') argument was caught, missing the two-step-via-variable form -- the MAJORITY spelling -- plus template literals and concatenation. Detector 1 fired on 14 of ~450 files, so 35 genuine guards sat unregistered while the lint reported 0 violations, including cursor-reviewer (reads docs/COMMANDS.md, asserts .includes('--cursor')) and inventory-headings-countfree. The "accepted blind spot" this shipped with was the common case, not a fringe. 4. With detection fixed the true population is 115 files: 63 genuine guards, 52 incidental. Running all 63 in a REQUIRED check on a one-line typo fix is the cost #764 exists to avoid -- install.test.cjs is 7840 lines and reads exactly one docs file, docs/AGENTS.md, for its frontmatter. Dropping it reproduces the bug; running it for a typo elsewhere is waste. Hence the map. Then a second review round found six more, all fixed here: - fragment-single-edit-propagation.install.test.cjs was EXEMPTED as "overlay fixture only". False: it reads the real docs/registries/eos.json and asserts on a registry entry name, and reads the real ADR-0001 and asserts its H1. A docs-only PR touching either would have gone green and red next -- #3753 shipping again, from inside the fix for it. Now registered against both paths, and all 52 remaining exemptions were re-audited one by one. - The SUITES-collision guard compared RAW registry keys, but run-tests.cjs strips a leading `tests/` BEFORE its suite check. So it caught 'all' and missed 'tests/all' -- the only spelling that can actually occur, since every key carries the prefix. One typo would have run all 824 test files inside the required job. Now normalized the same way run-tests.cjs normalizes. - The lint failed OPEN on an unreadable tests dir or candidate file: 0 violations, ok:true. A guard that cannot read its input must never report success. - The exemption ratchet gated identity only, so a baselined file that later STARTED asserting on shipped docs stayed exempt silently -- 52 permanently blind files. The baseline now fingerprints the docs paths each exempted file references and fails when that set changes, naming what changed. - The exemption marker was still honored inside a multi-line template literal in the header window. The scanner now tracks template-literal and block-comment state. - `git diff --name-only | grep '^docs/'` silently dropped C-quoted non-ASCII docs paths, making docs_changed=false a green zero-guard check. Both call sites now pass -c core.quotepath=false. - The run step was gated on hashFiles(), which a force-committed .docs-guard-tests.txt would satisfy. The step now rm -f's both scratch files first and gates on an output it sets itself. Three empty states, deliberately distinct, because conflating them rebuilds #3753: an empty or malformed registry HARD-FAILS; docs changed with no guard covering them logs and skips; no docs change is already gated. The middle state must never be expressed as an empty --files-from, which prints `no tests in suite "all"` and exits 0 -- a green check that guarded nothing. With the current registry that state is unreachable, because the six '*' entries always match; the branch is kept as defensive handling for a future registry and says so. timeout-minutes: 15 bounds the required job against a hanging fork-supplied test; it had none. npm ci was added because the job never installed dependencies -- the previous single-file step got away without it, the registry does not. docs/contributing/docs-guard-registration.md documents the rule, following its sibling cross-platform-portability-rules.md, and CONTRIBUTING.md's CI Test Quality Checks table links to it. It is also load-bearing: without a docs/ file in the diff this PR would not have triggered its own lane, shipping an unexercised change to a required check. One unrelated fix, included because this PR surfaced it and CLAUDE.md forbids deferring a defect found while working. On this branch's first CI run, `full test (windows-latest, 24, shard 3/3)` was CANCELLED at exactly 30 minutes; tests were still passing 0.8s before the cancel, so it is a wall-clock timeout, not a hang, and a cancelled job reddens `Required tests`. The cause is not this PR's test file, which costs ~60ms. Shard composition is unstable: adding ONE file to the unit suite reshuffled 115 of 268 files between shards, and shard 3 drew a heavier mix. Underneath that is a real pre-existing defect. tests/ci-test-job-timeout-budget.test.cjs requires every lane's budget to be >= 1.5x its MEASURED cost -- "a lane that got slower must be re-budgeted, not excused" -- and its test-full entry recorded 19m from a windows-22 shard. That is stale. Measured on `next` with none of this PR's changes present: 26m18s (run 32614439702, windows-latest/24 shard 3/3), 23m36s and 23m17s on shard 2/3. So the lane costs ~26m and the 30-minute cap carried 1.14x headroom, not 1.5x. The gate had been out of compliance with its own rule; this PR was merely the file addition that reshuffled shard 3 past the cliff. Fixed as that file prescribes: measuredMinutes 19 -> 27 with fresh evidence, and test-full timeout-minutes 30 -> 45. The rule's minimum for 27m is 41; 45 is deliberately above it because the reshuffle means per-shard worst case moves run to run, and a budget pinned to the exact minimum would be re-breached by the next test file anyone adds. Only that one job's timeout changed; test.yml's scope, matrix and steps are untouched, so #764's saving is unaffected. Raising that cap let the Windows shard finish (28m45s, inside 45) and uncovered a real failure the 30-minute cancel had been masking: `new quick-task branch branches off origin/main (#2916)` died with `outcome=timed_out exitCode=null`, SIGTERM, at the 15000ms bound. tests/quick-branching.test.cjs:149 `runStep` runs a `#!/usr/bin/env bash` script executing MULTIPLE git commands, but was bound to GIT_TIMEOUT_MS (15000) -- the norm for a SINGLE git plumbing call. tests/helpers/timeouts.cjs already documents this exact failure and exists to fix it: HOOK_FANOUT_TIMEOUT_MS was created after PR #3285 recorded "outcome=timed_out exitCode=null at exactly the 15000ms probe bound while every other lane passed the same commit", and calls that "a bound sized for the wrong class, not a slow machine". Our failure is that case verbatim, so both sites move to the class norm rather than to a bigger number. The same class also failed on `next` itself 21 hours earlier -- run 32608945654, windows-latest/24 shard 1/3, `plan touching only src/ in a submodule project keeps worktree isolation ENABLED` -- where tests/worktree-safety.test.cjs:5845 `runGate` fans out to `git config --file .gitmodules` under a hardcoded 30000. Fixed too, since it is a defect in the tree regardless of which branch surfaced it. A survey of the whole tests/ tree found the same class-mismatch at further bash fan-out sites bound under 60000ms, and the maintainer approved sweeping them rather than leaving them latent to surface the same way one at a time. 16 fan-out sites across 16 files now use the class norm. The sweep is class-correctness, not raising numbers until things pass. Sites were moved ONLY where the bash body demonstrably spawns something (git, node, npm, a CLI); self-contained shell snippets were left where they are, and are listed as deliberately unchanged: pure if/printf bodies (copilot-install), pure array/case builtins (code-review-pipeline-regression:638), a documented pure-shell gsd_run stub (host-integration), single-process hook calls (workflow-guard:222/271/302), and a deliberately tight 5000ms fast-check hook (gsd-write-guard.property). Nothing was lowered. process-seam.test.cjs:513 (literal 300) is untouched on purpose -- it tests timeout BEHAVIOR, so raising it would destroy what it asserts. Shared file-level constants were the trap here, and were handled per file rather than by redefinition: GIT_TIMEOUT_MS has ~15 users in git-base-branch and only 1 is a fan-out; WORKTREE_TIMEOUT_MS has 16 users in worktree.test.cjs and 3 are; PROBE_TIMEOUT_MS has several in three more files. In each the CALL SITE was changed and the constant left alone, so no single-plumbing-call site silently inherited a 60s bound. The one exception is hooks-opt-in.test.cjs, where HOOK_TIMEOUT_MS has exactly one consumer -- spawnHook, the fan-out itself -- so redefining it is identical in effect and reads better. Only two of these sites have actually been observed failing. The rest cite that shared class and those two run ids rather than inventing evidence of their own. Co-authored-by: sim --- .github/workflows/docs-required.yml | 133 ++- .github/workflows/test.yml | 14 +- .gitignore | 6 +- CONTEXT.md | 4 +- CONTRIBUTING.md | 1 + bin/install.js | 2 +- docs/INVENTORY-MANIFEST.json | 2 +- docs/INVENTORY.md | 2 +- docs/contributing/docs-guard-registration.md | 78 ++ eslint.config.mjs | 4 +- package.json | 2 +- scripts/docs-guard-registry.cjs | 339 ++++++++ ...-reasons.cjs => gsd-test-gate-reasons.cjs} | 6 + scripts/lint-docs-guard-registration.cjs | 495 +++++++++++ ...ocs-guard-registration.exempt-baseline.cjs | 174 ++++ ....cjs => lint-fix-has-regression-tests.cjs} | 18 +- scripts/lint-health-diagnostic-rule-table.cjs | 2 +- scripts/lint-removed-but-needed.cjs | 4 +- scripts/lint-source-test-name-collision.cjs | 241 ++++++ scripts/select-docs-guards.cjs | 56 ++ src/install-engine.cts | 2 +- ...est-home-guard.cts => real-home-guard.cts} | 10 + src/surface.cts | 4 +- tests/adr-parser.test.cjs | 1 + tests/adr-parser.unit.test.cjs | 1 + .../agent-marker-documentation-guard.test.cjs | 1 + tests/antigravity-upgrades.test.cjs | 1 + tests/capability-cli.test.cjs | 1 + tests/check-ui-safety-gate.test.cjs | 16 +- tests/ci-docs-guard-registry.test.cjs | 801 ++++++++++++++++++ tests/ci-test-job-timeout-budget.test.cjs | 13 +- tests/ci-test-scope.test.cjs | 1 + tests/cline-install.test.cjs | 1 + ...close-phase-todos-padded-resolves.test.cjs | 11 +- tests/code-review-depth.test.cjs | 1 + .../code-review-pipeline-regression.test.cjs | 12 +- tests/codebuddy-upgrades.test.cjs | 1 + tests/commands.test.cjs | 3 + tests/commit-docs-bypass.test.cjs | 1 + tests/complexity-trigger.test.cjs | 1 + tests/cursor-imperative-reference.test.cjs | 1 + ...declarative-reference-antigravity.test.cjs | 1 + tests/declarative-reference-zcode.test.cjs | 1 + tests/emitted-attribution.test.cjs | 1 + tests/eslint-rules.test.cjs | 4 + tests/estimate-calibrate.test.cjs | 1 + tests/execute-phase-worktree-guard.test.cjs | 12 +- tests/executed-plan.test.cjs | 2 +- tests/gen-context-index.test.cjs | 1 + tests/gen-registry.test.cjs | 1 + tests/git-base-branch.test.cjs | 10 +- tests/graphify-auto-update.slow.test.cjs | 16 +- tests/graphify-visualization.test.cjs | 13 +- tests/gsd-agent-isolation-guard.test.cjs | 1 + tests/helpers.cjs | 2 +- tests/hermes-dispatch-upgrade.test.cjs | 1 + tests/hooks-opt-in.test.cjs | 11 +- tests/install-minimal-hooks.test.cjs | 1 + tests/install-runtime-artifacts.test.cjs | 5 +- tests/install-write-confinement.test.cjs | 2 +- tests/install.test.cjs | 15 +- ...ller-migration-config-root-marker.test.cjs | 1 + ...taller-migration-pi-extension-ext.test.cjs | 1 + tests/installer-migrations.test.cjs | 1 + tests/kimi-upgrades.test.cjs | 1 + tests/lint-allow-test-rule-refs.test.cjs | 4 + tests/lint-docs-command-form.test.cjs | 4 + tests/lint-docs-required.test.cjs | 3 + .../lint-source-test-name-collision.test.cjs | 212 +++++ tests/manifest-version-sync.test.cjs | 1 + tests/milestone-archive.test.cjs | 1 + tests/model-resolver.test.cjs | 1 + tests/new-project-mvp-prompt.test.cjs | 1 + tests/onboard-command.test.cjs | 1 + tests/opencode-command-dir-plural.test.cjs | 1 + tests/phase.test.cjs | 1 + tests/pr-branch-planning-filter.test.cjs | 1 + tests/precommit-alias-drift-hook.test.cjs | 12 +- tests/quick-branching.test.cjs | 10 +- tests/removed-but-needed-lint.test.cjs | 1 + tests/repo-invariants.test.cjs | 1 + tests/require-issue-link-policy.test.cjs | 4 + tests/reviewer-manifest-body.test.cjs | 1 + tests/run-tests-harness.test.cjs | 1 + .../runtime-artifact-layout-surface.test.cjs | 4 +- tests/runtime-name-policy.test.cjs | 1 + ...ecurity-prompt-injection.security.test.cjs | 1 + tests/shipped-reference-cites.test.cjs | 1 + tests/state.test.cjs | 1 + tests/test-failure-reasons.test.cjs | 2 +- tests/unreachable-shell-guard.test.cjs | 17 +- tests/workflow-guard.test.cjs | 11 +- tests/worktree-cleanup.test.cjs | 19 +- tests/worktree-safety.test.cjs | 17 +- tests/worktree.test.cjs | 22 +- 95 files changed, 2837 insertions(+), 92 deletions(-) create mode 100644 docs/contributing/docs-guard-registration.md create mode 100644 scripts/docs-guard-registry.cjs rename scripts/{test-failure-reasons.cjs => gsd-test-gate-reasons.cjs} (76%) create mode 100644 scripts/lint-docs-guard-registration.cjs create mode 100644 scripts/lint-docs-guard-registration.exempt-baseline.cjs rename scripts/{lint-fix-has-regression-test.cjs => lint-fix-has-regression-tests.cjs} (79%) create mode 100644 scripts/lint-source-test-name-collision.cjs create mode 100644 scripts/select-docs-guards.cjs rename src/{test-home-guard.cts => real-home-guard.cts} (97%) create mode 100644 tests/ci-docs-guard-registry.test.cjs create mode 100644 tests/lint-source-test-name-collision.test.cjs diff --git a/.github/workflows/docs-required.yml b/.github/workflows/docs-required.yml index 7b05c50cb..47762bf2f 100644 --- a/.github/workflows/docs-required.yml +++ b/.github/workflows/docs-required.yml @@ -15,6 +15,14 @@ permissions: jobs: docs-lint: runs-on: ubuntu-latest + # This job supplies the ALREADY-REQUIRED `docs-lint` context + # (.github/rulesets/main-protection.json) and runs `npm ci` plus every + # registered docs-guard test — all fork-PR-modifiable. Without a bound, a + # fork PR that hangs a registered test pins this required check for + # GitHub's 6h default, repeatably. The lane itself runs in ~210s; 15 + # minutes matches test.yml's most common per-job timeout (5 of its 8 jobs + # use 15) while giving ~4x headroom. + timeout-minutes: 15 steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 with: @@ -35,6 +43,15 @@ jobs: - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 with: node-version: '24' + cache: 'npm' + - name: Install dependencies + # Needed for the docs-guard registry below: several registered tests + # (e.g. tests/capability-registry.test.cjs) require devDependencies + # such as fast-check that are not present without an install. The + # prior single-file `docs-parity-live-registry` step got away without + # this because that one test has no external deps; the full registry + # does not have that luxury. + run: npm ci - name: Run docs-required lint env: GITHUB_BASE_REF: ${{ github.base_ref }} @@ -45,12 +62,122 @@ jobs: env: BASE_REF: ${{ github.event.pull_request.base.ref }} run: | - if git diff --name-only "origin/${BASE_REF}...HEAD" | grep -q '^docs/'; then + if git -c core.quotepath=false diff --name-only "origin/${BASE_REF}...HEAD" | grep -q '^docs/'; then echo "docs_changed=true" >> "$GITHUB_OUTPUT" else echo "docs_changed=false" >> "$GITHUB_OUTPUT" fi - - name: Docs parity — live registry check + # This generalizes the former single-file `docs-parity-live-registry` + # check to the docs-guard registry (#3753), and then (#3753 follow-up) + # SELECTS only the guards that read the docs/ paths actually changed in + # this PR, instead of always running the entire registry — a one-line + # typo fix in an unrelated doc should not pay for tests/install.test.cjs + # (7840 lines) just because it reads docs/AGENTS.md. It lives in THIS + # job rather than a dedicated `paths:`-filtered workflow specifically + # because this job has no `paths:` filter and therefore ALWAYS reports + # a status, which is what lets it supply the already-required + # `docs-lint` context (.github/rulesets/main-protection.json). A + # `paths:`-filtered workflow never reports on a non-docs PR, so it can + # never be made a required context without hanging every non-docs PR + # forever — that was the fatal flaw in the dedicated + # `.github/workflows/docs-guards.yml` this replaces. + - name: Select docs guards for the changed docs/ paths + id: select-docs-guards if: steps.docs-changed.outputs.docs_changed == 'true' - run: node --test tests/docs-parity-live-registry.test.cjs + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + # Security follow-up FIX 5: destroy any FORCE-COMMITTED copy of + # these files before this step regenerates them. Both are + # gitignored, but gitignore does not stop `git add -f`; a fork PR + # that force-commits a malicious `.docs-guard-tests.txt` must not + # have that content survive into the run below. + rm -f .docs-guard-tests.txt .docs-changed-paths.txt + + # Reuse the base ref already fetched above — no re-fetch. + git -c core.quotepath=false diff --name-only "origin/${BASE_REF}...HEAD" | grep '^docs/' > .docs-changed-paths.txt || true + node -e " + const fs = require('fs'); + const { DOCS_GUARD_TESTS, assertNoSuiteCollision, DOCS_GUARD_TEST_FILES } = require('./scripts/docs-guard-registry.cjs'); + const { selectDocsGuards } = require('./scripts/select-docs-guards.cjs'); + + // State (a) — a bad edit to the registry must never yield a green + // check that guarded nothing. This is distinct from state (b) + // below: an EMPTY/MALFORMED registry is always a hard failure, + // regardless of what changed. + if (typeof DOCS_GUARD_TESTS !== 'object' || DOCS_GUARD_TESTS === null || DOCS_GUARD_TEST_FILES.length === 0) { + console.error('::error::docs-guard-registry.cjs exported an empty or missing DOCS_GUARD_TESTS map — refusing to run a docs-guard job with zero tests'); + process.exit(1); + } + // #3753 security follow-up FIX 3: a registry entry equal to a + // run-tests.cjs SUITES token (e.g. a typo'd 'all') would otherwise + // be silently treated as a suite selector by selectExplicitFiles + // (scripts/run-tests.cjs:651), running the ENTIRE suite inside this + // required job instead of erroring. Redundant with + // lint-docs-guard-registration.cjs's own check and + // docs-guard-registry.cjs's module-load-time self-check — enforced + // here too so this derivation step never depends on another + // consumer having already run. + assertNoSuiteCollision(DOCS_GUARD_TEST_FILES); + + const changedDocsPaths = fs.readFileSync('.docs-changed-paths.txt', 'utf8') + .split('\n') + .map(s => s.trim()) + .filter(Boolean); + + const selected = selectDocsGuards(changedDocsPaths, DOCS_GUARD_TESTS); + + console.log('changed docs/ paths:'); + console.log(changedDocsPaths.map(p => ' ' + p).join('\n') || ' (none)'); + console.log('selected docs-guard tests (' + selected.length + ' of ' + DOCS_GUARD_TEST_FILES.length + '):'); + console.log(selected.map(f => ' ' + f).join('\n') || ' (none)'); + + if (selected.length === 0) { + // State (b) — docs/ changed, but no registered guard reads any + // of the changed paths. This is LEGITIMATE (e.g. a typo fix in a + // how-to guide no guard covers): exit 0 WITHOUT writing the + // selected-tests file, so the next step (gated on that file's + // presence) never invokes run-tests.cjs at all. Passing an empty + // list to \`run-tests.cjs --files-from\` would instead print + // 'no tests in suite \"all\"' and exit 0 — indistinguishable from + // a real pass, which is #3753's own failure mode one level down. + // + // UNREACHABLE with the current registry: six entries in + // DOCS_GUARD_TESTS carry the \`'*'\` sentinel (run on ANY docs/ + // change), so selectDocsGuards() never returns an empty array for + // a non-empty changed-docs-paths list — this branch guards a + // FUTURE registry shape (one with no wildcard entries), not a + // live case today. Corollary: every docs PR today runs at least + // those six tree-walking guards, regardless of which docs/ file + // changed. + console.log('no registered docs-guard reads any changed docs/ path — nothing to run.'); + process.exit(0); + } + + fs.writeFileSync('.docs-guard-tests.txt', selected.join('\n') + '\n'); + // Security follow-up FIX 5: gate the next step on an EXPLICIT step + // OUTPUT this step itself sets, not on hashFiles('.docs-guard-tests.txt') + // != ''. hashFiles only checks whether the file exists in the + // working tree with non-empty content — it cannot tell a file THIS + // STEP legitimately wrote from one a fork PR force-committed + // earlier in the same checkout (gitignored files are still + // addable with \`git add -f\`). An explicit output set only on this + // code path cannot be forged by a committed file. + fs.appendFileSync(process.env.GITHUB_OUTPUT, 'selected=true\n'); + " + + - name: Run selected docs-guard tests + # Only runs when the selection step itself set selected=true — state + # (b) above (docs changed, nothing selected) intentionally skips this + # step rather than invoking run-tests.cjs with an empty/absent list. + # Gated on the step's own OUTPUT (set only on the code path that just + # wrote .docs-guard-tests.txt), not on hashFiles(...) != '' — a + # force-committed copy of that file cannot forge this output. + if: steps.docs-changed.outputs.docs_changed == 'true' && steps.select-docs-guards.outputs.selected == 'true' + run: | + if [ ! -s .docs-guard-tests.txt ]; then + echo "::error::selected docs-guard test list is empty — refusing to run a green check that ran zero tests" >&2 + exit 1 + fi + node scripts/run-tests.cjs --files-from .docs-guard-tests.txt diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dbbf3ac33..b21708949 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -417,9 +417,17 @@ jobs: # a 20-minute cap) on 05b170e44 and 18m14s (91%) on 81eeb8a53. The Windows # shards are slow for platform reasons — process spawn and filesystem cost, # not extra work — and this lane has already blown its cap twice before - # (#1051, #1212). 30 is ~1.5x the worst observed shard, restoring the - # headroom that #1212's sharding bought and this suite has since eaten. - timeout-minutes: 30 + # (#1051, #1212). + # #3787: fresh measurement on windows-latest/24 shard 3/3 hit 26m18s (run + # 32614439702), so the 18m59s figure above is stale and the 30-minute cap + # only had ~1.14x headroom, in violation of this repo's own 1.5x rule + # (tests/ci-test-job-timeout-budget.test.cjs). 1.5x of 27m requires 41m + # minimum; 45 is used instead of the bare minimum because shard + # composition is unstable — adding one test file reshuffled 115 of 268 + # unit files between shards — so the per-shard worst case moves run to + # run and a budget pinned to the exact minimum would be re-breached by + # the next file anyone adds. + timeout-minutes: 45 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled # #2665: strict on Linux/macOS, report-only on Windows (see the `test` job note). diff --git a/.gitignore b/.gitignore index ae6f5579c..0e3755067 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,10 @@ hooks/.dist-staging-*/ # Coverage artifacts coverage/ +# Derived docs-guard test list (CI-generated by docs-required.yml, #3753) +.docs-guard-tests.txt +.docs-changed-paths.txt + # Animation assets animation/ *.gif @@ -79,7 +83,7 @@ build/ /gsd-core/bin/lib/install-effort-resolver.cjs /gsd-core/bin/lib/install-model-override-resolver.cjs /gsd-core/bin/lib/install-engine.cjs -/gsd-core/bin/lib/test-home-guard.cjs +/gsd-core/bin/lib/real-home-guard.cjs /gsd-core/bin/lib/embedding-adapter.cjs /gsd-core/bin/lib/adapter-declarative.cjs /gsd-core/bin/lib/adapter-imperative.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 96442be8d..34f7aeb38 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -286,8 +286,8 @@ Module owning install-time staging and content-rewrite selection for a pre-resol ### Install Fs Adapter Module Narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (`src/install-engine.cts`) — lands ADR-58's never-shipped `cleanup` rollout step (`registry → adapter → helpers → cleanup`, #2874, epic #2866 Phase 5). `installRuntimeArtifacts` now returns the executed plan it ran (`{ runtime, scope, kinds: [{kind, sourceDir, destDir, preserved}], cleanup: [{dir, ok}], postSteps }`) instead of `void`, including on the `combinedFamilyInstall` (OpenCode/Kilo) early-return path — no path may return `undefined` after this phase. Failure is unchanged: stage/rewrite errors still throw rather than becoming an `ok:false` value, so a caller cannot read success-shaped data off a failure path. Delivery is an ambient single mutable adapter (`current`), swapped for the duration of one synchronous install via `withInstallFs(deps.fs, fn)` and always restored in a `finally` — a `deps` parameter threaded through every function on the call tree (`install-profiles.cts`, `runtime-artifact-conversion.cts`, `commonjs-marker.cts`, `installer-migrations.cts`'s two reachable entry points) was rejected as a dozen+-site touch for no behavioral gain over the ambient swap, extending rather than replacing `createRuntimeArtifactInstallPlan`'s existing `deps` bag precedent (Runtime Artifact Install Plan Module). An injected `deps.fs` is a PARTIAL adapter merged over real `node:fs`; any method it omits — except `realpathSync`, the one method documented to degrade gracefully — now THROWS immediately if actually called, naming the missing method, rather than silently resolving to the real filesystem (#2875 defect fix: the prior silent fall-through let a fake adapter missing e.g. `rmSync` perform real destructive IO unnoticed). **Routes destination IO only, by design**: every write/probe against the install destination (copies, removals, snapshot/restore of preserved skill dirs, the manifest read) is fake-able; locating this package's own source tree (`findInstallSourceRoot`/`findAgentsSourceRoot`'s walk-up-from-`__dirname`, `readGsdCommandNames`) stays real and unrouted — a destination-fake's store starts empty and was never seeded with the repo's own paths, so routing that lookup would make every fake-adapter install throw instead of staging. The symlink-escape guard (`hasExistingSymlinkBetween`) and `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this seam — only their `existsSync`/`lstatSync`/`realpathSync` probes route through it, so an injected fake can change what a probe observes but never flip the security decision itself. Writes remain byte-identical to pre-#2874 (AC4/AC5); existing `void`-ignoring callers (`bin/install.js`) are unaffected. Source: `gsd-core/bin/lib/install-fs-adapter.cjs` (generated from `src/install-fs-adapter.cts`). See Runtime Artifact Install Plan Module, ADR-58. -### Test Home Guard Module -Refuses an in-process install whose destination would land in the developer's REAL home while a Node test runner is in scope (#3712, ADR-1239/#2088 territory). Interface: `assertTestHomeSandboxed(operation, runtime, kinds, deps?)` (throws), `isTestHomeGuardRefusal(err)`, `SANDBOX_MARKER`. Exists because a runtime kind may declare a global `home` override resolved from `os.homedir()` rather than from the caller's `configDir` — codex's skills kind (`home: ".agents"`) is the only live case — so sandboxing `configDir`/`targetDir` does NOT contain it, and `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) structurally cannot see the class: that gate confines a `destSubpath` to whatever root it is handed, and here the root IS the escaped home. The observed failure was silent destruction — a test file that sandboxed only `targetDir` pruned all 71 `gsd-*` skills from a real `~/.agents/skills` via `_removeGsdEntries` while the suite exited 0 and the manifest still reported a healthy install. **Six writers** resolve a kind `home` and then write or destroy under it and therefore carry a call: `installRuntimeArtifacts`, `uninstallRuntimeArtifacts` (Install Engine Module), `applySurface` (Surface Module), and `migrateLegacyDevPreferencesToSkill` — which CREATES rather than prunes, which is why it was missed on the first pass, and which runs from `_runLegacyInstallMigrations` BEFORE `installRuntimeArtifacts`' own assertion, so it guards the destination it already resolved rather than resolving a second time (generative-fix divergence) — plus `installOpencodeFamilySkills` and `installAgentsKindStandalone`, guarded against a future descriptor change rather than a present escape since no combined-family or agents kind declares a `home` today. Each of the four reachable writers carries an optional `deps: { os?, env? }` tail parameter — the guard's trigger condition is "HOME equals the passwd home", which cannot be simulated without pointing at the developer's real home — so `tests/install-write-confinement.test.cjs` drives the REAL entrypoint for each one and deleting any guard call site turns a row red. `migrateLegacyDevPreferencesToSkill` guards only when `target.hasHomeOverride` — that same resolution's own answer to "did the skills kind declare a `home`?", never inferred from `installRoot !== targetDir`, which is FALSE whenever the override resolves onto `targetDir` itself (a configDir of `$HOME/.agents`, exactly where codex's override points); both directions of that condition are pinned, so widening it to refuse ordinary confined migrations fails a row too. **Gated on `NODE_TEST_CONTEXT`** (set by `node --test`), never on `GSD_TEST_MODE` — several in-process test files, including the one that caused #3712, never set the latter — so ordinary installs outside a Node test context are untouched and codex still installs to `$HOME/.agents` normally. **The predicate asks about the DESTINATION, not about HOME state**, and about the path the write RESOLVES to rather than how it is spelled: a stale layout captured before `sandboxHome()` still names the real `~/.agents` while HOME is sandboxed, so a HOME-state check would wave it through, and an aliased `/.agents` symlink or junction would otherwise redirect an allowed path into the real home. Canonicalization itself fails CLOSED: a component that cannot be resolved (`EACCES`/`EPERM` on a directory whose mode changed, `ELOOP` on a symlink cycle, `EIO` on a failing mount) is REFUSED rather than falling back to the lexical spelling — that fallback was a second, unnamed fail-open, and precisely the ALLOW an unresolvable alias needs, since the sandbox spelling then satisfies the nested-sandbox exemption (Codex review of #3725). Only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split rather than inventing a second one: both mean "no such path as named", which is the ordinary shape of a destination no install has created yet. A destination inside the real home is exempted only when all three hold — HOME differs from the passwd home by filesystem identity, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users`), and the destination resolves beneath it. That exemption is not a softening: on Windows `os.tmpdir()` is under `%USERPROFILE%`, so EVERY test sandbox is a descendant of the real home and containment alone cannot separate the safe case from the dangerous one — without it the whole Windows matrix refuses. Every containment decision compares by filesystem identity (`st_dev`+`st_ino`), never pathname (the one pathname comparison in the module is `sameDirectory`'s equality shortcut on the marker branch, described below): `path.resolve` resolves neither symlinks nor case and `realpath` returns a canonical pathname two routes to one directory can disagree on. **Fails CLOSED**, with one named exception: where no passwd entry is readable (some CI images) it falls back to a path-valued marker set by `sandboxHome()`/`installSpawnEnv()` in `tests/helpers.cjs`, which must identify, must equal the home in effect, AND must contain every resolved destination. All three are load-bearing: matching HOME attests that a caller sandboxed HOME and says nothing about where these destinations resolve, so on its own the marker waved through the very stale-layout shape the primary branch refuses; and `sameDirectory`'s pathname-equality shortcut can answer yes for two identical UNidentifiable paths, which is not enough to place a destination against. It remains a deliberate weakening, since nothing on such a host can contradict a marker naming the real home. `installSpawnEnv` re-points the marker at an explicitly overridden HOME, so a spawn that supplies its own sandbox is not refused by a marker still naming the helper's default one. Refusals are STAMPED so `bin/install.js` can rethrow them without running `_codexPreConfigRollback()`, which deletes and recreates every snapshotted `gsd-*` directory in the resolved skills root — otherwise the guard's own refusal would provoke the mutation it exists to prevent. **Known limits, stated rather than implied**: a subordinate bind mount of the real `~/.agents` into the sandbox is not closed (a bind mount is not a link, so realpath keeps the mount-point spelling; closing it needs non-portable mount-table introspection), and a cross-process TOCTOU swap between check and write is out of reach. Source: `gsd-core/bin/lib/test-home-guard.cjs` (generated from `src/test-home-guard.cts`). See Install Engine Module, Surface Module, Runtime Artifact Install Plan Module, Runtime Artifact Layout Module. +### Real Home Guard Module +Refuses an in-process install whose destination would land in the developer's REAL home while a Node test runner is in scope (#3712, ADR-1239/#2088 territory). Interface: `assertTestHomeSandboxed(operation, runtime, kinds, deps?)` (throws), `isTestHomeGuardRefusal(err)`, `SANDBOX_MARKER`. Exists because a runtime kind may declare a global `home` override resolved from `os.homedir()` rather than from the caller's `configDir` — codex's skills kind (`home: ".agents"`) is the only live case — so sandboxing `configDir`/`targetDir` does NOT contain it, and `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) structurally cannot see the class: that gate confines a `destSubpath` to whatever root it is handed, and here the root IS the escaped home. The observed failure was silent destruction — a test file that sandboxed only `targetDir` pruned all 71 `gsd-*` skills from a real `~/.agents/skills` via `_removeGsdEntries` while the suite exited 0 and the manifest still reported a healthy install. **Six writers** resolve a kind `home` and then write or destroy under it and therefore carry a call: `installRuntimeArtifacts`, `uninstallRuntimeArtifacts` (Install Engine Module), `applySurface` (Surface Module), and `migrateLegacyDevPreferencesToSkill` — which CREATES rather than prunes, which is why it was missed on the first pass, and which runs from `_runLegacyInstallMigrations` BEFORE `installRuntimeArtifacts`' own assertion, so it guards the destination it already resolved rather than resolving a second time (generative-fix divergence) — plus `installOpencodeFamilySkills` and `installAgentsKindStandalone`, guarded against a future descriptor change rather than a present escape since no combined-family or agents kind declares a `home` today. Each of the four reachable writers carries an optional `deps: { os?, env? }` tail parameter — the guard's trigger condition is "HOME equals the passwd home", which cannot be simulated without pointing at the developer's real home — so `tests/install-write-confinement.test.cjs` drives the REAL entrypoint for each one and deleting any guard call site turns a row red. `migrateLegacyDevPreferencesToSkill` guards only when `target.hasHomeOverride` — that same resolution's own answer to "did the skills kind declare a `home`?", never inferred from `installRoot !== targetDir`, which is FALSE whenever the override resolves onto `targetDir` itself (a configDir of `$HOME/.agents`, exactly where codex's override points); both directions of that condition are pinned, so widening it to refuse ordinary confined migrations fails a row too. **Gated on `NODE_TEST_CONTEXT`** (set by `node --test`), never on `GSD_TEST_MODE` — several in-process test files, including the one that caused #3712, never set the latter — so ordinary installs outside a Node test context are untouched and codex still installs to `$HOME/.agents` normally. **The predicate asks about the DESTINATION, not about HOME state**, and about the path the write RESOLVES to rather than how it is spelled: a stale layout captured before `sandboxHome()` still names the real `~/.agents` while HOME is sandboxed, so a HOME-state check would wave it through, and an aliased `/.agents` symlink or junction would otherwise redirect an allowed path into the real home. Canonicalization itself fails CLOSED: a component that cannot be resolved (`EACCES`/`EPERM` on a directory whose mode changed, `ELOOP` on a symlink cycle, `EIO` on a failing mount) is REFUSED rather than falling back to the lexical spelling — that fallback was a second, unnamed fail-open, and precisely the ALLOW an unresolvable alias needs, since the sandbox spelling then satisfies the nested-sandbox exemption (Codex review of #3725). Only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split rather than inventing a second one: both mean "no such path as named", which is the ordinary shape of a destination no install has created yet. A destination inside the real home is exempted only when all three hold — HOME differs from the passwd home by filesystem identity, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users`), and the destination resolves beneath it. That exemption is not a softening: on Windows `os.tmpdir()` is under `%USERPROFILE%`, so EVERY test sandbox is a descendant of the real home and containment alone cannot separate the safe case from the dangerous one — without it the whole Windows matrix refuses. Every containment decision compares by filesystem identity (`st_dev`+`st_ino`), never pathname (the one pathname comparison in the module is `sameDirectory`'s equality shortcut on the marker branch, described below): `path.resolve` resolves neither symlinks nor case and `realpath` returns a canonical pathname two routes to one directory can disagree on. **Fails CLOSED**, with one named exception: where no passwd entry is readable (some CI images) it falls back to a path-valued marker set by `sandboxHome()`/`installSpawnEnv()` in `tests/helpers.cjs`, which must identify, must equal the home in effect, AND must contain every resolved destination. All three are load-bearing: matching HOME attests that a caller sandboxed HOME and says nothing about where these destinations resolve, so on its own the marker waved through the very stale-layout shape the primary branch refuses; and `sameDirectory`'s pathname-equality shortcut can answer yes for two identical UNidentifiable paths, which is not enough to place a destination against. It remains a deliberate weakening, since nothing on such a host can contradict a marker naming the real home. `installSpawnEnv` re-points the marker at an explicitly overridden HOME, so a spawn that supplies its own sandbox is not refused by a marker still naming the helper's default one. Refusals are STAMPED so `bin/install.js` can rethrow them without running `_codexPreConfigRollback()`, which deletes and recreates every snapshotted `gsd-*` directory in the resolved skills root — otherwise the guard's own refusal would provoke the mutation it exists to prevent. **Known limits, stated rather than implied**: a subordinate bind mount of the real `~/.agents` into the sandbox is not closed (a bind mount is not a link, so realpath keeps the mount-point spelling; closing it needs non-portable mount-table introspection), and a cross-process TOCTOU swap between check and write is out of reach. Source: `gsd-core/bin/lib/real-home-guard.cjs` (generated from `src/real-home-guard.cts`). See Install Engine Module, Surface Module, Runtime Artifact Install Plan Module, Runtime Artifact Layout Module. ### User Artifact Staging Module Durable, on-disk staging for `USER_OWNED_ARTIFACTS` (Install Engine Module's `preserveUserArtifacts`/`restoreUserArtifacts` callers) across the preserve → wipe → restore window, closing #1874-F19: an in-memory-only `Map` held across a wipe is silently discarded on process death (Ctrl-C, OOM, a converter throw mid-copy), losing the user's file permanently (#2875, epic #2866 Phase 6, governed by ADR-3574). Interface: `stageUserArtifacts(destDir, fileNames, stagingRoot) -> StagedUserArtifacts`, `restoreStagedUserArtifacts(destDir, staged)`, `discardStagedUserArtifacts(staged)`, `recoverOrphanedUserArtifacts(stagingRoot, configDir) -> RecoveryResult` — four operations rather than two because call sites genuinely differ (one defers to a migration helper instead of restoring; another restores only on migration FAILURE). Synchronous only, every fs call routed through `installFs()` (Install Fs Adapter Module), which now GUARDS a partial injected adapter: any method the partial omits — except the one documented degrade-to-real-fs exception, `realpathSync` — throws immediately if actually called, instead of silently falling through to real fs (#2875 defect fix). Staging layout is fixed by convention — `/.gsd-staging/user-artifacts//{record.json,files/}` — a sibling of every wipe target this phase's four call sites wipe, so staging survives all of them while resolving inside `configDir`; `record.json` is written AFTER every file copy lands, never before, so a half-written staging directory (crash mid-copy) has no record and is never mistaken for a complete one. All staged/restored/recovered names are FLAT (no path separator of either platform's flavor) — matching every real caller's actual usage and rejected the same way traversal/NUL-byte names already were. **Durability alone is not the fix**: a staged copy nothing ever reads back is bytes-safe but user-visibly lost — the #1879-F15 inert-fix failure mode — so `recoverOrphanedUserArtifacts` is wired at the START of `bin/install.js`'s `install()` and `uninstall()`, before the ordinary preserve step, for every runtime; this is the only production entry point that makes recovery reachable rather than merely callable. Its "never throws" contract is enforced with a per-file try/catch (one bad name is reported via `skipped` and the batch continues) wrapped in a per-entry try/catch (one bad batch is reported and the next staging entry is still attempted) — an earlier version left `mkdirSync`/the symlink-safe copy/the final cleanup `rmSync` unguarded, so a single unrecoverable entry (e.g. a directory unexpectedly staged where a file was expected, or `symlinkSync` throwing `EPERM` for an unprivileged Windows user) threw out of the function entirely — before that entry was ever cleaned up — permanently bricking every future install/uninstall (#2875 defect fix). **Carries NO policy** (same discipline as the Install Fs Adapter Module): every path this module writes, and every path recovery reads OUT of an on-disk record before writing to it (attacker-influenceable the moment an install runs on a shared machine), is re-resolved through the SAME `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) every other write on this call tree uses, THEN through the SAME `hasExistingSymlinkBetween` (Install Engine Module) `_copyStaged`/`migrateLegacyDevPreferencesToSkill` apply to their own writes — never reimplemented, and required lazily (call-time, not module-load-time) specifically to avoid a real circular require with Install Engine Module, which imports this module statically. Lexical confinement (`assertDestWithinConfigHome`) alone cannot see a symlinked ANCESTOR directory between `configDir` and a recorded `destDir`; the `hasExistingSymlinkBetween` re-check closes that gap (#2875 defect fix). `recoverOrphanedUserArtifacts` takes `configDir` as a REQUIRED, EXPLICIT parameter — it is never derived from `stagingRoot`'s own path shape, which would rest the confinement guarantee on a naming convention rather than an explicit caller-supplied value. Never overwrites something already present at the recovered destination, decided by `lstatSync` rather than `existsSync` — `existsSync` FOLLOWS symlinks and reports `false` for a DANGLING one, so it cannot see a dangling symlink an attacker planted at the destination to redirect the eventual `copyFileSync`/`symlinkSync` outside `configDir`; `restoreStagedUserArtifacts` applies the same `lstatSync`-based refusal before writing (#2875 defect fix — both were previously `existsSync`-based). Symlink-safe: staged files copy via Installer Migration Module's `copyPreservingSymlink` (itself newly routed through `installFs()` this phase, all five of its fs calls), which never dereferences a symlink — a managed path replaced by a link to (e.g.) `~/.ssh/id_rsa` cannot have the referent's bytes copied into the staging tree or back out of it; a consumer that reads a staged copy's CONTENT back (rather than re-copying it) must separately check for a staged symlink before `readFileSync`, or it will follow the link and read the referent (Install Engine Module's `_runLegacyInstallMigrations` applies this guard). Staging failure is a HARD throw (not swallowed) so a caller cannot proceed to wipe the source directory having staged nothing — worse than no staging at all; recovery and restore/discard degrade instead (missing files, missing staging root, malformed records are all "nothing to do", never a crash). `stagingRoot` confinement against `configDir`, and the symlinked-staging-root refusal (`hasExistingSymlinkBetween`), are both call-site responsibilities (Install Engine Module's `_resolveUserArtifactStagingRoot`, mirrored locally in `bin/install.js`) — this module accepts no `configDir` parameter to `stageUserArtifacts` and cannot perform that outer check itself. **Known limitation, documented rather than closed**: concurrent installs targeting the SAME `destDir` are not safe against each other — the staging key is `sha256(destDir)`, and both the stage-time entryDir-clear and the recovery-time end-of-batch cleanup unconditionally `rmSync` an `entryDir` they did not necessarily create, so two processes racing the same `destDir` can have one wipe the other's in-flight or just-committed batch; closing this fully needs either a cross-process lock (its own crash-safety design surface) or a guarantee installs never run concurrently against one `configDir`, neither of which this module can decide unilaterally. Explicitly out of scope: fsync durability (crash-safe against process death only, not power loss); routing the raw-`fs` uninstall wipe at Install Engine Module's `_runLegacyUninstallCleanup` (only the staging call itself routes through `installFs()` there — the surrounding wipe stays unrouted, matching Phase 5's deliberate exclusion of the uninstall tree). Source: `gsd-core/bin/lib/user-artifact-staging.cjs` (generated from `src/user-artifact-staging.cts`). See Install Engine Module, Install Fs Adapter Module, Runtime Artifact Install Plan Module, Installer Migration Module, ADR-3574. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b66d8a175..47dbc37bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1195,6 +1195,7 @@ The following checks run on every PR in addition to the test suite: |-----|----------------|-------------| | `Lint — ESLint` | No source-grep tests (see above), via the `local/no-source-grep` rule | Replace with `runGsdTools()` behavioral tests, or add `// allow-test-rule: ` | | `Lint — cross-platform portability` | Windows-portability defects in tests, via `local/no-path-literal-in-assert` (more rules land per [ADR-1703](docs/adr/1703-portability-enforcement-architecture.md)) — e.g. a path-returning call asserted against a hardcoded `/`-literal | Normalize the actual: `String(pathFn(...)).replace(/\\/g, '/')`, or structure platform-specific code behind a `process.platform !== 'win32'` guard. **No `eslint-disable`** — see [cross-platform-portability-rules.md](docs/contributing/cross-platform-portability-rules.md) | +| `lint-docs-guard-registration.cjs` (via `npm run lint:ci`) | A test that reads shipped `docs/` content must be registered so it runs on the PR that changes those docs — otherwise it can only fail after merge | Register it in `scripts/docs-guard-registry.cjs`, mapping the test to the docs paths it reads, or mark it `// docs-guard-exempt: ` and list it in `scripts/lint-docs-guard-registration.exempt-baseline.cjs` — see [docs-guard-registration.md](docs/contributing/docs-guard-registration.md) | Run locally before pushing: `npm run lint` (or `npx eslint .`) diff --git a/bin/install.js b/bin/install.js index a8d4dc02a..70e5f6394 100755 --- a/bin/install.js +++ b/bin/install.js @@ -41,7 +41,7 @@ const { // consentRequired, hostPrecedenceRank) instead of the id being re-derived // and re-interpreted at each call site. See src/install-scope.cts. const { resolveScope } = require('../gsd-core/bin/lib/install-scope.cjs'); -const { isTestHomeGuardRefusal } = require('../gsd-core/bin/lib/test-home-guard.cjs'); +const { isTestHomeGuardRefusal } = require('../gsd-core/bin/lib/real-home-guard.cjs'); // getDirName (runtime -> local config dir name) is relocated out of this // installer to the runtime-name-policy leaf (ADR-1508 / #1510 Phase 1) so the // conversion module's rewrite engine can consume it without importing diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 3e3413feb..edf26a779 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -469,6 +469,7 @@ "prohibition-enforcement.cjs", "project-root.cjs", "prompt-budget.cjs", + "real-home-guard.cjs", "refactor-trigger-command-router.cjs", "research-provider.cjs", "research-store.cjs", @@ -508,7 +509,6 @@ "task-command-router.cjs", "teams-status.cjs", "template.cjs", - "test-home-guard.cjs", "text-lines.cjs", "token-scanner.cjs", "uat-predicate.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index d78426ae6..95b53172c 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -595,6 +595,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `profile-pipeline.cjs` | User behavioral profiling data pipeline, session file scanning | | `prompt-budget.cjs` | Pure token-budget accounting for review prompts — estimates tokens, applies deterministic trim priority (head-shrink PROJECT.md, proportional plan truncation, drop context/research/requirements, hard-fail guard), returns structured metadata for `review.max_prompt_tokens` (#3081) | | `observability/redaction.cjs` | Arg redaction policy for dispatch events — args omitted from every emitted event by default, opt-in verbatim inclusion via `GSD_AUDIT_ARGS=1`; stateless env read, no module-level caching (#177) | +| `real-home-guard.cjs` | Real-home confinement guard (#3712; renamed from `test-home-guard.cjs` — a source filename matching Node's `test-*` convention is collected and executed as a test by the remote runner) — refuses any of the six writers that resolve a kind `home` — `installRuntimeArtifacts`, `uninstallRuntimeArtifacts`, `applySurface`, `migrateLegacyDevPreferencesToSkill`, plus the descriptor-dependent `installOpencodeFamilySkills` and `installAgentsKindStandalone` when a `node --test` run would resolve a kind's global `home` override (codex skills -> `$HOME/.agents`, ADR-1239/#2088) inside the real passwd home, which silently pruned every `gsd-*` skill there; compares homes by filesystem identity (`st_dev`+`st_ino`) rather than by pathname, since a case-variant, symlinked, or bind-mounted HOME names one directory under two names; fails CLOSED unless both homes identify or one is definitively absent (the pathname-equality shortcut in `sameDirectory` can answer yes without identifying, so the marker branch requires the marker to identify separately before it may allow anything); a destination inside the real home is exempted only when HOME differs from the passwd home, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users` are not sandboxes), and the destination resolves beneath it (Windows puts the temp root inside the home, so containment alone cannot tell a sandbox from the danger); where no passwd entry is readable it falls back to `sandboxHome()`'s path-valued marker, which must identify AND contain every resolved destination — a marker matching HOME attests only that HOME was sandboxed, so on its own it waved through a layout captured before the sandbox that still named the real `~/.agents`; it remains a deliberate weakening rather than a closed door, since nothing there can contradict a marker naming the real home; does not defend against a subordinate bind mount of the real directory into the sandbox (realpath cannot unify bind-mounted spellings) or a cross-process TOCTOU swap; inert for normal installs outside a Node test context | | `refactor-trigger-command-router.cjs` | ADR-959 capability command router for `gsd-tools refactor` (issue #1953) — dispatches evaluate/status/accept/decline subcommands for the complexity-triggered refactor capability; owns capability-activation gating, git invocation (via the `git-base-branch.cjs` `phaseStartCommit`/`changedFilesSince` adapters), config reads, phase-directory resolution, and the optional broken-windows ledger integration around the pure `complexity-trigger.cjs` leaf | | `research-provider.cjs` | Research provider waterfall, confidence tiers, and planResearch (cache-hits + fetch plan) | | `research-store.cjs` | Content-addressed research cache: sha256 keys, per-source TTL staleness, two-tier (user ~/.gsd / project .planning) store | @@ -640,7 +641,6 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `task-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools task` | | `teams-status.cjs` | Detects agent-teams status from environment and runtime; pure core (#1355) | | `template.cjs` | Template selection and filling with variable substitution | -| `test-home-guard.cjs` | Test-home confinement guard (#3712) — refuses any of the six writers that resolve a kind `home` — `installRuntimeArtifacts`, `uninstallRuntimeArtifacts`, `applySurface`, `migrateLegacyDevPreferencesToSkill`, plus the descriptor-dependent `installOpencodeFamilySkills` and `installAgentsKindStandalone` when a `node --test` run would resolve a kind's global `home` override (codex skills -> `$HOME/.agents`, ADR-1239/#2088) inside the real passwd home, which silently pruned every `gsd-*` skill there; compares homes by filesystem identity (`st_dev`+`st_ino`) rather than by pathname, since a case-variant, symlinked, or bind-mounted HOME names one directory under two names; fails CLOSED unless both homes identify or one is definitively absent (the pathname-equality shortcut in `sameDirectory` can answer yes without identifying, so the marker branch requires the marker to identify separately before it may allow anything); a destination inside the real home is exempted only when HOME differs from the passwd home, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users` are not sandboxes), and the destination resolves beneath it (Windows puts the temp root inside the home, so containment alone cannot tell a sandbox from the danger); where no passwd entry is readable it falls back to `sandboxHome()`'s path-valued marker, which must identify AND contain every resolved destination — a marker matching HOME attests only that HOME was sandboxed, so on its own it waved through a layout captured before the sandbox that still named the real `~/.agents`; it remains a deliberate weakening rather than a closed door, since nothing there can contradict a marker naming the real home; does not defend against a subordinate bind mount of the real directory into the sandbox (realpath cannot unify bind-mounted spellings) or a cross-process TOCTOU swap; inert for normal installs outside a Node test context | | `text-lines.cjs` | Line-terminator handling seam — `splitLines`/`normalizeEol`/`detectEol`/`joinLines`, the sole owner of `\r?\n` splitting and CRLF normalization; closes #3360's split-then-match fix in `frontmatter.cjs` (ADR-3212 §3, epic #3212 Phase 2, #3413) | | `token-scanner.cjs` | Tokenizer-first seam for stateful grammars — `tokenizeShellLike` (quote-aware shell tokenizer, the primitive `hooks/lib/git-cmd.js` migrated onto) and `indentWidth` (bullet-nesting depth, closes #3169's cross-reference-vs-declaration false positive in `decisions.cts`) (ADR-3212 §4, epic #3212 Phase 3, #3414) | | `normalize-test-command.cjs` | Normalizes a resolved test command to a one-shot form so a watch-mode runner (vitest/jest) cannot hang a verification gate (#1857); shared by all three live test-command gates (regression, post-merge, audit-fix) | diff --git a/docs/contributing/docs-guard-registration.md b/docs/contributing/docs-guard-registration.md new file mode 100644 index 000000000..dca418115 --- /dev/null +++ b/docs/contributing/docs-guard-registration.md @@ -0,0 +1,78 @@ +# Docs guard registration + +A test that reads shipped `docs/` content and asserts on it must be registered so it actually +runs on the PR that changes those docs — otherwise it can only fail *after* merge, on the shared +branch. This page is the practical reference + how-to; the mechanics live in +[`scripts/docs-guard-registry.cjs`](../../scripts/docs-guard-registry.cjs), +[`scripts/select-docs-guards.cjs`](../../scripts/select-docs-guards.cjs), and +[`scripts/lint-docs-guard-registration.cjs`](../../scripts/lint-docs-guard-registration.cjs). + +## The problem + +`classify()` in [`scripts/ci-test-scope.cjs`](../../scripts/ci-test-scope.cjs) deliberately +normalizes a docs-only diff to an empty test list (from #764) — a PR that only touches `docs/` +is not expected to pay for the full suite. That is correct for the common case, but it means a +test whose *input* is shipped prose (it reads a `docs/*.md` file and asserts on its content) never +runs on the PR that edits that file. The regression is caught only when it lands on `next` and +some *other* PR's CI happens to touch code that reruns the full matrix — or not at all. That is how +`next` broke on `dacae9273`: a docs edit shipped with zero test coverage of its own change. + +## The rule + +A test file that reads shipped `docs/` content must do one of two things: + +- be registered as a key in `DOCS_GUARD_TESTS` in `scripts/docs-guard-registry.cjs`, mapped to the + `docs/` path patterns it reads, or +- carry a `// docs-guard-exempt: ` header comment (in the file's first 20 lines) **and** + have its basename listed in `DOCS_GUARD_EXEMPT_BASELINE` in + `scripts/lint-docs-guard-registration.exempt-baseline.cjs`. + +This is enforced by `scripts/lint-docs-guard-registration.cjs`, run via `npm run lint:ci`. A +docs-reading test file that is neither registered nor exempted fails the lint. + +## How selection works + +The registered guards are not all run on every docs PR. `scripts/select-docs-guards.cjs` maps the +PR's actually-changed `docs/` paths to the subset of `DOCS_GUARD_TESTS` that reads any of them, so +a one-line typo fix in one doc does not pay for every other registered guard. Each registry entry +is an array of patterns, and a pattern is one of three kinds: + +- **Exact path** — `'docs/AGENTS.md'` matches that file only. +- **Trailing-slash directory prefix** — `'docs/adr/'` matches any changed path beneath it + (`docs/adr/README.md`, `docs/adr/1703-....md`, …). The trailing slash is load-bearing: a changed + path is compared with `startsWith('docs/adr/')`, so `docs/adrenaline.md` does **not** match + `docs/adr/` — only a real path *under* that directory does. +- **`'*'`** — matches any docs/ change at all. + +## Which to choose: register or exempt + +Register the test when it asserts on the **content** of shipped prose — the test would need to +change (or would break) if the doc's wording, structure, or specific values changed. For example, +`tests/config-field-docs.test.cjs` reads `docs/CONFIGURATION.md` and asserts every config field +documented there matches the real schema — that is a content assertion, so it is registered. + +Exempt the test when it touches `docs/` only incidentally: overlay/fixture input, a path +predicate, an existence check with no content assertion, or a scan-exclusion list. For example, a +test that calls `fs.existsSync('docs/something.md')` to confirm a file was created, without ever +reading or asserting on its contents, is exempt — nothing about registering it would catch a real +regression, because it never inspects the prose. + +## Use `'*'` when the path cannot be resolved statically + +Some tests walk `docs/` recursively, or build the path they read from a runtime-computed +variable rather than a fixed literal — `tests/context7-tool-name-parity.test.cjs` and +`tests/docs-parity-live-registry.test.cjs` are examples already in the registry, both mapped to +`'*'`. When a test's read cannot be pinned to a specific file or directory prefix, register it with +`'*'` rather than guessing a narrower pattern. A guessed-narrow pattern that misses the actual path +is worse than no registration at all: the guard silently stops running on exactly the PR that +should have triggered it, and nothing in CI signals the gap — that silent-gap failure mode is the +whole defect class this registry exists to prevent. + +## Why the exemption is ratcheted + +`DOCS_GUARD_EXEMPT_BASELINE` pins the known-good set of exemptions by file basename, the same +identity-ratchet pattern the repo already uses for the `// allow-test-rule:` marker (ADR-456). A +brand-new `// docs-guard-exempt:` marker that is not already in the baseline fails the lint, so +adding one is always a visible, reviewable diff — not a silent opt-out a test author can add +without anyone noticing. A baseline entry whose file no longer exists, or no longer carries the +marker, is reported stale and must be pruned in the same PR that removes it. diff --git a/eslint.config.mjs b/eslint.config.mjs index 8d24fb575..721df06a9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -82,8 +82,8 @@ export default tseslint.config( // lint the src/install-model-override-resolver.cts source, not this. 'gsd-core/bin/lib/install-model-override-resolver.cjs', 'gsd-core/bin/lib/install-engine.cjs', - // #3712: tsc-generated runtime artifact — lint src/test-home-guard.cts, not this. - 'gsd-core/bin/lib/test-home-guard.cjs', + // #3712: tsc-generated runtime artifact — lint src/real-home-guard.cts, not this. + 'gsd-core/bin/lib/real-home-guard.cjs', // #2874 (epic #2866 Phase 5): tsc-generated runtime artifact — lint the // src/install-fs-adapter.cts source, not this. 'gsd-core/bin/lib/install-fs-adapter.cjs', diff --git a/package.json b/package.json index 20db55a34..8fb3ba4b6 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,7 @@ "lint:table-schema-drift": "node scripts/lint-table-schema-drift.cjs", "lint:frontmatter-scalar-broad-grep": "node scripts/lint-frontmatter-scalar-broad-grep.cjs", "lint:removed-but-needed": "node scripts/lint-removed-but-needed.cjs", - "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-unreachable-guard-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-state-write-path-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-planning-artifact-writer-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs && node scripts/lint-no-adhoc-regex-escape.cjs && node scripts/lint-vendored-deps.cjs && npm run lint:hooks-runtime-build-seam && node scripts/check-contract-drift.cjs", + "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-tests.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-unreachable-guard-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-state-write-path-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-planning-artifact-writer-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs && node scripts/lint-no-adhoc-regex-escape.cjs && node scripts/lint-vendored-deps.cjs && node scripts/lint-docs-guard-registration.cjs && node scripts/lint-source-test-name-collision.cjs && npm run lint:hooks-runtime-build-seam && node scripts/check-contract-drift.cjs", "lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs", "lint:regression-names": "node scripts/lint-regression-test-names.cjs", "lint:descriptions": "node scripts/lint-descriptions.cjs", diff --git a/scripts/docs-guard-registry.cjs b/scripts/docs-guard-registry.cjs new file mode 100644 index 000000000..22a3c8fcd --- /dev/null +++ b/scripts/docs-guard-registry.cjs @@ -0,0 +1,339 @@ +#!/usr/bin/env node +'use strict'; + +/** + * docs-guard-registry.cjs — the sole source of truth for which doc-reading + * test files the docs-guard lane must run. + * + * ## Why this is its own module, not a `scripts/ci-test-scope.cjs` RULE + * + * A prior version of this PR added a `docs guards` RULE to `RULES` in + * scripts/ci-test-scope.cjs, on the theory that classify()'s `!codeChanged` + * normalization (which zeroes fullMatrix/targeted_tests/windows_tests for + * docs-only diffs) made the RULE inert to classify()'s scope decision. + * + * That is true for docs-ONLY diffs and FALSE for MIXED docs+code diffs: + * `codeChanged` is true whenever ANY changed file is product/pipeline code, + * so the normalization never runs, and every one of this registry's test + * files joined `targeted_tests` for every mixed PR. Probed on that version: + * `node scripts/ci-test-scope.cjs --files "docs/a.md src/semver.cts"` + * returned 25 targeted_tests, vs. 3 on `origin/next` — an unstated blowup + * of the scoped/targeted lane on every mixed docs+code PR. + * + * Root cause: `RULES` answers "given these changed files, what should + * test.yml run" for the MAIN scoped-lane pipeline. The docs-guard registry + * is a LANE MANIFEST for a completely different consumer (the `docs-lint` + * job in .github/workflows/docs-required.yml, gated on a `docs_changed` + * step output). Putting a lane manifest inside a scope-classification rule + * set was the defect; extracting it here removes any path by which it can + * influence classify() at all. + * + * ## Who reads this + * + * - .github/workflows/docs-required.yml (`docs-lint` job) — derives the + * PER-PR-SELECTED subset of this registry from scripts/select-docs-guards.cjs, + * which is itself driven by this module's DOCS_GUARD_TESTS map. This job has + * no `paths:` filter, so it always reports a status and can supply the + * already-required `docs-lint` context; a dedicated `paths:`-filtered + * workflow cannot be made required without hanging non-docs PRs forever. + * - scripts/lint-docs-guard-registration.cjs — derives its registration + * lint's comparison set from this module (so a docs-reading test file + * that is neither registered here nor exempted fails the lint). + * - scripts/select-docs-guards.cjs — the pure selector that maps a PR's + * changed docs/ paths to the subset of this registry that actually needs + * to run (#3753 follow-up: a flat "run everything" list is disproportionate + * for a single-file docs typo fix). + * + * ## Registry shape (#3753 follow-up) + * + * `DOCS_GUARD_TESTS` is a MAP from test file path to the array of docs/ + * paths it actually reads, so a changed-docs-file can be resolved to the + * narrow subset of guards that read it, instead of always running the + * entire registry. Each value is a non-empty array of PATTERNS: + * + * - a plain path (e.g. `'docs/AGENTS.md'`) matches that exact file only; + * - a trailing-slash path (e.g. `'docs/adr/'`) matches any changed path + * under that directory prefix — use this for a test that walks or + * `readdirSync`s a whole docs/ subdirectory; + * - the sentinel `'*'` means "run on ANY docs/ change" — reserved for a + * test that cannot be resolved to a narrower set of paths (the path is + * computed, looped over an unresolvable variable, or the test walks + * docs/ generally). Conservative fallback: when in doubt, use `'*'`, + * never a guessed-narrow path — a false negative here (a guard that + * silently stops running) is exactly the #3753 defect class this + * registry exists to prevent. + * + * `DOCS_GUARD_TEST_FILES` (derived: `Object.keys(DOCS_GUARD_TESTS)`) is kept + * as a flat array export so the registration lint and its parity test + * continue to consume a plain file list without needing to know about the + * map shape. + * + * ## Adding a docs guard + * + * Add the test file's path (relative to the repo root, `tests/`) as a + * key in DOCS_GUARD_TESTS below, with the docs/ paths it reads as the value + * (or `['*']` if that cannot be resolved narrowly). Do not duplicate this + * list anywhere else — a second, independently maintained list is exactly + * the #3753 defect class (10 registered-but-not-run guards, silently + * drifted) this registry exists to prevent from recurring. + * + * Sorted alphabetically by basename for unambiguous diffs. + */ + +/** + * Duplicate of scripts/run-tests.cjs:50's `SUITES` array. That array is not + * exported by run-tests.cjs (module.exports there is deliberately narrow; + * run-tests.cjs's own behavior is intentional and out of scope for this + * registry to alter), so it cannot be imported directly without changing a + * shared, behavior-locked file. + * + * Why this must exist at all: `run-tests.cjs`'s `selectExplicitFiles` + * (scripts/run-tests.cjs:651) treats any registry entry that EQUALS a + * SUITES member (e.g. `all`, `unit`) as a suite selector, not a filename — + * so a typo in DOCS_GUARD_TESTS matching a suite name would silently run + * the ENTIRE suite inside the required `docs-lint` job instead of erroring. + * `assertNoSuiteCollision` below rejects that at the registry boundary + * instead. + * + * Divergence risk: if run-tests.cjs's real SUITES list ever changes and this + * copy is not updated, this check could under- or over-reject. That risk is + * covered by a parity test (tests/ci-docs-guard-registry.test.cjs) that + * drives run-tests.cjs's own exported `selectExplicitFiles` behaviorally — + * for every token here it asserts run-tests.cjs treats it as a suite + * selector (never "file not found"), and for a control non-member it + * asserts the opposite — so a real divergence fails that test rather than + * silently drifting. + */ +const RUN_TESTS_SUITES = ['all', 'unit', 'integration', 'install', 'security', 'slow', 'qa']; + +/** + * Normalize a registry entry EXACTLY the way scripts/run-tests.cjs's + * `splitFileList` (:617-625) normalizes a requested token before its SUITES + * check (:651): strip a leading `tests/` and normalize `\`->`/`. Without this, + * comparing RAW registry keys (which all carry the `tests/` prefix by + * convention) against RUN_TESTS_SUITES misses the realistic typo `'tests/all'` + * entirely — proven by probe: `assertNoSuiteCollision(['tests/all'])` did not + * throw, and `selectExplicitFiles(allFiles, 'tests/all')` selected all 824 + * files (the whole suite) inside the required docs-lint job. + * + * @param {string} t + * @returns {string} + */ +function normalizeForSuiteCheck(t) { + return t.replace(/\\/g, '/').replace(/^tests\//, ''); +} + +/** + * Reject any docs-guard registry entry that collides with a run-tests.cjs + * suite token (see RUN_TESTS_SUITES doc comment above). Throws with every + * offending entry named, rather than failing on only the first. Compares + * entries AFTER normalizeForSuiteCheck, mirroring run-tests.cjs's own + * splitFileList normalization, so a `tests/`-prefixed or backslash-spelled + * entry that would collide post-normalization is caught here too. + * + * @param {string[]} tests + */ +function assertNoSuiteCollision(tests) { + const collisions = tests.filter((t) => RUN_TESTS_SUITES.includes(normalizeForSuiteCheck(t))); + if (collisions.length > 0) { + throw new Error( + 'docs-guard-registry: DOCS_GUARD_TESTS entry collides with a run-tests.cjs SUITES token ' + + `(${collisions.join(', ')}) — scripts/run-tests.cjs:651's selectExplicitFiles treats a ` + + 'registry entry that equals a suite name as a suite selector, not a filename, so this ' + + 'would silently run the entire suite instead of the intended file(s). Fix the registry entry.', + ); + } +} + +/** + * Map from docs-guard test file to the docs/ path patterns it reads. See + * this module's header comment for pattern semantics (exact / trailing-slash + * dir-prefix / `'*'` sentinel). Every value here was derived by reading the + * test file's actual read call(s) — not guessed — per #3753's own lesson: an + * unresolvable read is recorded as `'*'`, never a narrowed guess. + */ +const DOCS_GUARD_TESTS = { + 'tests/adr-15-progress-converge.test.cjs': [ + 'docs/COMMANDS.md', + 'docs/how-to/run-phases-autonomously.md', + ], + // Walks docs/adr/ as a directory (builds/reads docs/adr/README.md and + // fixture ADRs throughout) and separately reads docs/contributor-standards.md. + 'tests/adr-index-gate.test.cjs': ['docs/adr/', 'docs/contributor-standards.md'], + 'tests/agent-classification-parity.test.cjs': ['docs/AGENTS.md', 'docs/INVENTORY.md'], + 'tests/analyze-dependencies.test.cjs': ['docs/COMMANDS.md'], + 'tests/autonomous-converge.test.cjs': [ + 'docs/COMMANDS.md', + 'docs/how-to/run-phases-autonomously.md', + ], + 'tests/capability-matrix-sync.test.cjs': ['docs/reference/capability-matrix.md'], + 'tests/capability-registry.test.cjs': [ + 'docs/tutorials/build-your-first-capability.md', + 'docs/tutorials/install-your-first-capability.md', + 'docs/reference/capability-manifest.md', + ], + 'tests/claude-md.test.cjs': ['docs/COMMANDS.md'], + 'tests/claude-orchestration.test.cjs': ['docs/explanation/claude-orchestration-capability.md'], + 'tests/command-contract.test.cjs': [ + 'docs/INVENTORY.md', + 'docs/ja-JP/INVENTORY.md', + 'docs/ko-KR/INVENTORY.md', + 'docs/zh-CN/INVENTORY.md', + 'docs/pt-BR/INVENTORY.md', + ], + // uncoveredFiles(...) scans 'docs' as a generic coverage-scan root + // (commit-files-pathspec.test.cjs:1618) — cannot be resolved to specific + // files without re-deriving the scan's own file-discovery logic. + 'tests/commit-files-pathspec.test.cjs': ['*'], + 'tests/config-field-docs.test.cjs': ['docs/CONFIGURATION.md'], + 'tests/config.test.cjs': ['docs/CONFIGURATION.md'], + 'tests/context-index-sync.test.cjs': ['docs/CONTEXT-INDEX.json'], + 'tests/context-predicates-query.test.cjs': ['docs/contributor-standards.md'], + // SCAN_DIRS includes 'docs' and recursively walks every .md file under it + // (context7-tool-name-parity.test.cjs:31-38) — a generic tree walk, not a + // fixed file set. + 'tests/context7-tool-name-parity.test.cjs': ['*'], + 'tests/contributor-standards.test.cjs': ['docs/contributor-standards.md'], + 'tests/cursor-reviewer.test.cjs': [ + 'docs/COMMANDS.md', + 'docs/FEATURES.md', + 'docs/ja-JP/COMMANDS.md', + 'docs/ja-JP/FEATURES.md', + 'docs/ko-KR/COMMANDS.md', + 'docs/ko-KR/FEATURES.md', + ], + 'tests/discuss-all-flag.test.cjs': ['docs/COMMANDS.md'], + 'tests/discuss-mode.test.cjs': ['docs/workflow-discuss-mode.md'], + // Walks docs/*.md and every docs//*.md dir dynamically + // (docs-parity-live-registry.test.cjs:42, 428) — deliberately generic. + 'tests/docs-parity-live-registry.test.cjs': ['*'], + 'tests/drift-detection.test.cjs': ['docs/CONFIGURATION.md', 'docs/AGENTS.md'], + 'tests/edge-probe-docs-fixtures.test.cjs': ['docs/adr/550-spec-phase-probe-contract.md'], + 'tests/edit-phase.test.cjs': [ + 'docs/INVENTORY.md', + 'docs/INVENTORY-MANIFEST.json', + 'docs/COMMANDS.md', + ], + 'tests/effort-surface-axis.test.cjs': ['docs/reference/host-integration-capability-matrix.md'], + 'tests/execute-phase-active-flags.test.cjs': [ + 'docs/reference/host-integration-capability-matrix.md', + ], + 'tests/execute-phase-wave.test.cjs': ['docs/COMMANDS.md'], + 'tests/external-job-waiting.test.cjs': ['docs/reference/planning-artifacts.md'], + // Rows 8/9 (negative controls) read real docs/registries/eos.json and + // docs/adr/0001-dispatch-policy-module.md and assert on their EXACT + // committed content (an entry's name field, ADR-0001's H1 title) as a + // sanity check before mutating an overlay fixture — a content edit to + // either file changes the string this test asserts on. + 'tests/fragment-single-edit-propagation.install.test.cjs': [ + 'docs/registries/eos.json', + 'docs/adr/0001-dispatch-policy-module.md', + ], + 'tests/gsd-write-guard.test.cjs': ['docs/USER-GUIDE.md'], + 'tests/host-integration-descriptors.test.cjs': [ + 'docs/reference/host-integration-capability-matrix.md', + ], + 'tests/install.test.cjs': ['docs/AGENTS.md', 'docs/INVENTORY.md', 'docs/INVENTORY-MANIFEST.json'], + // SOURCE_DIRS includes 'docs' and walks it recursively for every .md file + // (intel.test.cjs:1223-1228) — a generic tree walk. + 'tests/intel.test.cjs': ['*'], + 'tests/inventory-headings-countfree.test.cjs': ['docs/INVENTORY.md'], + 'tests/inventory-manifest-sync.test.cjs': ['docs/INVENTORY.md', 'docs/INVENTORY-MANIFEST.json'], + 'tests/kilo-upgrades.test.cjs': ['docs/how-to/connect-gsd-mcp-server.md'], + 'tests/live-config-guard.test.cjs': ['docs/TESTING-SUITES.md'], + 'tests/model-catalog-runtime-defaults.test.cjs': ['docs/CONFIGURATION.md'], + // Scans every git-tracked file in the whole repo via `git ls-files` + // (no-pending-3212-markers.test.cjs:37-46), which includes all of docs/ — + // cannot be resolved to a fixed docs/ path set. + 'tests/no-pending-3212-markers.test.cjs': ['*'], + 'tests/phase6-capability-docs.test.cjs': ['docs/how-to/develop-a-capability.md', 'docs/README.md'], + 'tests/phase6-review-capabilities.test.cjs': [ + 'docs/reference/review-verification-capabilities.md', + 'docs/how-to/develop-a-capability.md', + 'docs/README.md', + ], + 'tests/plan-checker-coupling.test.cjs': ['docs/AGENTS.md'], + 'tests/plan-phase-drift-guard.test.cjs': [ + 'docs/CONFIGURATION.md', + 'docs/COMMANDS.md', + 'docs/USER-GUIDE.md', + 'docs/ARCHITECTURE.md', + 'docs/INVENTORY.md', + 'docs/INVENTORY-MANIFEST.json', + ], + 'tests/plan-phase-stall-detection.test.cjs': ['docs/CONFIGURATION.md'], + 'tests/plan-review-convergence.test.cjs': [ + 'docs/CONFIGURATION.md', + 'docs/USER-GUIDE.md', + 'docs/ARCHITECTURE.md', + ], + 'tests/planner-estimate-emission.test.cjs': ['docs/reference/plan-md.md', 'docs/CONFIGURATION.md'], + 'tests/precondition-element.test.cjs': ['docs/reference/plan-md.md'], + 'tests/product-name-purity.test.cjs': [ + 'docs/README.md', + 'docs/zh-CN/README.md', + 'docs/ko-KR/README.md', + 'docs/ja-JP/README.md', + 'docs/pt-BR/README.md', + ], + 'tests/progress-forensic.test.cjs': ['docs/COMMANDS.md'], + 'tests/repo-layout.test.cjs': ['docs/contributing/bootstrap.md'], + 'tests/reversibility-tagging.test.cjs': ['docs/reference/plan-md.md'], + 'tests/reviewer-docs-parity.test.cjs': [ + 'docs/COMMANDS.md', + 'docs/ja-JP/COMMANDS.md', + 'docs/ko-KR/COMMANDS.md', + 'docs/pt-BR/COMMANDS.md', + 'docs/zh-CN/COMMANDS.md', + 'docs/FEATURES.md', + 'docs/ja-JP/FEATURES.md', + 'docs/ko-KR/FEATURES.md', + 'docs/pt-BR/FEATURES.md', + 'docs/zh-CN/FEATURES.md', + ], + 'tests/runtime-converters.test.cjs': ['docs/CONFIGURATION.md'], + 'tests/secure-phase.test.cjs': ['docs/CONFIGURATION.md'], + 'tests/security-dead-exports.regression.test.cjs': ['docs/FEATURES.md'], + 'tests/security.test.cjs': ['docs/INVENTORY-MANIFEST.json'], + // Walks docs/ recursively (todos-done-rename-guard.test.cjs:19-22 + // SCAN_DIRS) looking for stale references — a generic tree walk. + 'tests/todos-done-rename-guard.test.cjs': ['*'], + 'tests/tracer-bullet.test.cjs': [ + 'docs/COMMANDS.md', + 'docs/reference/plan-md.md', + 'docs/how-to/plan-a-phase.md', + 'docs/AGENTS.md', + ], + 'tests/ui-spec-inventory-provenance.test.cjs': [ + 'docs/FEATURES.md', + 'docs/how-to/design-a-ui-phase.md', + 'docs/ja-JP/FEATURES.md', + 'docs/ja-JP/how-to/design-a-ui-phase.md', + 'docs/zh-CN/FEATURES.md', + 'docs/zh-CN/how-to/design-a-ui-phase.md', + 'docs/ko-KR/FEATURES.md', + 'docs/ko-KR/how-to/design-a-ui-phase.md', + 'docs/pt-BR/how-to/design-a-ui-phase.md', + ], + 'tests/verifier-behavior-unverified.test.cjs': ['docs/reference/planning-artifacts.md'], + 'tests/verifier-coincidental-reliance.test.cjs': ['docs/AGENTS.md'], + 'tests/verify.test.cjs': ['docs/reference/plan-md.md'], + 'tests/workflow-fragments.test.cjs': ['docs/reference/workflow-fragments.md'], +}; + +/** + * Flat file-list view of DOCS_GUARD_TESTS, kept for consumers (the + * registration lint and its parity test) that only need "which test files + * are registered", not their per-file docs path patterns. + */ +const DOCS_GUARD_TEST_FILES = Object.keys(DOCS_GUARD_TESTS); + +assertNoSuiteCollision(DOCS_GUARD_TEST_FILES); + +module.exports = { + DOCS_GUARD_TESTS, + DOCS_GUARD_TEST_FILES, + RUN_TESTS_SUITES, + assertNoSuiteCollision, + normalizeForSuiteCheck, +}; diff --git a/scripts/test-failure-reasons.cjs b/scripts/gsd-test-gate-reasons.cjs similarity index 76% rename from scripts/test-failure-reasons.cjs rename to scripts/gsd-test-gate-reasons.cjs index 22b411537..e8a173f50 100644 --- a/scripts/test-failure-reasons.cjs +++ b/scripts/gsd-test-gate-reasons.cjs @@ -1,5 +1,11 @@ 'use strict'; +// Renamed (was `test-failure-reasons.cjs`): the old basename matched Node's +// `test-*.EXT` test-collection pattern, so the remote push-gate runner +// collected and executed this SOURCE file as a test while GitHub CI (which +// globs only tests/**/*.test.cjs) never saw it — see +// scripts/lint-source-test-name-collision.cjs. + const TEST_GATE_REASON = Object.freeze({ PASS: 'pass', TEST_FAILURE: 'test_failure', diff --git a/scripts/lint-docs-guard-registration.cjs b/scripts/lint-docs-guard-registration.cjs new file mode 100644 index 000000000..7b293b75e --- /dev/null +++ b/scripts/lint-docs-guard-registration.cjs @@ -0,0 +1,495 @@ +#!/usr/bin/env node +'use strict'; + +/** + * lint-docs-guard-registration.cjs — every test file that READS a docs/ path + * (via a real filesystem read call, not merely a string mention) must either + * be named in the docs-guard lane registry, or carry an explicit + * `// docs-guard-exempt: ` marker. + * + * Exported pure function `checkDocsGuardRegistration({ testsDir, registry })` + * so tests can drive it against a synthetic fixture directory; also runnable + * as a CLI against the real tree (`node scripts/lint-docs-guard-registration.cjs`). + */ + +const fs = require('fs'); +const path = require('path'); +const { assertWithinAllowlist } = require('./lib/allowlist-ratchet.cjs'); +const { assertNoSuiteCollision } = require('./docs-guard-registry.cjs'); +const { + DOCS_GUARD_EXEMPT_BASELINE, + DOCS_GUARD_EXEMPT_DOCS_PATHS, +} = require('./lint-docs-guard-registration.exempt-baseline.cjs'); + +/** + * The lint's registry MUST derive from scripts/docs-guard-registry.cjs's + * `DOCS_GUARD_TESTS` export — not from a second, hand-maintained list. + * #3753 found exactly that split: this file used to carry its own + * DOCS_GUARD_REGISTRY literal (20 entries) while the list that actually + * drives the CI lane only had 10, so the lint reported "registered and + * fine" for ten guards the lane never ran — the silent gap #3753 exists to + * close, rebuilt one level down. Two lists holding one shared fact are free + * to drift; one list read from two places cannot. + * + * Returns basenames (test files all live flat under tests/), matching the + * shape `checkDocsGuardRegistration` expects. + */ +function deriveDocsGuardRegistry() { + const { DOCS_GUARD_TEST_FILES } = require('./docs-guard-registry.cjs'); + if (!Array.isArray(DOCS_GUARD_TEST_FILES) || DOCS_GUARD_TEST_FILES.length === 0) { + throw new Error( + 'lint-docs-guard-registration: scripts/docs-guard-registry.cjs exported an empty or ' + + 'missing DOCS_GUARD_TEST_FILES — cannot derive a registry. An empty registry would either flag ' + + 'every docs-reading test as unregistered, or (with an empty comparison set) silently pass ' + + 'with zero coverage. Fix the registry rather than defaulting this to [].', + ); + } + // Redundant with docs-guard-registry.cjs's own module-load-time self-check + // (both consumers of that module — this lint and the docs-required.yml + // derivation step — must independently reject a suite-token collision per + // #3753's security follow-up FIX 3), but kept explicit here rather than + // relying solely on the shared module's require()-time throw: this call + // makes the guard visible and independently testable from this file's own + // exports, instead of depending on an implicit side effect of another + // module's load. + assertNoSuiteCollision(DOCS_GUARD_TEST_FILES); + return DOCS_GUARD_TEST_FILES.map(t => path.basename(t)); +} + +// Real filesystem read calls we treat as "this file reads a path". A docs/ +// path appearing only inside a string literal handed to something else +// (e.g. assert.equal(msg, 'see docs/foo.md')) must NOT trip this. +// +// Detector 1 (segment-shaped): `fs.readFileSync(path.join(ROOT, 'docs', 'x.md'))`. +// The docs/ path is built from separate path-segment arguments to a known +// Node fs read function, so it looks for a bare `'docs'` (or `"docs"`/`` `docs` ``) +// segment, or a `'docs/...'` literal, inside the parenthesized argument list +// of one of READ_FN_NAMES. +const READ_FN_NAMES = ['readFileSync', 'readFile', 'readdirSync', 'readdir', 'createReadStream']; +const READ_CALL_RE = new RegExp( + `\\b(?:${READ_FN_NAMES.join('|')})\\s*\\(([^()]*(?:\\([^()]*\\)[^()]*)*)\\)`, + 'g', +); +const DOCS_QUOTED_PATH_RE = /(['"`])\/?docs\/[^'"`]*\1/; +const DOCS_QUOTED_SEGMENT_RE = /(['"`])docs\1/; + +// Detector 2 (single-string-shaped, #3753): `readShipped('docs/how-to/x.md')`. +// Detector 1 is blind to this spelling — the docs/ path is a single string +// literal, not a `path.join('docs', ...)` segment, and it is not always +// handed straight to a bare `fs.read*` call. This is exactly how +// tests/ui-spec-inventory-provenance.test.cjs reads — the guard whose +// unregistered drift broke `next` on dacae9273 and motivated #3753 in the +// first place — so a lint that cannot see its own motivating case's spelling +// is not a fix. +// +// A naive "flag any 'docs/...' string literal" rule produces 59 hits on the +// real tree (most are mention-only, e.g. assert messages), which is enough +// false-positive noise that the lint gets disabled rather than obeyed. This +// heuristic instead requires the docs/ literal to be an argument to a call +// whose CALLEE NAME looks like a reader (contains read/load/parse/shipped/ +// content/file/doc, case-insensitively) — e.g. `readShipped`, `readRepoFile`, +// `loadDoc`, `parseContent`. That name-shape restriction is what keeps the +// count at 6 new hits instead of 59, at the cost of also matching a few +// call sites (e.g. `groupFilesBySubrepo('docs/x.md', ...)`) whose name +// happens to contain one of those substrings without actually reading a +// file — those get `// docs-guard-exempt:` markers instead of registration. +// +// Detector 3 (co-occurrence, #3753 correctness follow-up — DEFECT B): the +// two detectors above only catch a docs/ PATH EXPRESSION passed INLINE, as +// an argument, to a call in the same statement. The dominant real idiom in +// this repo builds the path first (`const P = path.join(ROOT, 'docs', +// 'X.md');`) and reads it later (`fs.readFileSync(P);`) — a two-step form +// neither detector above can see, along with template-literal +// (`` `${ROOT}/docs/X.md` ``) and string-concat (`ROOT + '/docs/X.md'`) +// paths. This detector decouples "does the file build a docs/ path" from +// "does the file perform a read call" and flags the file when BOTH are true +// anywhere in it, regardless of whether they share a call site. This trades +// precision for recall deliberately: a false positive costs one +// `// docs-guard-exempt:` marker with a reason; a false negative is the +// #3753 bug shipping again. +// +// A docs/ path EXPRESSION is: a quoted 'docs/...' / "docs/..." literal +// (optionally with a leading slash, for the `ROOT + '/docs/x.md'` +// string-concat form), a backtick template literal containing `docs/` +// anywhere inside it (covers both a bare `` `docs/x.md` `` literal and an +// interpolated `` `${ROOT}/docs/x.md` `` prefix), or a bare `'docs'` segment +// passed as one of the arguments to a `path.join(...)` call anywhere in the +// file (not just when that call is itself an argument to a read function). +const DOCS_TEMPLATE_LITERAL_RE = /`[^`]*\bdocs\/[^`]*`/; +const PATH_JOIN_CALL_RE = /\bpath\s*\.\s*join\s*\(([^()]*(?:\([^()]*\)[^()]*)*)\)/g; + +function pathJoinHasDocsSegment(content) { + let match; + PATH_JOIN_CALL_RE.lastIndex = 0; + while ((match = PATH_JOIN_CALL_RE.exec(content)) !== null) { + if (DOCS_QUOTED_SEGMENT_RE.test(match[1]) || DOCS_QUOTED_PATH_RE.test(match[1])) return true; + } + return false; +} + +function hasDocsPathExpression(content) { + return DOCS_QUOTED_PATH_RE.test(content) || + DOCS_TEMPLATE_LITERAL_RE.test(content) || + pathJoinHasDocsSegment(content); +} + +// A "real read" for the co-occurrence detector: any call to a known Node fs +// read function, ANYWHERE in the file — deliberately not requiring its +// argument to look like a docs/ path here (that pairing is what +// hasDocsPathExpression establishes separately). Does not include +// `existsSync`: a pure existence check with no content read is exactly the +// "incidental" case this lint's exemption path exists for, not a guard. +const READ_CALL_PRESENT_RE = new RegExp(`\\b(?:${READ_FN_NAMES.join('|')})\\s*\\(`); + +// Detector 2 (single-string-shaped, #3753): `readShipped('docs/how-to/x.md')`. +// Detector 1 is blind to this spelling — the docs/ path is a single string +// literal, not a `path.join('docs', ...)` segment, and it is not always +// handed straight to a bare `fs.read*` call. This is exactly how +// tests/ui-spec-inventory-provenance.test.cjs reads — the guard whose +// unregistered drift broke `next` on dacae9273 and motivated #3753 in the +// first place — so a lint that cannot see its own motivating case's spelling +// is not a fix. +// +// A naive "flag any 'docs/...' string literal" rule produces 59 hits on the +// real tree (most are mention-only, e.g. assert messages), which is enough +// false-positive noise that the lint gets disabled rather than obeyed. This +// heuristic instead requires the docs/ literal to be an argument to a call +// whose CALLEE NAME looks like a reader (contains read/load/parse/shipped/ +// content/file/doc, case-insensitively) — e.g. `readShipped`, `readRepoFile`, +// `loadDoc`, `parseContent`. The callee-name test is applied to the WHOLE +// captured identifier (never requiring a non-keyword prefix before it — see +// DEFECT A, #3753 correctness follow-up: a prior version of this regex wove +// the keyword alternation into the SAME character class as a mandatory +// leading identifier-start character, which made it structurally impossible +// for the keyword to start at index 0 and silently missed every bare `read(`, +// `load(`, `parse(`, `content(`, `file(`, and `doc(` callee — this repo's +// most common reader-helper name among them), at the cost of also matching a +// few call sites (e.g. `groupFilesBySubrepo('docs/x.md', ...)`) whose name +// happens to contain one of those substrings without actually reading a +// file — those get `// docs-guard-exempt:` markers instead of registration. +const READER_NAME_KEYWORDS_RE = /read|load|parse|shipped|content|file|doc/i; +const READER_CALL_RE = /\b([A-Za-z_$][\w$]*)\s*\(\s*[^)]{0,60}?["'`]docs\//gi; + +function readerNameCallHasDocsPath(content) { + let match; + READER_CALL_RE.lastIndex = 0; + while ((match = READER_CALL_RE.exec(content)) !== null) { + if (READER_NAME_KEYWORDS_RE.test(match[1])) return true; + } + return false; +} + +// Neither detector can see every spelling a docs read could take (e.g. a +// path built through an indirect helper with a read-agnostic name whose call +// site never contains a real fs read call in the same file). This registry +// is a curated, best-effort net with known holes, not an exhaustive static +// analysis. +function argsReadDocsPath(args) { + return DOCS_QUOTED_PATH_RE.test(args) || DOCS_QUOTED_SEGMENT_RE.test(args); +} + +function readsDocsPath(content) { + let match; + READ_CALL_RE.lastIndex = 0; + while ((match = READ_CALL_RE.exec(content)) !== null) { + if (argsReadDocsPath(match[1])) return true; + } + if (readerNameCallHasDocsPath(content)) return true; + return hasDocsPathExpression(content) && READ_CALL_PRESENT_RE.test(content); +} + +// Only scan the file's HEADER — the first EXEMPTION_SCAN_LINES lines. Scanning +// the whole file lets a `// docs-guard-exempt:`-shaped string embedded in a +// fixture/template literal (this lint's own test file writes exactly such +// strings to synthesize fixtures) exempt the entire real file it appears in. +// A header marker convention closes that hole while still finding every +// genuine exemption comment, which by convention sits near the top of the +// file next to its module docstring. +const EXEMPTION_SCAN_LINES = 20; + +// DEFECT C (#3753 correctness follow-up): a marker is only honored when the +// line it appears on is an actual COMMENT, not merely a line whose text +// contains the marker shape. Probed pre-fix: a string literal like +// const s = "// docs-guard-exempt: whatever"; +// self-exempted the file with zero signal — the header-window narrowing +// (EXEMPTION_SCAN_LINES) constrains WHERE the marker may appear but never +// constrained WHAT KIND of line it must be, so relocating the marker inside +// a string literal anywhere in the header window still worked. Requiring +// the line, after trimming leading whitespace, to actually START with a +// comment token (`//`, `/*`, or a JSDoc-block `*` continuation line) closes +// this without narrowing the legitimate cases: every real marker in this +// repo's tests/ sits at column 0 as a full-line `//` comment (verified +// against every current `docs-guard-exempt:` occurrence in tests/). +const EXEMPTION_LINE_IS_COMMENT_RE = /^(?:\/\/|\/\*|\*)/; + +// Security follow-up FIX 4: a marker line inside a multi-line template +// literal in the header window (e.g. `const F = \`\n// docs-guard-exempt: x\n\`;`) +// still exempted the file pre-fix — the line, taken on its own, starts with +// `//` and so passed EXEMPTION_LINE_IS_COMMENT_RE even though it is actually +// backtick-string CONTENT, not a real comment. Track backtick parity across +// lines: a line whose entire span is inside an open template literal (i.e. +// the literal was already open when the line STARTED) is never honored as a +// comment, no matter what its own text looks like. +function findExemption(content) { + const lines = content.split(/\r?\n/).slice(0, EXEMPTION_SCAN_LINES); + let inTemplateLiteral = false; + let inBlockComment = false; + for (const line of lines) { + const lineStartedInTemplateLiteral = inTemplateLiteral; + // Count unescaped backticks on this line to toggle template-literal state. + const backtickCount = (line.match(/\\`|`/g) || []).filter((tok) => tok === '`').length; + if (backtickCount % 2 === 1) inTemplateLiteral = !inTemplateLiteral; + if (lineStartedInTemplateLiteral) continue; + + // Track (real) /* ... */ block-comment state across lines: a line that + // is genuinely CONTENT inside an open, unterminated block comment is + // still a real comment (JS syntax), so it stays eligible — this state is + // tracked only so a future check can distinguish "inside a real block + // comment" from "inside a template literal" rather than conflating them. + const opensBlockComment = /\/\*/.test(line); + const closesBlockComment = /\*\//.test(line); + if (!inBlockComment && opensBlockComment && !closesBlockComment) { + inBlockComment = true; + } else if (inBlockComment && closesBlockComment) { + inBlockComment = false; + } + + if (!EXEMPTION_LINE_IS_COMMENT_RE.test(line.trimStart())) continue; + const m = /\/\/\s*docs-guard-exempt:(.*)$/.exec(line); + if (m) return { present: true, reason: m[1].trim() }; + } + return { present: false, reason: '' }; +} + +/** + * Security follow-up FIX 3: the exempt ratchet gated on file IDENTITY only — + * a baselined file that later STARTS genuinely reading shipped docs/ content + * stayed exempt with zero signal (probed: a baselined file doing + * `fs.readFileSync('docs/foo.md')` still reported `ok=true, violations=[]`). + * This extracts every distinct `docs/...` path TOKEN referenced anywhere in + * an exempted file's content, so the baseline can pin a per-file fingerprint + * of what it references and fail loudly when that set changes — forcing a + * human to re-confirm the exemption still holds. Deliberately broader than + * "only tokens passed to a read call" (this module's readsDocsPath heuristics + * above): the fingerprint's job is to catch ANY drift in what docs/ paths an + * exempted file mentions, not just the ones already read-call-shaped, since + * a mention today can become a read call tomorrow without changing the + * mention text at all. + */ +const DOCS_PATH_TOKEN_RE = /\/?docs\/[A-Za-z0-9_./-]*[A-Za-z0-9_-]/g; + +/** + * @param {string} content + * @returns {string[]} sorted, deduped list of docs/ path tokens referenced — + * a readable, diffable fingerprint (never an opaque hash) so a reviewer can + * see exactly what changed. + */ +function extractDocsPathReferences(content) { + const set = new Set(); + let m; + DOCS_PATH_TOKEN_RE.lastIndex = 0; + while ((m = DOCS_PATH_TOKEN_RE.exec(content)) !== null) { + set.add(m[0].replace(/^\//, '')); + } + return [...set].sort(); +} + +/** + * Compare each currently-exempted file's live docs-path fingerprint against + * its pinned baseline fingerprint. A file present in `current` whose sorted + * path list differs from the baseline's (paths added OR removed) is a + * violation: the exemption's premise ("this file doesn't really guard + * shipped docs content") may no longer hold and a human must re-confirm it. + * + * @param {Record} current - file -> live sorted docs/ path list. + * @param {Record} baseline - file -> pinned sorted docs/ path list. + * @returns {Array<{ file: string, reason: string }>} + */ +function checkExemptFingerprints(current, baseline) { + const violations = []; + for (const [file, paths] of Object.entries(current)) { + const baselinePaths = baseline[file]; + if (!baselinePaths) continue; // identity ratchet (checkExemptBaseline) already flags a novel exemption + const currentJoined = paths.join('\n'); + const baselineJoined = [...baselinePaths].sort().join('\n'); + if (currentJoined !== baselineJoined) { + violations.push({ + file, + reason: + `the docs paths referenced by ${file} changed; re-confirm the exemption still holds and ` + + `update the baseline in ${EXEMPT_BASELINE_FILE}:${EXEMPT_BASELINE_DOCS_PATHS_CONST}. ` + + `was: [${baselinePaths.join(', ') || '(none)'}], now: [${paths.join(', ') || '(none)'}]`, + }); + } + } + return violations; +} + +/** + * File referenced in this module's own remedy messages below (kept as a + * named constant so the CLI path and the message text cannot drift). + */ +const EXEMPT_BASELINE_FILE = 'scripts/lint-docs-guard-registration.exempt-baseline.cjs'; +const EXEMPT_BASELINE_CONST = 'DOCS_GUARD_EXEMPT_BASELINE'; +const EXEMPT_BASELINE_DOCS_PATHS_CONST = 'DOCS_GUARD_EXEMPT_DOCS_PATHS'; + +/** + * Ratchet the `// docs-guard-exempt:` marker on file identity, mirroring + * scripts/lint-allow-test-rule-refs.cjs's `allow-test-rule` identity ratchet + * (scripts/lib/allowlist-ratchet.cjs, ADR-456's pattern): a NEW exemption not + * already in the pinned baseline fails the lint, and a baseline entry whose + * file no longer carries a real marker (removed, renamed, or the marker was + * deleted) is reported STALE and must be pruned — the baseline only ever + * moves by deliberate edit, never silently grows or goes stale unnoticed. + * + * Unlike the sibling gate's generic "do not just add to the allowlist" + * framing (which fits an offender-tracking allowlist), the correct remedy + * for a genuinely-warranted new docs-guard exemption really is to add it to + * the baseline — so the novel-entry case gets its own explicit remedy line + * here rather than reusing that wording verbatim. + * + * @param {string[]} exemptedFiles - basenames with a present, non-empty + * `docs-guard-exempt:` marker, found in the current tree. + * @param {string[]} baseline - the pinned baseline (DOCS_GUARD_EXEMPT_BASELINE). + * @returns {Array<{ file: string, reason: string }>} + */ +function checkExemptBaseline(exemptedFiles, baseline) { + const violations = []; + const { novel } = assertWithinAllowlist({ + label: 'docs-guard-exempt', + current: exemptedFiles, + known: baseline, + fail: (msg) => violations.push({ file: '(docs-guard-exempt baseline)', reason: msg }), + pruneHint: + `its file was removed, renamed, or no longer carries a docs-guard-exempt marker — ` + + `prune it from the baseline in ${EXEMPT_BASELINE_FILE}:${EXEMPT_BASELINE_CONST}`, + }); + if (novel.length > 0) { + violations.push({ + file: '(docs-guard-exempt baseline)', + reason: + `${novel.length} NEW docs-guard-exempt marker(s) not in the pinned baseline: ` + + `${novel.join(', ')} — if this exemption is correct, add it to the baseline in ` + + `${EXEMPT_BASELINE_FILE}:${EXEMPT_BASELINE_CONST}`, + }); + } + return violations; +} + +/** + * @param {{ testsDir: string, registry: string[], exemptBaseline?: string[] }} opts + * `exemptBaseline`, when provided, ratchets the docs-guard-exempt marker on + * file identity against that pinned list (see `checkExemptBaseline`). + * Omitted entirely by callers (e.g. isolated fixture-only tests) that do + * not want the ratchet applied. + * @returns {{ ok: boolean, violations: Array<{ file: string, reason: string }>, exemptedFiles: string[] }} + */ +function checkDocsGuardRegistration({ testsDir, registry, exemptBaseline, exemptDocsPathsBaseline }) { + const violations = []; + const exemptedFiles = []; + const exemptedDocsPaths = {}; + + for (const entry of registry) { + const full = path.join(testsDir, entry); + if (!fs.existsSync(full)) { + violations.push({ file: entry, reason: `registry entry does not exist on disk: ${full}` }); + } + } + + const registrySet = new Set(registry); + let files; + try { + files = fs.readdirSync(testsDir).filter(f => f.endsWith('.test.cjs')); + } catch (err) { + // A guard that cannot read its own input must never report success. + // Probed pre-fix: checkDocsGuardRegistration({testsDir:'/nonexistent', + // registry:[]}) returned {ok:true, violations:[]} — a green check that + // guarded nothing. Treat an unreadable testsDir as a HARD VIOLATION. + return { + ok: false, + violations: [ + { + file: '(testsDir)', + reason: `cannot read testsDir ${testsDir}: ${err.message} — a docs-guard registration ` + + 'lint that cannot read its own input must fail, never silently report zero violations', + }, + ], + exemptedFiles: [], + }; + } + + for (const file of files) { + const full = path.join(testsDir, file); + let content; + try { + content = fs.readFileSync(full, 'utf8'); + } catch (err) { + // Same class: a directory or broken symlink named `*.test.cjs` (or any + // other read failure) must be a hard violation, not a silent skip. + violations.push({ + file, + reason: `cannot read candidate test file ${full}: ${err.message} — an unreadable ` + + 'docs-guard candidate must fail the lint, not be silently skipped', + }); + continue; + } + + const exemption = findExemption(content); + if (exemption.present) { + if (exemption.reason.length === 0) { + violations.push({ file, reason: 'docs-guard-exempt marker present with no reason' }); + } else { + exemptedFiles.push(file); + exemptedDocsPaths[file] = extractDocsPathReferences(content); + } + continue; + } + + if (readsDocsPath(content) && !registrySet.has(file)) { + violations.push({ + file, + reason: 'reads a docs/ path but is not registered in the docs-guard lane and carries no docs-guard-exempt marker', + }); + } + } + + if (exemptBaseline) { + violations.push(...checkExemptBaseline(exemptedFiles, exemptBaseline)); + } + + if (exemptDocsPathsBaseline) { + violations.push(...checkExemptFingerprints(exemptedDocsPaths, exemptDocsPathsBaseline)); + } + + return { ok: violations.length === 0, violations, exemptedFiles, exemptedDocsPaths }; +} + +module.exports = { + checkDocsGuardRegistration, + checkExemptBaseline, + checkExemptFingerprints, + extractDocsPathReferences, + deriveDocsGuardRegistry, + EXEMPT_BASELINE_FILE, + EXEMPT_BASELINE_CONST, + EXEMPT_BASELINE_DOCS_PATHS_CONST, +}; + +if (require.main === module) { + const ROOT = path.join(__dirname, '..'); + const result = checkDocsGuardRegistration({ + testsDir: path.join(ROOT, 'tests'), + registry: deriveDocsGuardRegistry(), + exemptBaseline: DOCS_GUARD_EXEMPT_BASELINE, + exemptDocsPathsBaseline: DOCS_GUARD_EXEMPT_DOCS_PATHS, + }); + if (!result.ok) { + process.stderr.write(`lint-docs-guard-registration: ${result.violations.length} violation(s)\n`); + for (const v of result.violations) { + process.stderr.write(` ${v.file}: ${v.reason}\n`); + } + process.exitCode = 1; + } else { + console.log('ok lint-docs-guard-registration: 0 violations'); + } +} diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs new file mode 100644 index 000000000..a9486526c --- /dev/null +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -0,0 +1,174 @@ +'use strict'; + +/** + * Pinned baseline for scripts/lint-docs-guard-registration.cjs's + * `docs-guard-exempt` identity ratchet — mirrors + * scripts/lint-allow-test-rule-refs.cjs's `allow-test-rule` ratchet + * (scripts/lib/allowlist-ratchet.cjs, ADR-456's pattern). + * + * Every test file basename here is a KNOWN, already-reviewed + * `// docs-guard-exempt: ` marker. Nothing caps how many files may + * carry the marker in the abstract, but ADDING a new one anywhere in + * tests/*.test.cjs fails `lint-docs-guard-registration` until this baseline + * is deliberately updated — so a new exemption always shows up as a + * reviewable diff here, instead of silently opting a file out of the + * docs-guard registration requirement with zero signal. + * + * A file listed here that no longer exists, or no longer carries a real + * `docs-guard-exempt:` marker in its first 20 lines, is reported STALE and + * must be pruned (ratchet-down, same as the sibling gate). + * + * Seeded 2026-08-23 (#3753 security follow-up FIX 2) from every real marker + * found via scripts/lint-docs-guard-registration.cjs's own `findExemption` + * scan of tests/*.test.cjs — derived, not retyped from memory. + * + * Extended 2026-08-23 (#3753 correctness follow-up): the reader-detection + * fixes (Defects A/B/C) surfaced 86 previously-undetected docs/ readers. 40 + * were genuine docs-content guards (added to DOCS_GUARD_TESTS in + * scripts/docs-guard-registry.cjs); the 46 added here touch a docs/ path + * only incidentally (fixture data, external URL citations, comment-only + * mentions, metadata labels, or scan-exclusion lists) — see each file's own + * `docs-guard-exempt:` marker for its specific reason. + */ +const DOCS_GUARD_EXEMPT_BASELINE = [ + 'adr-parser.test.cjs', + 'adr-parser.unit.test.cjs', + 'agent-marker-documentation-guard.test.cjs', + 'antigravity-upgrades.test.cjs', + 'capability-cli.test.cjs', + 'ci-docs-guard-registry.test.cjs', + 'ci-test-scope.test.cjs', + 'cline-install.test.cjs', + 'code-review-depth.test.cjs', + 'code-review-pipeline-regression.test.cjs', + 'codebuddy-upgrades.test.cjs', + 'commands.test.cjs', + 'commit-docs-bypass.test.cjs', + 'complexity-trigger.test.cjs', + 'cursor-imperative-reference.test.cjs', + 'declarative-reference-antigravity.test.cjs', + 'declarative-reference-zcode.test.cjs', + 'emitted-attribution.test.cjs', + 'eslint-rules.test.cjs', + 'estimate-calibrate.test.cjs', + 'gen-context-index.test.cjs', + 'gen-registry.test.cjs', + 'gsd-agent-isolation-guard.test.cjs', + 'hermes-dispatch-upgrade.test.cjs', + 'install-minimal-hooks.test.cjs', + 'install-runtime-artifacts.test.cjs', + 'installer-migration-config-root-marker.test.cjs', + 'installer-migration-pi-extension-ext.test.cjs', + 'installer-migrations.test.cjs', + 'kimi-upgrades.test.cjs', + 'lint-allow-test-rule-refs.test.cjs', + 'lint-docs-command-form.test.cjs', + 'lint-docs-required.test.cjs', + 'manifest-version-sync.test.cjs', + 'milestone-archive.test.cjs', + 'model-resolver.test.cjs', + 'new-project-mvp-prompt.test.cjs', + 'onboard-command.test.cjs', + 'opencode-command-dir-plural.test.cjs', + 'phase.test.cjs', + 'pr-branch-planning-filter.test.cjs', + 'precommit-alias-drift-hook.test.cjs', + 'removed-but-needed-lint.test.cjs', + 'repo-invariants.test.cjs', + 'require-issue-link-policy.test.cjs', + 'reviewer-manifest-body.test.cjs', + 'run-tests-harness.test.cjs', + 'runtime-name-policy.test.cjs', + 'security-prompt-injection.security.test.cjs', + 'shipped-reference-cites.test.cjs', + 'state.test.cjs', + 'worktree-safety.test.cjs', +]; + +/** + * Security follow-up FIX 3: a per-file fingerprint of every distinct + * `docs/...` path TOKEN referenced by each baselined file (derived via + * scripts/lint-docs-guard-registration.cjs's `extractDocsPathReferences`, + * never retyped from memory). A baselined file whose live fingerprint + * DIFFERS from the recorded one here fails the lint — the exemption's + * premise ("this file doesn't really guard shipped docs content") may no + * longer hold and a human must re-confirm it before the entry is updated. + * Keeping this a plain, sorted, diffable path list (not a hash) is + * deliberate: a reviewer can see exactly WHAT changed from the PR diff + * alone. + * + * Seeded 2026-08-23 alongside DOCS_GUARD_EXEMPT_BASELINE above, from the + * exact same scan. + */ +const DOCS_GUARD_EXEMPT_DOCS_PATHS = { + 'adr-parser.test.cjs': ['docs/adr/0001.md', 'docs/adr/0002.md', 'docs/adr/0010.md', 'docs/adr/NNNN.md'], + 'adr-parser.unit.test.cjs': ['docs/adr/0001.md', 'docs/adr/0099.md', 'docs/adr/NNNN.md'], + 'agent-marker-documentation-guard.test.cjs': ['docs/reference', 'docs/reference/workflow-fragments.md'], + 'antigravity-upgrades.test.cjs': ['docs/cli', 'docs/cli/gcli-migration', 'docs/cli/permissions'], + 'capability-cli.test.cjs': ['docs/reference/gsd-capability-command.md'], + 'ci-docs-guard-registry.test.cjs': [ + 'docs/AGENTS.md', 'docs/COMMANDS.md', 'docs/INVENTORY.md', 'docs/a.md', 'docs/adr', + 'docs/adr/0001-example.md', 'docs/adrenaline.md', 'docs/bar.md', 'docs/foo.md', 'docs/how-to/foo.md', + 'docs/how-to/some-unrelated-guide.md', 'docs/how-to/x.md', 'docs/some-unrelated-file.md', + 'docs/totally-unrelated.md', + ], + 'ci-test-scope.test.cjs': [ + 'docs/a.md', 'docs/adr', 'docs/adr/22-plan-drift-guard.md', 'docs/how-to/configure-model-profiles.md', + 'docs/installer-migrations.md', 'docs/ja-JP', 'docs/ja-JP/USAGE.md', 'docs/usage.md', 'docs/x.md', + ], + 'cline-install.test.cjs': ['docs/guide.md'], + 'code-review-depth.test.cjs': ['docs/src/auth/x.ts'], + 'code-review-pipeline-regression.test.cjs': ['docs/DEVELOPMENT.md'], + 'codebuddy-upgrades.test.cjs': ['docs/cli/sub-agents'], + 'commands.test.cjs': ['docs/x.md'], + 'commit-docs-bypass.test.cjs': [ + 'docs/40-design.md', 'docs/CONFIGURATION.md', 'docs/readme.md', 'docs/tracked-var-mentioning', + ], + 'complexity-trigger.test.cjs': ['docs/readme.md'], + 'cursor-imperative-reference.test.cjs': ['docs/sdk/typescript'], + 'declarative-reference-antigravity.test.cjs': ['docs/cli/features'], + 'declarative-reference-zcode.test.cjs': ['docs/reference/host-integration-capability-matrix.md'], + 'emitted-attribution.test.cjs': ['docs/README.md', 'docs/tests', 'docs/tests/helpers/install-shared.cjs'], + 'eslint-rules.test.cjs': ['docs/readme.md'], + 'estimate-calibrate.test.cjs': [ + 'docs/adr', 'docs/adr/2629-phase-effort-estimation-calibration.md', 'docs/reference', + 'docs/reference/planning-artifacts.md', + ], + 'gen-context-index.test.cjs': ['docs/CONTEXT-INDEX.json', 'docs/INVENTORY-MANIFEST.json'], + 'gen-registry.test.cjs': ['docs/registries', 'docs/registries/reviewers.json'], + 'gsd-agent-isolation-guard.test.cjs': ['docs/adr/1239-...md', 'docs/adr/1239-gsd-embeddable-orchestration-engine.md'], + 'hermes-dispatch-upgrade.test.cjs': ['docs/guides/delegation-patterns.md'], + 'install-minimal-hooks.test.cjs': ['docs/en/hooks', 'docs/en/users/features/hooks'], + 'install-runtime-artifacts.test.cjs': ['docs/adr/58-...md', 'docs/adr/58-runtime-install-policy-module.md', 'docs/cli/slash-commands'], + 'installer-migration-config-root-marker.test.cjs': ['docs/installer-migrations.md'], + 'installer-migration-pi-extension-ext.test.cjs': ['docs/installer-migrations.md'], + 'installer-migrations.test.cjs': ['docs/installer-migrations.md'], + 'kimi-upgrades.test.cjs': ['docs/reference/host-integration-capability-matrix.md'], + 'lint-allow-test-rule-refs.test.cjs': ['docs/readme.md'], + 'lint-docs-command-form.test.cjs': ['docs/adr', 'docs/adr/999-example.md', 'docs/how-to/example.md'], + 'lint-docs-required.test.cjs': [ + 'docs/COMMANDS.md', 'docs/USER-GUIDE.md', 'docs/adr', 'docs/adr/0001-foo.md', 'docs/adr/0099-new.md', + 'docs/agents', 'docs/agents/triage-labels.md', + ], + 'manifest-version-sync.test.cjs': [], + 'milestone-archive.test.cjs': ['docs/TESTING-SUITES.md'], + 'model-resolver.test.cjs': ['docs/TESTING-SUITES.md'], + 'new-project-mvp-prompt.test.cjs': ['docs/CONFIGURATION.md'], + 'onboard-command.test.cjs': ['docs/adr/0001-runtime.md'], + 'opencode-command-dir-plural.test.cjs': ['docs/commands'], + 'phase.test.cjs': ['docs/adr/3524-...md', 'docs/adr/3524-cjs-sdk-hard-seam.md'], + 'pr-branch-planning-filter.test.cjs': ['docs/readme.md'], + 'precommit-alias-drift-hook.test.cjs': ['docs/adr/0174-...md', 'docs/adr/0174-retire-gsd-sdk-package-boundary.md'], + 'removed-but-needed-lint.test.cjs': ['docs/getting-started.md', 'docs/gsd-new-workspace.md', 'docs/setup.md'], + 'repo-invariants.test.cjs': ['docs/FEATURES.md', 'docs/workflows/README'], + 'require-issue-link-policy.test.cjs': ['docs/-prefixed', 'docs/CONFIGURATION.md', 'docs/a.md', 'docs/b.md', 'docs/guide.md'], + 'reviewer-manifest-body.test.cjs': ['docs/how-to/ship-a-reviewer-lane.md'], + 'run-tests-harness.test.cjs': ['docs/TESTING-SUITES.md'], + 'runtime-name-policy.test.cjs': ['docs/customize/skills'], + 'security-prompt-injection.security.test.cjs': ['docs/notes.md'], + 'shipped-reference-cites.test.cjs': [], + 'state.test.cjs': ['docs/CONFIGURATION.md'], + 'worktree-safety.test.cjs': ['docs/SUMMARY.md'], +}; + +module.exports = { DOCS_GUARD_EXEMPT_BASELINE, DOCS_GUARD_EXEMPT_DOCS_PATHS }; diff --git a/scripts/lint-fix-has-regression-test.cjs b/scripts/lint-fix-has-regression-tests.cjs similarity index 79% rename from scripts/lint-fix-has-regression-test.cjs rename to scripts/lint-fix-has-regression-tests.cjs index 2707ca01b..b365a7729 100644 --- a/scripts/lint-fix-has-regression-test.cjs +++ b/scripts/lint-fix-has-regression-tests.cjs @@ -2,10 +2,16 @@ 'use strict'; /** - * lint-fix-has-regression-test.cjs — gate: every fix(#NNNN) commit must + * lint-fix-has-regression-tests.cjs — gate: every fix(#NNNN) commit must * include at least one behavioral test file (tests/*.test.cjs) that is NOT * an auto-generated fixture/baseline. * + * Renamed (was `lint-fix-has-regression-test.cjs`, singular): the old + * basename matched Node's `*-test.EXT` test-collection pattern, so the + * remote push-gate runner collected and executed this SOURCE file as a + * test while GitHub CI (which globs only tests/**\/*.test.cjs) never saw + * it — see scripts/lint-source-test-name-collision.cjs. + * * ## Why * * CONTRIBUTING.md:47: "Fix it. Write a test that would have caught the bug." @@ -82,7 +88,7 @@ function getChangedTestFiles(baseRef) { function main() { if (process.env.GSD_SKIP_REGRESSION_TEST_GATE === '1') { - console.log('lint-fix-has-regression-test: SKIPPED (GSD_SKIP_REGRESSION_TEST_GATE=1)'); + console.log('lint-fix-has-regression-tests: SKIPPED (GSD_SKIP_REGRESSION_TEST_GATE=1)'); return; } @@ -92,12 +98,12 @@ function main() { try { fixCommits = getFixCommits(baseRef); } catch { - console.log(`lint-fix-has-regression-test: no fix/feat commits found vs ${baseRef}, skipping`); + console.log(`lint-fix-has-regression-tests: no fix/feat commits found vs ${baseRef}, skipping`); return; } if (fixCommits.length === 0) { - console.log('lint-fix-has-regression-test: no fix/feat commits, passing'); + console.log('lint-fix-has-regression-tests: no fix/feat commits, passing'); return; } @@ -113,7 +119,7 @@ function main() { .map((c) => ` ${c.sha} ${c.subject}`) .join('\n'); throw new ExitError(1, - `lint-fix-has-regression-test: ${fixCommits.length} fix/feat commit(s) but ZERO behavioral test files (*.test.cjs) in the diff.\n` + + `lint-fix-has-regression-tests: ${fixCommits.length} fix/feat commit(s) but ZERO behavioral test files (*.test.cjs) in the diff.\n` + `Auto-generated fixtures (tests/fixtures/, *-baseline.json) do NOT count.\n\n` + `Fix commits:\n${commitList}\n\n` + `CONTRIBUTING.md:47: "Write a test that would have caught the bug."\n` + @@ -123,7 +129,7 @@ function main() { } console.log( - `lint-fix-has-regression-test: PASS — ${fixCommits.length} fix/feat commit(s), ` + + `lint-fix-has-regression-tests: PASS — ${fixCommits.length} fix/feat commit(s), ` + `${testFiles.length} behavioral test file(s): ${testFiles.join(', ')}` ); } diff --git a/scripts/lint-health-diagnostic-rule-table.cjs b/scripts/lint-health-diagnostic-rule-table.cjs index 893108243..dedb50c9e 100644 --- a/scripts/lint-health-diagnostic-rule-table.cjs +++ b/scripts/lint-health-diagnostic-rule-table.cjs @@ -58,7 +58,7 @@ * real, non-mocked `buildPlanningSnapshot(tmpCwd)` call (see * `tests/health-diagnostic-rules/root-existence.test.cjs`). This guard * therefore verifies the fixture-proof invariant STATICALLY against the - * test files' own text — mirroring `scripts/lint-fix-has-regression-test.cjs`'s + * test files' own text — mirroring `scripts/lint-fix-has-regression-tests.cjs`'s * house style — rather than dynamically re-running fixture-building code * this guard does not own. */ diff --git a/scripts/lint-removed-but-needed.cjs b/scripts/lint-removed-but-needed.cjs index b97aaa5d5..8c5cdfd04 100644 --- a/scripts/lint-removed-but-needed.cjs +++ b/scripts/lint-removed-but-needed.cjs @@ -215,7 +215,7 @@ function getDeletedFiles(root, baseRef) { // Deliberately let a git failure (unresolvable ref, no merge base, etc.) // propagate as a plain Error — main() treats ANY scan() failure as "cannot // resolve this base ref in this environment" and degrades to a skip, - // matching lint-fix-has-regression-test.cjs. There is no failure mode here + // matching lint-fix-has-regression-tests.cjs. There is no failure mode here // that should hard-exit non-zero; a real drift is only ever reported once // the diff succeeds and findSurvivingReferences finds a violation. const out = cp.execFileSync('git', ['diff', '--name-status', `${baseRef}...HEAD`], { @@ -284,7 +284,7 @@ function main() { } catch (e) { // origin/ unreachable in this environment (e.g. a shallow local // clone with no matching remote-tracking ref) — degrade to a skip rather - // than a false failure, matching lint-fix-has-regression-test.cjs. + // than a false failure, matching lint-fix-has-regression-tests.cjs. console.log(`lint-removed-but-needed: could not resolve ${baseRef}, skipping (${e.message})`); return; } diff --git a/scripts/lint-source-test-name-collision.cjs b/scripts/lint-source-test-name-collision.cjs new file mode 100644 index 000000000..a0e0c040b --- /dev/null +++ b/scripts/lint-source-test-name-collision.cjs @@ -0,0 +1,241 @@ +#!/usr/bin/env node +'use strict'; + +/** + * lint-source-test-name-collision.cjs — no SOURCE file may carry a basename + * that matches Node's built-in test-file collection patterns. + * + * ## Why (the incident) + * + * `src/test-home-guard.cts` was a SOURCE module (a runtime guard, not a + * test) whose filename happened to match Node's `test-*` collection + * convention. `scripts/run-tests.cjs` — what local `npm test` and GitHub CI + * use — globs only `tests/**\/*.test.cjs`, so CI never saw it. But the + * REMOTE test runner (the push gate; see CLAUDE.md's `gsd-test` section) + * collects test files the way `node --test` does by default, across the + * whole tree, so it picked the file up and executed it AS a test, where it + * exited 1. Net effect: `next` was green on GitHub CI and red on the push + * gate, blocking every push repo-wide. Proven by a control run on the + * unmodified `next` tip 622f43353: `37199 passed / 1 failed`, + * `throw · src/test-home-guard.cts`. The file has since been renamed to + * `src/real-home-guard.cts` in this branch; this lint exists so a source + * file can never silently re-acquire a collectable name again. + * + * ## What "collectable" means + * + * Node's test runner (`node --test`, and by extension the remote runner + * that dispatches this repo's push-gate suite) collects any file whose path + * matches, by default: + * + * **\/*.test.?(c|m)js **\/*-test.?(c|m)js **\/*_test.?(c|m)js + * **\/test-*.?(c|m)js **\/test.?(c|m)js **\/test/** + * + * Node also resolves `.ts`/`.cts`/`.mts` through the same collector once + * type-stripping is active (unflagged since Node 23.6; this repo's + * `engines.node` floor is >=24) — which is exactly how a `.cts` file ended + * up collected in the incident above. This lint therefore checks each of + * the six extensions `js`, `cjs`, `mjs`, `ts`, `cts`, `mts` against each + * basename-shaped pattern, plus a path-based check for any file living + * inside a directory literally named `test` (not `tests` — this repo's own + * test directory is deliberately outside the scanned source dirs, see + * below). + * + * ## Scanned (source/shipped) directories + * + * src/, scripts/, hooks/, bin/, gsd-core/bin/ (excluding + * gsd-core/bin/lib/**, see below), eslint-rules/ + * + * ## Exempted + * + * - tests/ — files there are SUPPOSED to match; that is the point. + * - node_modules/, .git/ — never source we own. + * - gsd-core/bin/lib/** — build output generated from src/*.cts by + * `npm run build:lib` (tsc), and gitignored (verified: every file under + * it, including the incident's own post-fix + * `gsd-core/bin/lib/real-home-guard.cjs`, is listed in .gitignore). A + * generated file inherits its source's basename 1:1, so scanning it + * would double-report the exact same defect `src/` already caught — + * noise, not signal. `gsd-core/bin/shared/*.json` is data, not code, + * but is harmlessly included since it never matches a JS/TS extension. + * + * Exported pure(ish) function `checkSourceTestNameCollisions({ dirs, root })` + * so tests can drive it against synthetic fixture directories; also runnable + * as a CLI against the real tree + * (`node scripts/lint-source-test-name-collision.cjs`). + */ + +const fs = require('fs'); +const path = require('path'); +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); + +// The six extensions Node's test runner collects, per the incident: the +// documented `?(c|m)js` set (js, cjs, mjs) plus the TypeScript-loader +// equivalents (ts, cts, mts) that the same collector resolves once +// type-stripping is active (unflagged since Node 23.6; this repo's +// engines.node floor is >=24, per package.json). +const COLLECTED_EXTENSIONS = ['js', 'cjs', 'mjs', 'ts', 'cts', 'mts']; +const EXT_ALT = COLLECTED_EXTENSIONS.join('|'); + +// Basename-shaped patterns, translated 1:1 from Node's documented defaults: +// **/*.test.?(c|m)js **/*-test.?(c|m)js **/*_test.?(c|m)js +// **/test-*.?(c|m)js **/test.?(c|m)js +// (the sixth default, **/test/**, is a path-shaped check — see +// isUnderLiteralTestDir below, not a basename regex.) +const BASENAME_PATTERNS = [ + { name: '*.test.EXT', re: new RegExp(`\\.test\\.(?:${EXT_ALT})$`) }, + { name: '*-test.EXT', re: new RegExp(`-test\\.(?:${EXT_ALT})$`) }, + { name: '*_test.EXT', re: new RegExp(`_test\\.(?:${EXT_ALT})$`) }, + { name: 'test-*.EXT', re: new RegExp(`^test-.*\\.(?:${EXT_ALT})$`) }, + { name: 'test.EXT', re: new RegExp(`^test\\.(?:${EXT_ALT})$`) }, +]; + +/** + * Source/shipped directories this guard checks, relative to repo root. + * Confirmed against the repo layout: src/, scripts/, hooks/, bin/, + * gsd-core/bin/, eslint-rules/ all ship first-party source or shipped + * tooling; nothing else at the top level carries executable source outside + * tests/. + */ +const DEFAULT_SCAN_DIRS = ['src', 'scripts', 'hooks', 'bin', 'gsd-core/bin', 'eslint-rules']; + +// Directories to never descend into anywhere in the tree. +const ALWAYS_EXCLUDE_DIR_NAMES = new Set(['node_modules', '.git']); + +// Relative dir prefixes (POSIX-joined, relative to repo root) that are +// generated build output and must not be scanned — see the module doc for +// why gsd-core/bin/lib is excluded (it 1:1-inherits src/*.cts basenames, so +// scanning it double-reports the same defect src/ already catches). +const GENERATED_OUTPUT_PREFIXES = ['gsd-core/bin/lib']; + +function toPosix(p) { + return p.split(path.sep).join('/'); +} + +function isGeneratedOutput(relPath) { + const posixRel = toPosix(relPath); + return GENERATED_OUTPUT_PREFIXES.some( + (prefix) => posixRel === prefix || posixRel.startsWith(`${prefix}/`) + ); +} + +/** + * Node's **\/test/** default: any file living inside a directory literally + * named `test` (singular) anywhere in its path. Deliberately does NOT match + * `tests/` (plural) — this repo's real test directory is a sibling of the + * scanned source dirs, not nested inside one, and is never itself scanned. + */ +function isUnderLiteralTestDir(relPath) { + return toPosix(relPath).split('/').slice(0, -1).includes('test'); +} + +function matchingBasenamePatterns(basename) { + return BASENAME_PATTERNS.filter((p) => p.re.test(basename)).map((p) => p.name); +} + +function walk(dir, root, out) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch (err) { + // A scan dir that cannot be read must never be silently treated as + // "zero files, zero violations" — that would be a green check that + // guarded nothing (same class of bug as an empty registry elsewhere in + // this repo's lints). + out.unreadable.push({ dir: path.relative(root, dir) || dir, error: err.message }); + return; + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + const rel = path.relative(root, full); + if (entry.isDirectory()) { + if (ALWAYS_EXCLUDE_DIR_NAMES.has(entry.name)) continue; + if (isGeneratedOutput(rel)) continue; + walk(full, root, out); + } else if (entry.isFile()) { + out.scanned.push(rel); + } + } +} + +/** + * @param {{ dirs?: string[], root: string }} opts + * `dirs` — scan dirs relative to `root` (defaults to DEFAULT_SCAN_DIRS). + * `root` — the directory `dirs` are resolved against (repo root for the + * real CLI run; a synthetic fixture root in tests). + * @returns {{ ok: boolean, violations: Array<{file:string, patterns:string[], reason:string}>, scanned: string[] }} + */ +function checkSourceTestNameCollisions({ dirs = DEFAULT_SCAN_DIRS, root }) { + const violations = []; + const out = { scanned: [], unreadable: [] }; + + for (const dir of dirs) { + const full = path.join(root, dir); + if (!fs.existsSync(full)) continue; // a configured dir that doesn't exist is not this lint's problem + walk(full, root, out); + } + + if (out.unreadable.length > 0) { + return { + ok: false, + violations: out.unreadable.map((u) => ({ + file: u.dir, + patterns: [], + reason: `cannot read scan directory ${u.dir}: ${u.error} — a collision guard that cannot ` + + 'read its own input must fail, never silently report zero violations', + })), + scanned: out.scanned, + }; + } + + for (const rel of out.scanned) { + const basename = path.basename(rel); + const patterns = matchingBasenamePatterns(basename); + const underTestDir = isUnderLiteralTestDir(rel); + if (patterns.length === 0 && !underTestDir) continue; + + const matched = underTestDir ? [...patterns, '**/test/**'] : patterns; + violations.push({ + file: toPosix(rel), + patterns: matched, + reason: + `basename matches Node's test-collection pattern(s) [${matched.join(', ')}] — the remote ` + + 'test runner (the push gate) collects files by this convention and executes them AS ' + + 'tests, while GitHub CI (scripts/run-tests.cjs) globs only tests/**/*.test.cjs and never ' + + 'sees it; the failure then appears ONLY at the push gate (incident: src/test-home-guard.cts, ' + + 'control run on next tip 622f43353: 37199 passed / 1 failed, throw · src/test-home-guard.cts). ' + + 'Remedy: rename the file out of the pattern; do NOT add a runner-side exclusion.', + }); + } + + violations.sort((a, b) => a.file.localeCompare(b.file)); + + return { ok: violations.length === 0, violations, scanned: out.scanned }; +} + +module.exports = { + checkSourceTestNameCollisions, + COLLECTED_EXTENSIONS, + DEFAULT_SCAN_DIRS, + GENERATED_OUTPUT_PREFIXES, + isUnderLiteralTestDir, + matchingBasenamePatterns, +}; + +function main() { + const ROOT = path.join(__dirname, '..'); + const result = checkSourceTestNameCollisions({ root: ROOT }); + + if (!result.ok) { + process.stderr.write( + `lint-source-test-name-collision: ${result.violations.length} violation(s) among ${result.scanned.length} scanned file(s)\n\n` + ); + for (const v of result.violations) { + process.stderr.write(` ${v.file}\n ${v.reason}\n`); + } + throw new ExitError(1); + } + + console.log(`ok lint-source-test-name-collision: ${result.scanned.length} file(s) scanned, 0 violations`); +} + +if (require.main === module) runMain(main); diff --git a/scripts/select-docs-guards.cjs b/scripts/select-docs-guards.cjs new file mode 100644 index 000000000..364fb9ddc --- /dev/null +++ b/scripts/select-docs-guards.cjs @@ -0,0 +1,56 @@ +#!/usr/bin/env node +'use strict'; + +/** + * select-docs-guards.cjs — pure selector mapping a PR's changed docs/ paths + * to the subset of scripts/docs-guard-registry.cjs's DOCS_GUARD_TESTS that + * actually reads any of them (#3753 follow-up). + * + * Deliberately dependency-free: no fs, no git, no process. Callers (the + * docs-required.yml workflow, tests) are responsible for producing the + * changed-paths list and reading the registry; this module only implements + * the matching semantics so they are independently unit-testable. + * + * Pattern semantics (mirrors scripts/docs-guard-registry.cjs's header doc): + * - a plain path matches an exact changed path; + * - a trailing-slash path is a DIRECTORY PREFIX match — 'docs/adr/' + * matches 'docs/adr/README.md' but must NOT match 'docs/adrenaline.md' + * (a naive `startsWith('docs/adr')` without the trailing slash would + * wrongly match the latter; matching against the full prefix INCLUDING + * the trailing slash is what keeps this boundary correct); + * - the sentinel '*' matches any non-empty changedDocsPaths. + */ + +/** + * @param {string} changedPath - a single changed docs/ path (e.g. 'docs/AGENTS.md'). + * @param {string} pattern - one entry from a registry value array. + * @returns {boolean} + */ +function patternMatches(changedPath, pattern) { + if (pattern === '*') return true; + if (pattern.endsWith('/')) return changedPath.startsWith(pattern); + return changedPath === pattern; +} + +/** + * @param {string[]} changedDocsPaths - repo-relative paths under docs/ that + * changed in this PR. An empty array always yields an empty selection. + * @param {Record} registry - DOCS_GUARD_TESTS shape: test + * file -> array of patterns it reads. + * @returns {string[]} sorted, deduped list of selected test file paths. + */ +function selectDocsGuards(changedDocsPaths, registry) { + if (!Array.isArray(changedDocsPaths) || changedDocsPaths.length === 0) return []; + + const selected = new Set(); + for (const [testFile, patterns] of Object.entries(registry)) { + const hit = patterns.some((pattern) => + changedDocsPaths.some((changedPath) => patternMatches(changedPath, pattern)), + ); + if (hit) selected.add(testFile); + } + + return [...selected].sort(); +} + +module.exports = { selectDocsGuards, patternMatches }; diff --git a/src/install-engine.cts b/src/install-engine.cts index 36b2720db..b7fcbf5f1 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -32,7 +32,7 @@ import retiredArtifactCleanup = require('./retired-artifact-cleanup.cjs'); import { posixNormalize } from './shell-command-projection.cjs'; import { isPathConfined } from './external-descriptor-trust.cjs'; import { ensureCommonJsMarker } from './commonjs-marker.cjs'; -import testHomeGuard = require('./test-home-guard.cjs'); +import testHomeGuard = require('./real-home-guard.cjs'); // #2874 (ADR-58 cleanup phase): the injectable fs seam for the // installRuntimeArtifacts call tree. `installFs()` resolves to real // `node:fs` unless a call is wrapped in `withInstallFs(deps.fs, ...)` — diff --git a/src/test-home-guard.cts b/src/real-home-guard.cts similarity index 97% rename from src/test-home-guard.cts rename to src/real-home-guard.cts index 8aa5631d1..8fabeb233 100644 --- a/src/test-home-guard.cts +++ b/src/real-home-guard.cts @@ -3,6 +3,16 @@ /** * #3712 — refuse to let an in-process test run reach the developer's REAL home. * + * NAMED `real-home-guard`, NOT `test-home-guard`: this is a SOURCE module, and a + * source filename matching Node's `test-*` convention gets COLLECTED and EXECUTED + * as a test by the remote runner, which never imports it and throws immediately + * on load — proven against the unmodified `next` tip 622f43353 (linux-node24, + * 37199 passed / 1 failed, failure at `src/test-home-guard.cts`). Meanwhile + * `scripts/run-tests.cjs` only globs `tests/**\/*.test.cjs`, so GitHub CI never + * sees the failure and stays green while the remote push-gate runner goes red. + * Any name works except one matching `test-*`, `*-test`, `*_test`, `*.test.*`, + * or `test.*`. + * * A runtime kind may declare a global `home` override that is resolved from * `os.homedir()` rather than from the caller's `configDir` (today: codex's * skills kind, `home: ".agents"`, ADR-1239 / #2088 — Codex auto-discovers diff --git a/src/surface.cts b/src/surface.cts index 837a6d5f5..f4a1f50a9 100644 --- a/src/surface.cts +++ b/src/surface.cts @@ -35,10 +35,10 @@ import { platformWriteSync, posixNormalize } from './shell-command-projection.cj // eslint-disable-next-line @typescript-eslint/no-require-imports import installProfiles = require('./install-profiles.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -import testHomeGuard = require('./test-home-guard.cjs'); +import testHomeGuard = require('./real-home-guard.cjs'); /** - * #3712 test seam, mirroring the `Deps` shape in src/test-home-guard.cts. Declared + * #3712 test seam, mirroring the `Deps` shape in src/real-home-guard.cts. Declared * here rather than imported because that module uses `export =` on a value. */ type TestHomeGuardDeps = { diff --git a/tests/adr-parser.test.cjs b/tests/adr-parser.test.cjs index ea79d49ad..574b49524 100644 --- a/tests/adr-parser.test.cjs +++ b/tests/adr-parser.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/adr/NNNN.md below is a sourcePath metadata label passed to parseAdrMarkdown, never read from disk. const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); diff --git a/tests/adr-parser.unit.test.cjs b/tests/adr-parser.unit.test.cjs index ee05dde35..12f51ca26 100644 --- a/tests/adr-parser.unit.test.cjs +++ b/tests/adr-parser.unit.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/adr/NNNN.md below is a sourcePath metadata label passed to parseAdrMarkdown, never read from disk. 'use strict'; /** diff --git a/tests/agent-marker-documentation-guard.test.cjs b/tests/agent-marker-documentation-guard.test.cjs index 3710924ff..dfbf0c3dc 100644 --- a/tests/agent-marker-documentation-guard.test.cjs +++ b/tests/agent-marker-documentation-guard.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/reference/workflow-fragments.md is cited only in a comment as a prior example; this file reads agents/*.md, never docs/. 'use strict'; // allow-test-rule: source-text-is-the-product see #2995 — parses the literal text of shipped diff --git a/tests/antigravity-upgrades.test.cjs b/tests/antigravity-upgrades.test.cjs index bb9033423..8de414f67 100644 --- a/tests/antigravity-upgrades.test.cjs +++ b/tests/antigravity-upgrades.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/cli/... substrings are external antigravity.google URL citations in comments, not repo paths. 'use strict'; /** diff --git a/tests/capability-cli.test.cjs b/tests/capability-cli.test.cjs index 072e42d16..c783480e3 100644 --- a/tests/capability-cli.test.cjs +++ b/tests/capability-cli.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/reference/gsd-capability-command.md is cited only in a header comment as rationale; no docs/ path is ever read. 'use strict'; /** diff --git a/tests/check-ui-safety-gate.test.cjs b/tests/check-ui-safety-gate.test.cjs index 45d5d22a8..604ad6a52 100644 --- a/tests/check-ui-safety-gate.test.cjs +++ b/tests/check-ui-safety-gate.test.cjs @@ -298,7 +298,7 @@ const os = require('node:os'); const { cleanup } = require('./helpers.cjs'); const { runNode, runHook } = require('./helpers/process-seam.cjs'); const { toLegacyResult } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HELPER_PATH = path.join(__dirname, '..', 'bin', 'lib', 'ui-safety-gate.cjs'); const PLAN_PHASE_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'plan-phase.md'); @@ -578,11 +578,18 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep ].join('\n'); function runGateFrom(consumingDir, phaseSection) { + // Bash FAN-OUT: the snippet runs `git rev-parse`, a `for` loop probing + // multiple candidate paths, and `node` — the wrong class for + // `PROBE_TIMEOUT_MS` (a single short CLI probe). Same class as the + // observed CI failures in tests/quick-branching.test.cjs (PR #3787 run + // 32668773524) and tests/worktree-safety.test.cjs (`next` run + // 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for + // the class rationale. const result = runHook('-c', [GATE_SNIPPET], { interpreter: 'bash', cwd: consumingDir, env: { ...process.env, RUNTIME_DIR: REPO_ROOT, PHASE_SECTION: phaseSection }, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); return toLegacyResult(result); } @@ -622,12 +629,15 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep path.join(installedLibDir, 'ui-safety-gate.cjs') ); + // Same bash FAN-OUT class as runGateFrom above (git rev-parse + a + // candidate-path probe loop + node) — see HOOK_FANOUT_TIMEOUT_MS in + // ./helpers/timeouts.cjs. const res = toLegacyResult( runHook('-c', [GATE_SNIPPET], { interpreter: 'bash', cwd: consumingProject, env: { ...process.env, RUNTIME_DIR: fakeRuntime, PHASE_SECTION: 'Build the analytics dashboard' }, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }) ); assert.strictEqual(res.status, 0, `bash failed: ${res.stderr}`); diff --git a/tests/ci-docs-guard-registry.test.cjs b/tests/ci-docs-guard-registry.test.cjs new file mode 100644 index 000000000..b91d6341c --- /dev/null +++ b/tests/ci-docs-guard-registry.test.cjs @@ -0,0 +1,801 @@ +'use strict'; + +// docs-guard-exempt: this file only WRITES synthetic 'docs/...' fixtures into +// throwaway temp dirs (writeFileSync()) to exercise +// scripts/lint-docs-guard-registration.cjs's own reader-detection behavior — +// it never reads real shipped docs/ content itself. The fixture strings +// happen to contain readFileSync('docs/...')-shaped text, which trips this +// lint's own plain-text scan; that is expected of a lint's own test file. +// +// Coverage for the docs-guard lane (#3753): +// - scripts/docs-guard-registry.cjs — the sole registry of doc-reading test +// files the docs-guard lane must run; +// - a registration lint (scripts/lint-docs-guard-registration.cjs) that +// flags a test file which reads a docs/ path but is neither registered +// nor exempted; +// - the docs-guard lane's registry run lives inside the ALREADY-REQUIRED +// `docs-lint` job in .github/workflows/docs-required.yml, gated on a +// `docs_changed` step output, rather than in a dedicated +// `paths:`-filtered workflow. A `paths:`-filtered workflow never reports +// a check on a non-docs PR, so it can never be added to the required +// contexts in .github/rulesets/main-protection.json without hanging +// every non-docs PR forever — that was the fatal flaw in a prior version +// of this PR that stood up .github/workflows/docs-guards.yml as a +// separate workflow (deleted; see git history). +// +// #3753 follow-up: this file (net-new; no predecessor exists on origin/next) +// replaces an earlier, since-abandoned version of this PR that put the registry inside a `docs guards` RULE +// in scripts/ci-test-scope.cjs's RULES array, on the theory that classify()'s +// `!codeChanged` normalization made the RULE inert to the scope decision. +// That theory held for docs-ONLY diffs and broke for MIXED docs+code diffs, +// where codeChanged is true and the normalization never runs — every one of +// the registry's ~20 tests joined targeted_tests on EVERY mixed PR. Probed: +// `node scripts/ci-test-scope.cjs --files "docs/a.md src/semver.cts"` +// returned 25 targeted_tests with the RULE in place, vs. 3 on `origin/next`. +// The fix extracts the registry to its own module (scripts/docs-guard-registry.cjs) +// that classify() never reads at all, and scripts/ci-test-scope.cjs is +// reverted byte-for-byte to `origin/next`. This file carries only the new +// registry's own tests plus the regression pin in the "classify() is +// untouched" describe block below — there is no `docs guards` RULE for it +// to cover, on `origin/next` or anywhere else. +// +// #3753 follow-up 2 (registry shape change): DOCS_GUARD_TESTS changed from a +// flat array to a MAP (test file -> docs/ path patterns it reads), and +// scripts/select-docs-guards.cjs's pure selectDocsGuards() resolves a PR's +// changed docs/ paths to the narrow subset of guards that need to run — +// instead of the docs-lint job always running the entire registry. See the +// "docs-guard selector" describe block below for the selector's own coverage. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const fs = require('fs'); +const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + +const ROOT = path.join(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'ci-test-scope.cjs'); +const { + DOCS_GUARD_TESTS, + DOCS_GUARD_TEST_FILES, + RUN_TESTS_SUITES, + assertNoSuiteCollision, +} = require('../scripts/docs-guard-registry.cjs'); +const { selectDocsGuards } = require('../scripts/select-docs-guards.cjs'); +const { + checkDocsGuardRegistration, + checkExemptBaseline, + checkExemptFingerprints, + extractDocsPathReferences, + deriveDocsGuardRegistry, + EXEMPT_BASELINE_FILE, + EXEMPT_BASELINE_CONST, +} = require('../scripts/lint-docs-guard-registration.cjs'); +const { + DOCS_GUARD_EXEMPT_BASELINE, + DOCS_GUARD_EXEMPT_DOCS_PATHS, +} = require('../scripts/lint-docs-guard-registration.exempt-baseline.cjs'); +const { selectExplicitFiles, walkTestFiles } = require('../scripts/run-tests.cjs'); + +function scopeFor(files) { + const r = runNode([SCRIPT, '--files', files.join(' ')], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); + return JSON.parse(r.stdout); +} + +describe('docs-guard registry (scripts/docs-guard-registry.cjs)', () => { + test('every registry entry exists on disk', () => { + for (const f of DOCS_GUARD_TEST_FILES) { + assert.ok(fs.existsSync(path.join(ROOT, f)), `registry entry does not exist on disk: ${f}`); + } + }); + + // #10: a heuristic curation pass missed tests/ui-spec-inventory-provenance.test.cjs + // the day it broke `next` on dacae9273 — its membership in the registry is + // therefore pinned here BY NAME, not derived from any heuristic. + test('the guard that broke next on dacae9273 is in the registry', () => { + assert.ok( + DOCS_GUARD_TEST_FILES.includes('tests/ui-spec-inventory-provenance.test.cjs'), + 'tests/ui-spec-inventory-provenance.test.cjs must be pinned in the docs-guard registry by name', + ); + }); + + test('every registry VALUE is a non-empty array of strings', () => { + for (const [file, patterns] of Object.entries(DOCS_GUARD_TESTS)) { + assert.ok(Array.isArray(patterns) && patterns.length > 0, `${file}: registry value must be a non-empty array`); + for (const p of patterns) { + assert.strictEqual(typeof p, 'string', `${file}: every pattern must be a string, got ${JSON.stringify(p)}`); + } + } + }); +}); + +// Boundary coverage for the pure selector (limit-1 / limit / limit+1 style): +// exact match, dir-prefix match (including the prefix-confusion boundary a +// naive startsWith would get wrong), the '*' wildcard, and the empty-input +// edge. +describe('docs-guard selector (scripts/select-docs-guards.cjs)', () => { + const REGISTRY = { + 'tests/exact-reader.test.cjs': ['docs/AGENTS.md'], + 'tests/dir-reader.test.cjs': ['docs/adr/'], + 'tests/wildcard-reader.test.cjs': ['*'], + }; + + test('exact-file match selects only that guard', () => { + assert.deepStrictEqual( + selectDocsGuards(['docs/AGENTS.md'], REGISTRY), + ['tests/exact-reader.test.cjs', 'tests/wildcard-reader.test.cjs'].sort(), + ); + }); + + test('dir-prefix match selects on a nested file', () => { + const selected = selectDocsGuards(['docs/adr/0001-example.md'], REGISTRY); + assert.ok(selected.includes('tests/dir-reader.test.cjs'), `expected dir-reader selected, got ${JSON.stringify(selected)}`); + }); + + // Prefix-confusion boundary: 'docs/adrenaline.md' shares the literal + // prefix 'docs/adr' with the pattern 'docs/adr/', but is NOT under the + // docs/adr/ directory. A naive `startsWith('docs/adr')` (without the + // trailing slash) would wrongly match this. + test('dir-prefix match does NOT select on a same-prefix sibling file (docs/adrenaline.md)', () => { + const selected = selectDocsGuards(['docs/adrenaline.md'], REGISTRY); + assert.ok(!selected.includes('tests/dir-reader.test.cjs'), `expected dir-reader NOT selected, got ${JSON.stringify(selected)}`); + }); + + test("'*' guard is selected on any docs change", () => { + const selected = selectDocsGuards(['docs/some-unrelated-file.md'], REGISTRY); + assert.deepStrictEqual(selected, ['tests/wildcard-reader.test.cjs']); + }); + + test('empty changed set yields an empty selection', () => { + assert.deepStrictEqual(selectDocsGuards([], REGISTRY), []); + }); + + test('a changed docs file no guard reads yields an empty selection (minus the wildcard)', () => { + const registryNoWildcard = { 'tests/exact-reader.test.cjs': ['docs/AGENTS.md'] }; + assert.deepStrictEqual(selectDocsGuards(['docs/totally-unrelated.md'], registryNoWildcard), []); + }); + + // Proven-genuine cases (real registry, real files) pinned by name. + test('changing docs/COMMANDS.md selects tests/cursor-reviewer.test.cjs', () => { + const selected = selectDocsGuards(['docs/COMMANDS.md'], DOCS_GUARD_TESTS); + assert.ok(selected.includes('tests/cursor-reviewer.test.cjs'), `got: ${JSON.stringify(selected)}`); + }); + + test('changing docs/INVENTORY.md selects tests/inventory-headings-countfree.test.cjs', () => { + const selected = selectDocsGuards(['docs/INVENTORY.md'], DOCS_GUARD_TESTS); + assert.ok(selected.includes('tests/inventory-headings-countfree.test.cjs'), `got: ${JSON.stringify(selected)}`); + }); + + test('changing docs/AGENTS.md selects tests/install.test.cjs', () => { + const selected = selectDocsGuards(['docs/AGENTS.md'], DOCS_GUARD_TESTS); + assert.ok(selected.includes('tests/install.test.cjs'), `got: ${JSON.stringify(selected)}`); + }); + + // The whole point of the maintainer's decision: an unrelated docs change + // must NOT select tests/install.test.cjs (7840 lines) just because that + // file happens to also read docs/AGENTS.md. + test('changing an unrelated docs file does NOT select tests/install.test.cjs', () => { + const selected = selectDocsGuards(['docs/how-to/some-unrelated-guide.md'], DOCS_GUARD_TESTS); + assert.ok(!selected.includes('tests/install.test.cjs'), `got: ${JSON.stringify(selected)}`); + }); + + test('a real single-file docs change selects far fewer than the full 62-file registry', () => { + const selected = selectDocsGuards(['docs/how-to/foo.md'], DOCS_GUARD_TESTS); + assert.ok( + selected.length < DOCS_GUARD_TEST_FILES.length, + `expected a narrower selection than the full registry (${DOCS_GUARD_TEST_FILES.length}), got ${selected.length}`, + ); + }); +}); + +describe('docs-guard lane: classify() is untouched (#3753 mixed-diff regression)', () => { + // THE regression pin for this PR's blocker. Before the extraction, a `docs + // guards` RULE lived inside scripts/ci-test-scope.cjs's RULES array and + // fired on any 'docs/' path regardless of what else changed. Because the + // `!codeChanged` normalization only zeroes output when NO product/pipeline + // file changed, a mixed docs+code diff kept every one of the RULE's ~20 + // tests in targeted_tests. Measured on that version: + // node scripts/ci-test-scope.cjs --files "docs/a.md src/semver.cts" + // -> 25 targeted_tests (HEAD with the RULE) vs. 3 (origin/next, no RULE). + // scripts/ci-test-scope.cjs is now reverted byte-for-byte to origin/next, + // so this pins the count the 'TS runtime sources' RULE alone produces for a + // src/*.cts change, proving the docs-guard registry does not leak into the + // scoped lane via any path. Asserted BEHAVIORALLY rather than by diffing the + // file against origin/next: a ref-diff assertion is unavailable in a shallow + // CI checkout and in gsd-test's shallow clone, so it could only be written to + // skip when the ref is missing -- i.e. to pass vacuously wherever it actually + // runs. The OUTPUT is the contract; the bytes are not. + test('a mixed docs+code diff selects the same tests as before the docs-guard extraction', () => { + const result = scopeFor(['docs/a.md', 'src/semver.cts']); + assert.strictEqual(result.code_changed, true); + assert.deepStrictEqual( + result.targeted_tests, + [ + 'tests/emitted-attribution.test.cjs', + 'tests/emitted-provenance.test.cjs', + 'tests/semver-compare.test.cjs', + ], + `expected exactly the 'TS runtime sources' RULE's 3 tests (matching origin/next), not the ` + + `pre-fix 25 that resulted from the docs-guard registry leaking into targeted_tests on a ` + + `mixed docs+code diff: ${JSON.stringify(result.targeted_tests)}`, + ); + }); +}); + +describe('docs-guard lane: lint-docs-guard-registration.cjs', () => { + function withFixture(fn) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'docs-guard-lint-')); + try { + return fn(dir); + } finally { + cleanup(dir); + } + } + + test('a registered docs reader passes the registration lint', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'reader.test.cjs'), + "'use strict';\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: ['reader.test.cjs'] }); + assert.deepStrictEqual(result.violations, []); + assert.strictEqual(result.ok, true); + }); + }); + + test('an unregistered docs reader fails the lint', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'unregistered.test.cjs'), + "'use strict';\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'unregistered.test.cjs'), + `expected a violation naming unregistered.test.cjs, got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + test('an exempted docs reader passes the lint', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'exempt.test.cjs'), + "'use strict';\n// docs-guard-exempt: reads docs only to build an overlay\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.deepStrictEqual(result.violations, []); + assert.strictEqual(result.ok, true); + }); + }); + + test('an exemption without a reason is rejected', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'bare-exempt.test.cjs'), + "'use strict';\n// docs-guard-exempt:\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'bare-exempt.test.cjs'), + `expected a violation naming bare-exempt.test.cjs, got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + test('a registry entry pointing at a missing file fails the lint', () => { + withFixture(dir => { + const result = checkDocsGuardRegistration({ testsDir: dir, registry: ['does-not-exist.test.cjs'] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'does-not-exist.test.cjs'), + `expected a violation naming does-not-exist.test.cjs, got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + test('a test that only mentions a docs path is not treated as a reader', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'mentions-only.test.cjs'), + "'use strict';\nconst assert = require('node:assert/strict');\nconst msg = 'see docs/foo.md';\nassert.equal(msg, 'see docs/foo.md');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.deepStrictEqual(result.violations, []); + assert.strictEqual(result.ok, true); + }); + }); + + // #3753 follow-up: the shipped lint's original reader-detection caught the + // SEGMENT spelling (path.join(ROOT, 'docs', 'x.md')) but not the + // SINGLE-STRING spelling (readShipped('docs/how-to/x.md')) — exactly how + // tests/ui-spec-inventory-provenance.test.cjs reads, the guard that broke + // `next` on dacae9273. This is the regression pin: it must fail against the + // pre-fix lint and pass once the name-shaped reader-call detector exists. + test('the lint detects a docs path passed as a single string argument', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'single-string-reader.test.cjs'), + "'use strict';\nfunction readShipped(p) { return require('fs').readFileSync(p, 'utf8'); }\nreadShipped('docs/how-to/x.md');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'single-string-reader.test.cjs'), + `expected a violation naming single-string-reader.test.cjs, got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + // Over-correction guard: a naive "flag any 'docs/...' string literal" rule + // would trip on a docs path that only appears inside an assertion message, + // never passed to anything read-shaped. The name-shaped heuristic must not + // regress the existing mention-only exemption. + test('the lint still ignores a docs path that is only mentioned in a message', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'still-mentions-only.test.cjs'), + "'use strict';\nconst assert = require('node:assert/strict');\nconst msg = 'see docs/foo.md';\nassert.equal(msg, 'see docs/foo.md');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.deepStrictEqual(result.violations, []); + assert.strictEqual(result.ok, true); + }); + }); + + // Exemption-marker header-window regression pin (this PR's second + // reviewer finding): findExemption used to scan the WHOLE file, so a + // `// docs-guard-exempt:` string appearing inside a fixture/template + // literal anywhere in the file exempted the real file it lives in. This + // asserts the marker is only honored near the top (within the header + // window), not when it appears far down the file body. + test('a docs-guard-exempt marker deep in the file body (outside the header window) does not exempt it', () => { + withFixture(dir => { + const padding = Array.from({ length: 30 }, (_, i) => `// padding line ${i}`).join('\n'); + fs.writeFileSync( + path.join(dir, 'late-marker.test.cjs'), + `'use strict';\n${padding}\n// docs-guard-exempt: this should NOT count, it is not a header\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n`, + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'late-marker.test.cjs'), + `expected a violation naming late-marker.test.cjs (marker outside header window must not exempt), got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + // Security follow-up FIX 2: a guard that cannot read its own input must + // never report success. Pre-fix, checkDocsGuardRegistration({testsDir: + // '/nonexistent', registry: []}) returned {ok:true, violations:[]}. + test('an unreadable testsDir is a hard violation, not a silent pass', () => { + const result = checkDocsGuardRegistration({ testsDir: '/nonexistent-docs-guard-testsdir', registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => /cannot read testsDir/.test(v.reason)), + `expected a violation naming the unreadable testsDir, got: ${JSON.stringify(result.violations)}`, + ); + }); + + // Same class: a directory (or broken symlink) named `*.test.cjs` must fail + // the lint rather than being silently skipped by the readFileSync catch. + test('an unreadable candidate test file (a directory named *.test.cjs) is a hard violation', () => { + withFixture(dir => { + fs.mkdirSync(path.join(dir, 'a-directory.test.cjs')); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'a-directory.test.cjs' && /cannot read candidate test file/.test(v.reason)), + `expected a violation naming a-directory.test.cjs, got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + // Security follow-up FIX 4: a marker line inside a multi-line template + // literal in the header window (e.g. `const F = \`...\`;`) must NOT be + // honored as a real comment. Probed pre-fix: exempted=true. + test('a docs-guard-exempt marker inside a multi-line template literal does NOT exempt the file', () => { + withFixture(dir => { + fs.writeFileSync( + path.join(dir, 'template-literal-marker.test.cjs'), + "'use strict';\nconst F = `\n// docs-guard-exempt: this is fixture content, not a real comment\n`;\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ testsDir: dir, registry: [] }); + assert.strictEqual(result.ok, false); + assert.ok( + result.violations.some(v => v.file === 'template-literal-marker.test.cjs'), + `expected a violation naming template-literal-marker.test.cjs (marker inside a template literal must not exempt), got: ${JSON.stringify(result.violations)}`, + ); + }); + }); + + test('the lint registry and the docs-guard-registry module export the same list', () => { + const moduleRegistry = DOCS_GUARD_TEST_FILES.map(f => path.basename(f)); + const lintRegistry = deriveDocsGuardRegistry(); + assert.deepStrictEqual(lintRegistry, moduleRegistry, + 'the lint\'s default registry must be exactly scripts/docs-guard-registry.cjs\'s ' + + 'DOCS_GUARD_TEST_FILES export — a second, independently maintained list is the #3753 defect ' + + 'class this pins against'); + }); + + test('the repository currently satisfies the registration lint (including the exempt baseline ratchet)', () => { + const result = checkDocsGuardRegistration({ + testsDir: path.join(ROOT, 'tests'), + registry: DOCS_GUARD_TEST_FILES.map(f => path.basename(f)), + exemptBaseline: DOCS_GUARD_EXEMPT_BASELINE, + exemptDocsPathsBaseline: DOCS_GUARD_EXEMPT_DOCS_PATHS, + }); + assert.strictEqual(result.ok, true, + `expected the real tests/ tree to satisfy the docs-guard registration lint, ` + + `got ${result.violations.length} violation(s): ${JSON.stringify(result.violations)}`); + }); +}); + +// #3753 security follow-up FIX 3: the exempt ratchet gated on file IDENTITY +// only — a baselined file that later STARTS genuinely reading shipped docs/ +// content stayed exempt with zero signal. Probed pre-fix: a baselined file +// doing fs.readFileSync('docs/foo.md') still reported ok=true, violations=[]. +describe('docs-guard lane: docs-guard-exempt content-aware fingerprint ratchet (#3753 FIX 3)', () => { + test('an unchanged docs-path fingerprint passes', () => { + const violations = checkExemptFingerprints( + { 'a.test.cjs': ['docs/foo.md'] }, + { 'a.test.cjs': ['docs/foo.md'] }, + ); + assert.deepStrictEqual(violations, []); + }); + + test('an ADDED docs path fails', () => { + const violations = checkExemptFingerprints( + { 'a.test.cjs': ['docs/foo.md', 'docs/bar.md'] }, + { 'a.test.cjs': ['docs/foo.md'] }, + ); + assert.ok(violations.length > 0, 'expected a violation for an added docs path'); + assert.ok( + violations.some(v => v.file === 'a.test.cjs' && /docs paths referenced by a\.test\.cjs changed/.test(v.reason) && /re-confirm the exemption still holds/.test(v.reason)), + `expected an actionable re-confirm message, got: ${JSON.stringify(violations)}`, + ); + }); + + test('a REMOVED docs path fails', () => { + const violations = checkExemptFingerprints( + { 'a.test.cjs': ['docs/foo.md'] }, + { 'a.test.cjs': ['docs/foo.md', 'docs/bar.md'] }, + ); + assert.ok(violations.length > 0, 'expected a violation for a removed docs path'); + assert.ok( + violations.some(v => v.file === 'a.test.cjs' && /re-confirm the exemption still holds/.test(v.reason)), + `expected an actionable re-confirm message, got: ${JSON.stringify(violations)}`, + ); + }); + + test('a file with no baseline fingerprint yet is not flagged here (identity ratchet handles novelty)', () => { + const violations = checkExemptFingerprints({ 'novel.test.cjs': ['docs/foo.md'] }, {}); + assert.deepStrictEqual(violations, []); + }); + + test('end-to-end: a baselined exempt file that starts genuinely reading a NEW docs/ path fails the lint', () => { + withFixtureForExemptBaseline((dir) => { + fs.writeFileSync( + path.join(dir, 'novel-exempt.test.cjs'), + "'use strict';\n// docs-guard-exempt: reads docs only to build an overlay\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + const result = checkDocsGuardRegistration({ + testsDir: dir, + registry: [], + exemptBaseline: ['novel-exempt.test.cjs'], + exemptDocsPathsBaseline: { 'novel-exempt.test.cjs': ['docs/foo.md'] }, + }); + assert.strictEqual(result.ok, true, `expected unchanged fingerprint to pass, got: ${JSON.stringify(result.violations)}`); + + fs.writeFileSync( + path.join(dir, 'novel-exempt.test.cjs'), + "'use strict';\n// docs-guard-exempt: reads docs only to build an overlay\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\nfs.readFileSync('docs/bar.md', 'utf8');\n", + ); + const drifted = checkDocsGuardRegistration({ + testsDir: dir, + registry: [], + exemptBaseline: ['novel-exempt.test.cjs'], + exemptDocsPathsBaseline: { 'novel-exempt.test.cjs': ['docs/foo.md'] }, + }); + assert.strictEqual(drifted.ok, false, 'expected a NEW docs/ path reference to fail the lint'); + assert.ok( + drifted.violations.some(v => v.file === 'novel-exempt.test.cjs' && /re-confirm the exemption still holds/.test(v.reason)), + `expected a fingerprint-drift violation, got: ${JSON.stringify(drifted.violations)}`, + ); + }); + }); + + test('the shipped docs-paths baseline exactly matches the extracted fingerprint for every baselined file', () => { + for (const file of DOCS_GUARD_EXEMPT_BASELINE) { + const content = fs.readFileSync(path.join(ROOT, 'tests', file), 'utf8'); + const live = extractDocsPathReferences(content); + assert.deepStrictEqual( + live, + DOCS_GUARD_EXEMPT_DOCS_PATHS[file] || [], + `docs-paths fingerprint for ${file} is stale — regenerate DOCS_GUARD_EXEMPT_DOCS_PATHS`, + ); + } + }); +}); + +// #3753 security follow-up FIX 2: `// docs-guard-exempt:` had no ratchet — any +// non-empty reason permanently opted a file out with no cap and no review +// signal. Mirrors scripts/lint-allow-test-rule-refs.cjs's identity ratchet +// (scripts/lib/allowlist-ratchet.cjs). +describe('docs-guard lane: docs-guard-exempt baseline ratchet (#3753 FIX 2)', () => { + test('an un-baselined exemption fails', () => { + const violations = checkExemptBaseline(['newly-exempted.test.cjs'], ['already-known.test.cjs']); + assert.ok(violations.length > 0, 'expected at least one violation for a novel exemption'); + assert.ok( + violations.some(v => /newly-exempted\.test\.cjs/.test(v.reason) && /add it to the baseline/.test(v.reason)), + `expected a remedy naming the new file and pointing at the baseline, got: ${JSON.stringify(violations)}`, + ); + assert.ok( + violations.some(v => v.reason.includes(EXEMPT_BASELINE_FILE) && v.reason.includes(EXEMPT_BASELINE_CONST)), + `expected the remedy to cite ${EXEMPT_BASELINE_FILE}:${EXEMPT_BASELINE_CONST}, got: ${JSON.stringify(violations)}`, + ); + }); + + test('a baselined exemption passes', () => { + const violations = checkExemptBaseline(['already-known.test.cjs'], ['already-known.test.cjs']); + assert.deepStrictEqual(violations, []); + }); + + test('a baseline entry whose file no longer carries the marker (or was removed) is reported stale', () => { + const violations = checkExemptBaseline([], ['removed-file.test.cjs']); + assert.ok(violations.length > 0, 'expected a stale-entry violation'); + assert.ok( + violations.some(v => /removed-file\.test\.cjs/.test(v.reason) && /prune/i.test(v.reason)), + `expected a prune-the-stale-entry message naming removed-file.test.cjs, got: ${JSON.stringify(violations)}`, + ); + }); + + test('end-to-end via checkDocsGuardRegistration: an un-baselined marker fails, a baselined one passes', () => { + withFixtureForExemptBaseline((dir) => { + fs.writeFileSync( + path.join(dir, 'novel-exempt.test.cjs'), + "'use strict';\n// docs-guard-exempt: reads docs only to build an overlay\nconst fs = require('fs');\nfs.readFileSync('docs/foo.md', 'utf8');\n", + ); + + const failing = checkDocsGuardRegistration({ testsDir: dir, registry: [], exemptBaseline: [] }); + assert.strictEqual(failing.ok, false, 'expected an un-baselined docs-guard-exempt marker to fail the lint'); + assert.ok( + failing.violations.some(v => v.reason.includes('novel-exempt.test.cjs')), + `expected a violation naming novel-exempt.test.cjs, got: ${JSON.stringify(failing.violations)}`, + ); + + const passing = checkDocsGuardRegistration({ + testsDir: dir, + registry: [], + exemptBaseline: ['novel-exempt.test.cjs'], + }); + assert.strictEqual(passing.ok, true, + `expected a baselined docs-guard-exempt marker to pass, got: ${JSON.stringify(passing.violations)}`); + }); + }); + + test('the shipped baseline exactly matches every real docs-guard-exempt marker in tests/', () => { + const result = checkDocsGuardRegistration({ + testsDir: path.join(ROOT, 'tests'), + registry: DOCS_GUARD_TEST_FILES.map(f => path.basename(f)), + }); + assert.deepStrictEqual( + [...result.exemptedFiles].sort(), + [...DOCS_GUARD_EXEMPT_BASELINE].sort(), + 'scripts/lint-docs-guard-registration.exempt-baseline.cjs must exactly list every real ' + + 'docs-guard-exempt marker currently in tests/ — derived, not retyped from memory', + ); + }); +}); + +function withFixtureForExemptBaseline(fn) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'docs-guard-exempt-baseline-')); + try { + return fn(dir); + } finally { + cleanup(dir); + } +} + +// #3753 security follow-up FIX 3: a registry entry equal to a run-tests.cjs +// SUITES token (e.g. a typo'd 'all') would be silently treated by +// selectExplicitFiles (scripts/run-tests.cjs:651) as a suite selector, not a +// filename, running the ENTIRE suite inside the required docs-lint job. +describe('docs-guard lane: registry entries cannot collide with a SUITES token (#3753 FIX 3)', () => { + test('assertNoSuiteCollision rejects a registry containing a SUITES token', () => { + assert.throws( + () => assertNoSuiteCollision(['all']), + /collides with a run-tests\.cjs SUITES token/, + ); + }); + + test('assertNoSuiteCollision accepts the real, current DOCS_GUARD_TEST_FILES', () => { + assert.doesNotThrow(() => assertNoSuiteCollision(DOCS_GUARD_TEST_FILES)); + }); + + // Security follow-up FIX 1: every real registry key carries the `tests/` + // prefix by convention, so the realistic typo is 'tests/all', not bare + // 'all'. Pre-fix, assertNoSuiteCollision compared RAW keys against + // RUN_TESTS_SUITES and missed this entirely, while run-tests.cjs's own + // splitFileList strips `tests/` before its SUITES check — letting + // selectExplicitFiles silently run the ENTIRE suite (824 files) for a + // 'tests/all' entry inside the required docs-lint job. + test('assertNoSuiteCollision rejects the realistic tests/-prefixed typo (tests/all)', () => { + assert.throws( + () => assertNoSuiteCollision(['tests/all']), + /collides with a run-tests\.cjs SUITES token/, + ); + }); + + // Same collision spelled with a Windows backslash separator, mirroring + // run-tests.cjs's own `\`->`/` normalization in splitFileList. + test('assertNoSuiteCollision rejects a backslash-spelled tests\\all typo', () => { + assert.throws( + () => assertNoSuiteCollision(['tests\\all']), + /collides with a run-tests\.cjs SUITES token/, + ); + }); + + test('deriveDocsGuardRegistry (the lint\'s consumer) also rejects a SUITES-colliding registry', () => { + // deriveDocsGuardRegistry always reads the real DOCS_GUARD_TEST_FILES + // module export, so this drives the same assertNoSuiteCollision call it + // makes internally, directly against a synthetic colliding list — + // proving the lint's own derivation path (not just the registry module) + // enforces it. + assert.throws(() => assertNoSuiteCollision(['unit', ...DOCS_GUARD_TEST_FILES]), /unit/); + }); + + // Parity test (this repo's documented generative-fix-divergence rule): + // RUN_TESTS_SUITES in scripts/docs-guard-registry.cjs is a hand-maintained + // duplicate of scripts/run-tests.cjs:50's SUITES (not exported there, and + // run-tests.cjs's behavior is deliberately out of scope to change for this + // fix). Verified BEHAVIORALLY, never by re-reading run-tests.cjs's source + // text: for every token in the duplicate, run-tests.cjs's own exported + // selectExplicitFiles must treat it as a suite selector (never "not + // found"); for a control non-member it must NOT. + test('RUN_TESTS_SUITES stays behaviorally in sync with run-tests.cjs\'s real SUITES', () => { + const allFiles = walkTestFiles(path.join(ROOT, 'tests')); + for (const suite of RUN_TESTS_SUITES) { + const result = selectExplicitFiles(allFiles, suite, null); + assert.ok( + !result.error, + `expected run-tests.cjs to treat "${suite}" as a suite selector, but selectExplicitFiles ` + + `errored: ${result.error} — RUN_TESTS_SUITES has drifted from the real SUITES`, + ); + } + + const control = '__not_a_real_suite_or_file__'; + const controlResult = selectExplicitFiles(allFiles, control, null); + assert.ok( + controlResult.error && /not found/.test(controlResult.error), + 'control non-member unexpectedly resolved as a suite or file — the parity check itself is not discriminating', + ); + }); +}); + +describe('docs-guard lane: the workflow', () => { + test('the docs-required workflow derives from the registry module and runs the registered set, gated on docs_changed, without a paths filter', () => { + const workflowPath = path.join(ROOT, '.github', 'workflows', 'docs-required.yml'); + assert.ok(fs.existsSync(workflowPath), `expected ${workflowPath} to exist`); + + const text = fs.readFileSync(workflowPath, 'utf8'); + + // Deliberately NO `paths:` filter: this workflow must always report a + // status so it can supply the already-required `docs-lint` context + // (.github/rulesets/main-protection.json). A `paths:`-filtered workflow + // never reports on a non-docs PR and therefore can never be made + // required without hanging every non-docs PR forever. + assert.doesNotMatch(text, /^\s*paths:/m, 'expected NO `paths:` filter — this workflow must always report a status to be a valid required context'); + + // The required-context job id must survive; renaming it would silently + // un-require the whole gate. + assert.match(text, /^\s*docs-lint:/m, 'expected the `docs-lint` job id to still be present (it is the required-context name)'); + + assert.match(text, /docs-guard-registry\.cjs/, 'expected the workflow to derive its list from scripts/docs-guard-registry.cjs'); + assert.match(text, /run-tests\.cjs/, 'expected a run step invoking run-tests.cjs'); + assert.match(text, /--files-from/, 'expected the run step to use --files-from'); + + // The registry run must be gated on the same docs_changed detection this + // job already computes, so it does not run (and cannot fail) on + // non-docs PRs. + assert.match( + text, + /steps\.docs-changed\.outputs\.docs_changed == 'true'/, + 'expected the docs-guard registry steps to be gated on steps.docs-changed.outputs.docs_changed', + ); + }); + + // Second reviewer finding: the derivation step used to throw only when the + // rule/module was absent, never asserting the derived list was NON-EMPTY. + // Probed: `run-tests.cjs --files-from ` prints + // 'run-tests: no tests in suite "all"' and exits 0 — an emptied registry + // would yield a GREEN check having run zero tests, precisely the failure + // mode #3753 exists to close. + test('the workflow fails loudly if the derived registry file ends up empty', () => { + const workflowPath = path.join(ROOT, '.github', 'workflows', 'docs-required.yml'); + const text = fs.readFileSync(workflowPath, 'utf8'); + assert.match( + text, + /-s\s+\.docs-guard-tests\.txt/, + 'expected the workflow to check the derived file is non-empty (e.g. `[ -s .docs-guard-tests.txt ]`) ' + + 'before running run-tests.cjs against it', + ); + }); + + // Security follow-up FIX 5: a fork PR could FORCE-COMMIT + // `.docs-guard-tests.txt` (gitignored files are still addable with + // `git add -f`), which would satisfy `hashFiles('.docs-guard-tests.txt') + // != ''` and run an attacker-chosen test list. The fix rm -f's any + // committed copy at the start of selection, and gates the run step on an + // explicit step OUTPUT the selection step itself sets — never on hashFiles. + test('the selection step destroys any pre-existing copy of its output files before regenerating them', () => { + const workflowPath = path.join(ROOT, '.github', 'workflows', 'docs-required.yml'); + const text = fs.readFileSync(workflowPath, 'utf8'); + assert.match( + text, + /rm -f \.docs-guard-tests\.txt \.docs-changed-paths\.txt/, + 'expected the selection step to `rm -f` both derived files before regenerating them, so a ' + + 'force-committed copy cannot survive into the run', + ); + }); + + test('the run step is gated on an explicit step output, never on hashFiles', () => { + const workflowPath = path.join(ROOT, '.github', 'workflows', 'docs-required.yml'); + const text = fs.readFileSync(workflowPath, 'utf8'); + + // Assert the real gate property (parsed `if:` expressions), not a + // raw-text scan: `hashFiles(` legitimately appears in COMMENTS explaining + // why hashFiles was rejected, and a source-text ban on the substring + // fails on those comments even though no step is actually gated on it. + const ifExpressions = splitLines(text) + .map(line => line.trim()) + .filter(line => line.startsWith('if:')); + assert.ok(ifExpressions.length > 0, 'expected at least one `if:` expression in the workflow'); + for (const expr of ifExpressions) { + assert.doesNotMatch( + expr, + /hashFiles\(/, + 'expected NO step to be gated (via `if:`) on hashFiles(...) — a force-committed ' + + `.docs-guard-tests.txt would satisfy hashFiles() != '' and run an attacker-chosen test list. Offending expression: ${expr}`, + ); + } + + assert.match( + text, + /GITHUB_OUTPUT.*selected=true/, + 'expected the selection step to set an explicit selected=true step output only on the ' + + 'code path that legitimately wrote .docs-guard-tests.txt', + ); + + const runStepIfExpression = ifExpressions.find(expr => expr.includes('select-docs-guards.outputs.selected')); + assert.ok( + runStepIfExpression, + `expected an if: expression gating the run step on steps.select-docs-guards.outputs.selected, ` + + `found: ${JSON.stringify(ifExpressions)}`, + ); + assert.match( + runStepIfExpression, + /steps\.select-docs-guards\.outputs\.selected == 'true'/, + 'expected the run step to be gated on steps.select-docs-guards.outputs.selected, not on hashFiles', + ); + }); + + // Guard-can-fail proof: a hypothetical `if: hashFiles('x') != ''` line must + // trip the ifExpressions scan above. Exercised directly against the parsing + // logic (not by mutating the shipped workflow) so this stays a fast, + // deterministic unit check. + test('the if: hashFiles scan can actually fail (guard is not vacuous)', () => { + const hypothetical = " run:\n if: hashFiles('.docs-guard-tests.txt') != ''\n"; + const ifExpressions = splitLines(hypothetical) + .map(line => line.trim()) + .filter(line => line.startsWith('if:')); + assert.ok(ifExpressions.some(expr => /hashFiles\(/.test(expr)), 'expected the hypothetical hashFiles gate to be detected by the scan'); + }); +}); diff --git a/tests/ci-test-job-timeout-budget.test.cjs b/tests/ci-test-job-timeout-budget.test.cjs index cedce1415..76f75ba01 100644 --- a/tests/ci-test-job-timeout-budget.test.cjs +++ b/tests/ci-test-job-timeout-budget.test.cjs @@ -75,11 +75,14 @@ const LANE_COSTS = [ }, { job: 'test-full', - measuredMinutes: 19, - // Worst observed shard is `full test (windows-latest, 22, shard 3/3)`: - // 18m59s on 05b170e44 and 18m14s on 81eeb8a53. The Windows shards are slow - // for platform reasons, not extra work. - evidence: 'run 30650559192 — 18m59s, windows-22 shard 3/3', + measuredMinutes: 27, + // Lane moved from windows-22 to windows-latest/24 and is now sharded three + // ways. Worst observed shard is `full test (windows-latest, 24, shard + // 3/3)`: 26m18s on run 32614439702 (shard 2/3 23m36s, shard 1/3 19m22s), + // and 23m17s for shard 2/3 on run 32603886007. The previous 18m59s / + // windows-22 figure recorded here predated this cost and is stale — the + // lane is measurably slower now, not merely relabeled. + evidence: 'run 32614439702 — 26m18s, windows-latest/24 shard 3/3', }, { job: 'coverage-gate', diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index 8dd60f09c..98652a7a5 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/...' strings are synthetic changed-file inputs fed to scopeFor()/classify(); the docs/ literal is never read as file content. 'use strict'; const { describe, test } = require('node:test'); diff --git a/tests/cline-install.test.cjs b/tests/cline-install.test.cjs index f0f0f51bc..5f0037085 100644 --- a/tests/cline-install.test.cjs +++ b/tests/cline-install.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/guide.md' is a synthetic tool_input fixture path for a hook probe, not a real repo doc. // allow-test-rule: source-text-is-the-product // Workflow .md / agent .md / command .md / reference .md files — their text // IS what the runtime loads. Testing text content tests the deployed contract. diff --git a/tests/close-phase-todos-padded-resolves.test.cjs b/tests/close-phase-todos-padded-resolves.test.cjs index b1d3ed445..106f0d6d1 100644 --- a/tests/close-phase-todos-padded-resolves.test.cjs +++ b/tests/close-phase-todos-padded-resolves.test.cjs @@ -13,7 +13,7 @@ const path = require('node:path'); const { cleanup } = require('./helpers.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); const { throwIfFailed } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const EXECUTE_PHASE = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); @@ -118,7 +118,14 @@ describe('#2576: close_phase_todos normalizes padded vs unpadded resolves_phase const script = path.join(tmp, 'normalize.sh'); // argv array (no shell string) so a quoted input like '"05"' is passed verbatim. fs.writeFileSync(script, `${helper}\nnormalize_phase_num "$1"\n`); - const result = runHook(script, [input], { interpreter: 'bash', timeoutMs: PROBE_TIMEOUT_MS }); + // Bash FAN-OUT: `normalize_phase_num` pipes through `sed`, not a + // self-contained bash builtin — the wrong class for `PROBE_TIMEOUT_MS`. + // Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. + const result = runHook(script, [input], { interpreter: 'bash', timeoutMs: HOOK_FANOUT_TIMEOUT_MS }); throwIfFailed(result, `bash ${script} ${input}`); return result.stdout; } diff --git a/tests/code-review-depth.test.cjs b/tests/code-review-depth.test.cjs index de8c1f9fc..6c59a3d54 100644 --- a/tests/code-review-depth.test.cjs +++ b/tests/code-review-depth.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/src/auth/x.ts' is a synthetic files-list fixture entry, not a real repo doc. /** * Failing-first (RED) tests for #2554 — path-scoped code-review depth overrides. * diff --git a/tests/code-review-pipeline-regression.test.cjs b/tests/code-review-pipeline-regression.test.cjs index 7da10ff9f..75959936e 100644 --- a/tests/code-review-pipeline-regression.test.cjs +++ b/tests/code-review-pipeline-regression.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/DEVELOPMENT.md' is a synthetic files-list fixture entry, not read as content. // allow-test-rule: source-text-is-the-product // The workflow and agent .md files ARE the product: their text is loaded and // executed/interpreted at runtime by the agent host. Testing that specific @@ -27,7 +28,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { runHook } = require('./helpers/process-seam.cjs'); const { toLegacyResult, gitOrThrow } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS, GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { PROBE_TIMEOUT_MS, GIT_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempDir, createTempGitProject, cleanup, readFileNormalized } = require('./helpers.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -808,11 +809,18 @@ function runDerivation(repo, snippet, phase) { 'printf \'%s\\n\' "$FALLOW_BASE"', 'echo "===END==="', ].join('\n'); + // Bash FAN-OUT: the extracted snippet runs `git log` plus an `echo | tail` + // pipe — the wrong class for `PROBE_TIMEOUT_MS` (a single short CLI + // probe). Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. return toLegacyResult( runHook('-c', [script, 'bash'], { interpreter: 'bash', cwd: repo, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }) ); } diff --git a/tests/codebuddy-upgrades.test.cjs b/tests/codebuddy-upgrades.test.cjs index a727258a1..6f2d5a8b5 100644 --- a/tests/codebuddy-upgrades.test.cjs +++ b/tests/codebuddy-upgrades.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: codebuddy.ai/docs/... is an external URL citation in a comment, not a repo path. 'use strict'; /** diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 174224416..1cb190a81 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -1,6 +1,9 @@ // allow-test-rule: source-text-is-the-product // Reads .md/.json/.yml product files whose deployed text IS what the // runtime loads — testing text content tests the deployed contract. +// docs-guard-exempt: 'docs/x.md' below is a synthetic fixture path fed into +// groupFilesBySubrepo() to exercise its subrepo-grouping logic — no real +// docs/ file is ever read or asserted on for content. /** * GSD Tools Tests - Commands diff --git a/tests/commit-docs-bypass.test.cjs b/tests/commit-docs-bypass.test.cjs index b420f2434..c760c782b 100644 --- a/tests/commit-docs-bypass.test.cjs +++ b/tests/commit-docs-bypass.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/readme.md' is a fast-check filler token and 'commit_docs' is a config key name, not a docs/ path read. /** * commit_docs bypass guard (#1783; superseded/widened by #3585) * diff --git a/tests/complexity-trigger.test.cjs b/tests/complexity-trigger.test.cjs index 9f873559c..2f9b740a5 100644 --- a/tests/complexity-trigger.test.cjs +++ b/tests/complexity-trigger.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/readme.md' is a synthetic fixture path fed to isAnalyzablePath(), never read as content. 'use strict'; /** diff --git a/tests/cursor-imperative-reference.test.cjs b/tests/cursor-imperative-reference.test.cjs index a56d5fe61..1df04d073 100644 --- a/tests/cursor-imperative-reference.test.cjs +++ b/tests/cursor-imperative-reference.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: cursor.com/docs/... is an external URL citation in an assert message, not a repo path. // allow-test-rule: structural-regression-guard — AC2 requires asserting no `runtime === 'cursor'` string-equality branch remains in bin/install.js/src — the descriptor-migration contract is a property of the source text, so a source-grep is the only faithful check (#2089) 'use strict'; diff --git a/tests/declarative-reference-antigravity.test.cjs b/tests/declarative-reference-antigravity.test.cjs index 2c93de425..1bf5ab313 100644 --- a/tests/declarative-reference-antigravity.test.cjs +++ b/tests/declarative-reference-antigravity.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: antigravity.google/docs/... is an external URL citation in comments, not a repo path. // allow-test-rule: structural-regression-guard — AC2 requires asserting no `runtime === 'antigravity'` string-equality branch (nor an `isAntigravity` helper, nor a `canonical === 'antigravity'` branch) remains in bin/install.js, src/runtime-artifact-conversion.cts, src/shell-command-projection.cts, and src/runtime-name-policy.cts — the descriptor-migration contract is a property of the source text, so a source-grep is the only faithful check (#2096) 'use strict'; diff --git a/tests/declarative-reference-zcode.test.cjs b/tests/declarative-reference-zcode.test.cjs index 17a413ebd..ca4b64df3 100644 --- a/tests/declarative-reference-zcode.test.cjs +++ b/tests/declarative-reference-zcode.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/reference/host-integration-capability-matrix.md is cited only in a comment; this file never reads it. // allow-test-rule: structural-regression-guard — AC2: assert no `runtime === 'zcode'` string-equality branch, no live `isZcode` read remains in bin/install.js, src/install-engine.cts, src/surface.cts, or src/runtime-artifact-conversion.cts — a source-text property, so source-grep is the faithful check (#2101) 'use strict'; diff --git a/tests/emitted-attribution.test.cjs b/tests/emitted-attribution.test.cjs index 14121d3ef..b1f5ab96d 100644 --- a/tests/emitted-attribution.test.cjs +++ b/tests/emitted-attribution.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/README.md' and 'docs/tests/...' are synthetic changedPaths/fixture-path strings, never read as content. 'use strict'; /** diff --git a/tests/eslint-rules.test.cjs b/tests/eslint-rules.test.cjs index d151c9cc4..f2625b850 100644 --- a/tests/eslint-rules.test.cjs +++ b/tests/eslint-rules.test.cjs @@ -1,5 +1,9 @@ 'use strict'; +// docs-guard-exempt: 'docs/readme.md' appears only inside literal RuleTester +// fixture `code` strings (sample source text fed to no-source-grep for AST +// linting) — this file never itself reads a real docs/ file off disk. + /** * eslint-rules.test.cjs * diff --git a/tests/estimate-calibrate.test.cjs b/tests/estimate-calibrate.test.cjs index 83ad85d2b..61139146e 100644 --- a/tests/estimate-calibrate.test.cjs +++ b/tests/estimate-calibrate.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docPath is a .planning/estimation-calibration.json tmp fixture; the docs/adr and docs/reference citations are comment-only. /** * estimate-calibrate — build the calibration document from completed phases. * diff --git a/tests/execute-phase-worktree-guard.test.cjs b/tests/execute-phase-worktree-guard.test.cjs index 9935716bb..d60e26b01 100644 --- a/tests/execute-phase-worktree-guard.test.cjs +++ b/tests/execute-phase-worktree-guard.test.cjs @@ -13,6 +13,7 @@ const path = require('node:path'); const { cleanup } = require('./helpers.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.resolve(__dirname, '..'); const WORKFLOW = path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase.md'); @@ -62,12 +63,17 @@ function commitFile(dir, name, msg) { /** Run the extracted guard in `dir`. Never throws — returns the observed result. */ function runGuard(dir) { - // 30s: already bounded pre-migration (unchanged) — the guard runs a handful - // of git plumbing calls (rev-parse, log, status) against a small fixture repo. + // Bash FAN-OUT: the guard runs a sequence of git plumbing calls + // (rev-parse, log, status) under one `bash` interpreter, not a single git + // call — the wrong class for a plumbing-sized bound. Same class as the + // observed CI failures in tests/quick-branching.test.cjs (PR #3787 run + // 32668773524) and tests/worktree-safety.test.cjs (`next` run + // 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for + // the class rationale. const res = runHook('-c', [guardScript()], { interpreter: 'bash', cwd: dir, - timeoutMs: 30_000, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, }); return { status: res.exitCode, stdout: res.stdout || '', stderr: res.stderr || '' }; diff --git a/tests/executed-plan.test.cjs b/tests/executed-plan.test.cjs index 79ef8a435..fe6a76ea8 100644 --- a/tests/executed-plan.test.cjs +++ b/tests/executed-plan.test.cjs @@ -58,7 +58,7 @@ const TEST_ATTRIBUTION = () => 'Co-Authored-By: Test '; * install would write into the developer's real home directory. * * #3712: promoted to tests/helpers.cjs, from the byte-identical copy that used - * to live here. It now also sets the sandbox marker src/test-home-guard.cts + * to live here. It now also sets the sandbox marker src/real-home-guard.cts * needs to stay permissive on hosts with no readable passwd entry. */ const { sandboxHome } = require('./helpers.cjs'); diff --git a/tests/gen-context-index.test.cjs b/tests/gen-context-index.test.cjs index 8959b2953..cbd856620 100644 --- a/tests/gen-context-index.test.cjs +++ b/tests/gen-context-index.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/CONTEXT-INDEX.json is mentioned only in module-doc comments; the test reads root CONTEXT.md and mocked fs, never a real docs/ path. 'use strict'; /** diff --git a/tests/gen-registry.test.cjs b/tests/gen-registry.test.cjs index 9f9fb8744..ce5502641 100644 --- a/tests/gen-registry.test.cjs +++ b/tests/gen-registry.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/registries/ paths are synthetic fixtures written under a tmpdir, never the shipped docs/ tree. 'use strict'; process.env.GSD_TEST_MODE = '1'; diff --git a/tests/git-base-branch.test.cjs b/tests/git-base-branch.test.cjs index bdc24cbe9..1b666e225 100644 --- a/tests/git-base-branch.test.cjs +++ b/tests/git-base-branch.test.cjs @@ -35,7 +35,7 @@ const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); // #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs. -const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { GIT_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // ─── helpers ────────────────────────────────────────────────────────────────── @@ -1174,7 +1174,13 @@ function runHandleBranchingStep(bash, cwd, branchName) { const script = `#!/usr/bin/env bash\nset -uo pipefail\nBRANCH_NAME="${branchName}"\n${bash}\n`; fs.writeFileSync(scriptPath, script, { mode: 0o755 }); try { - const r = runHook(scriptPath, [], { interpreter: 'bash', cwd, env: GIT_ENV, timeoutMs: GIT_TIMEOUT_MS }); + // Bash FAN-OUT (a sequence of git commands under one `bash` interpreter), + // not a single git plumbing call — the wrong class for `GIT_TIMEOUT_MS`. + // Same class as the observed CI failures in tests/quick-branching.test.cjs + // (PR #3787 run 32668773524) and tests/worktree-safety.test.cjs (`next` + // run 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs + // for the class rationale. + const r = runHook(scriptPath, [], { interpreter: 'bash', cwd, env: GIT_ENV, timeoutMs: HOOK_FANOUT_TIMEOUT_MS }); throwIfFailed(r, `runHandleBranchingStep: bash ${scriptPath}`); return r.stdout; } finally { diff --git a/tests/graphify-auto-update.slow.test.cjs b/tests/graphify-auto-update.slow.test.cjs index fd5b20486..75b369be7 100644 --- a/tests/graphify-auto-update.slow.test.cjs +++ b/tests/graphify-auto-update.slow.test.cjs @@ -15,7 +15,7 @@ const os = require('node:os'); const { createTempProject, cleanup, runGsdTools, delay } = require('./helpers.cjs'); const { runGit, runHook: seamRunHook } = require('./helpers/process-seam.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { graphifyStatus, @@ -251,10 +251,14 @@ describe('auto-update', () => { const PATH = pathPrepend ? `${pathPrepend}${path.delimiter}${process.env.PATH || ''}` : process.env.PATH || ''; - // 30000ms: already bounded pre-migration (unchanged) — this is the `slow` - // suite and the hook itself dispatches a detached graphify rebuild that - // some tests wait on separately; the hook's own synchronous return (gate - // checks + status-file write) is fast, so 30s stays generous headroom. + // Bash FAN-OUT: `gsd-graphify-update.sh` is a real hook shelling out to + // `git` and `graphify` (mocked here) under one `bash` interpreter — the + // wrong class for a plumbing-sized bound, even though the hook's own + // synchronous return (gate checks + status-file write) is fast. Same + // class as the observed CI failures in tests/quick-branching.test.cjs + // (PR #3787 run 32668773524) and tests/worktree-safety.test.cjs (`next` + // run 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs + // for the class rationale. // Original invoked the hook with stdio: 'ignore' — the seam always // captures stdout/stderr instead, but every call site of this wrapper // below reads only `.status`; the captured output is simply unread. @@ -268,7 +272,7 @@ describe('auto-update', () => { CI: '', ...env, }, - timeoutMs: 30000, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); return { status: r.exitCode, stdout: r.stdout, stderr: r.stderr }; } diff --git a/tests/graphify-visualization.test.cjs b/tests/graphify-visualization.test.cjs index 99cea213d..a7586eebc 100644 --- a/tests/graphify-visualization.test.cjs +++ b/tests/graphify-visualization.test.cjs @@ -614,7 +614,7 @@ const fs = require('fs'); const path = require('path'); const { runHook } = require('./helpers/process-seam.cjs'); const { toLegacyResult } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempDir, cleanup, readFileNormalized } = require('./helpers.cjs'); @@ -711,7 +711,14 @@ function populateSandbox(includeHtml) { */ function runBlock(block) { // The extracted block is a shell chain (&&, [ -f ] guards, ||) — it stays - // a `bash -c` invocation rather than being decomposed into argv. + // a `bash -c` invocation rather than being decomposed into argv. Bash + // FAN-OUT: the block spawns `graphify update .` (and further commands + // chained via && / ||) under one `bash` interpreter, not a single CLI + // probe — the wrong class for `PROBE_TIMEOUT_MS`. Same class as the + // observed CI failures in tests/quick-branching.test.cjs (PR #3787 run + // 32668773524) and tests/worktree-safety.test.cjs (`next` run + // 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for + // the class rationale. const r = runHook('-c', [block], { interpreter: 'bash', cwd: sandbox, @@ -720,7 +727,7 @@ function runBlock(block) { PATH: fakeBin + ':' + process.env.PATH, HOME: fakeHome, }, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); return toLegacyResult(r); } diff --git a/tests/gsd-agent-isolation-guard.test.cjs b/tests/gsd-agent-isolation-guard.test.cjs index ab3dbeaae..a6cb75f38 100644 --- a/tests/gsd-agent-isolation-guard.test.cjs +++ b/tests/gsd-agent-isolation-guard.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/adr/1239-...md is cited only in a comment as rationale; never read. 'use strict'; /** diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 9695b887e..23db5ad16 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -1060,7 +1060,7 @@ function sandboxHome(t, dir) { const savedMarker = process.env[TEST_HOME_SANDBOX_MARKER]; process.env.HOME = dir; process.env.USERPROFILE = dir; - // Records WHICH directory this call sandboxed to. src/test-home-guard.cts fails + // Records WHICH directory this call sandboxed to. src/real-home-guard.cts fails // CLOSED when it cannot read a passwd entry to compare HOME against (some CI // images), and consults this only in that branch, accepting it only when it // names the home actually in effect — so a stale marker cannot vouch for a diff --git a/tests/hermes-dispatch-upgrade.test.cjs b/tests/hermes-dispatch-upgrade.test.cjs index 7d698c8d1..140eae636 100644 --- a/tests/hermes-dispatch-upgrade.test.cjs +++ b/tests/hermes-dispatch-upgrade.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: github.com/.../docs/... is an external URL citation in a comment, not a repo path. 'use strict'; /** diff --git a/tests/hooks-opt-in.test.cjs b/tests/hooks-opt-in.test.cjs index 49adcdb08..e19b01abf 100644 --- a/tests/hooks-opt-in.test.cjs +++ b/tests/hooks-opt-in.test.cjs @@ -21,11 +21,18 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { runHook } = require('./helpers/process-seam.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HOOKS_DIR = path.join(__dirname, '..', 'hooks'); const isWindows = process.platform === 'win32'; -// 15000: a single bash hook script under test, not an install or a build. -const HOOK_TIMEOUT_MS = 15000; +// This is a bash FAN-OUT: the hook itself runs under `bash`, and it shells +// out to `node` (see hookEnv below, which puts node on PATH for exactly that +// reason). 15000ms was sized for a single-probe class, not this one. Same +// class as the observed CI failures in tests/quick-branching.test.cjs (PR +// #3787 run 32668773524) and tests/worktree-safety.test.cjs (`next` run +// 32608945654) — see HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the +// class rationale. +const HOOK_TIMEOUT_MS = HOOK_FANOUT_TIMEOUT_MS; // Ensure the running node binary is on PATH so bash hooks can call `node` // (Claude Code shell sessions do not have `node` on PATH). diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index b295a432e..b8040e6c2 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/... substrings are external URL citations (qwenlm/code.claude.com) in comments, not repo paths. /** * Installer Module — Sections 9–11 + 13. * diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index f88ec6da8..9f67468d3 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: codebuddy.ai/docs/... is an external URL and docs/adr/58-...md is a comment citation; neither is read. // allow-test-rule: source-text-is-the-product // Reads .md/.json/.yml product files whose deployed text IS what the // runtime loads — testing text content tests the deployed contract. @@ -220,7 +221,7 @@ function readAllSkillMd(dir) { // install/uninstall so codex's resolved skills dir is configDir/.agents/skills. // // #3712: promoted to tests/helpers.cjs, from the byte-identical copy that used to -// live here. It now also sets the sandbox marker src/test-home-guard.cts needs to +// live here. It now also sets the sandbox marker src/real-home-guard.cts needs to // stay permissive on hosts with no readable passwd entry. const { sandboxHome } = require('./helpers.cjs'); @@ -5086,7 +5087,7 @@ describe('Bug #2911: migrateLegacyDevPreferencesToSkill honors the skills-kind h function withFakeHome(fakeHome, fn) { const savedHome = process.env.HOME; const savedUserProfile = process.env.USERPROFILE; - // #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails + // #3712: record WHICH home this sandboxed to. src/real-home-guard.cts fails // closed on hosts with no readable passwd entry, and this is what proves a // genuinely-sandboxed caller there. Without it these calls would be refused. const savedMarker = process.env.GSD_TEST_HOME_SANDBOX; diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs index e4b3b14a6..a7fcda4e0 100644 --- a/tests/install-write-confinement.test.cjs +++ b/tests/install-write-confinement.test.cjs @@ -3215,7 +3215,7 @@ describe('#3712 in-process home confinement', () => { const installEngine = require('../gsd-core/bin/lib/install-engine.cjs'); const surface = require('../gsd-core/bin/lib/surface.cjs'); const runtimeArtifactLayout = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs'); - const testHomeGuard = require('../gsd-core/bin/lib/test-home-guard.cjs'); + const testHomeGuard = require('../gsd-core/bin/lib/real-home-guard.cjs'); const escapingKinds = (home) => [{ kind: 'skills', home, destSubpath: 'skills' }]; const confinedKinds = [{ kind: 'skills', destSubpath: 'skills' }]; diff --git a/tests/install.test.cjs b/tests/install.test.cjs index c41f1049a..be48050c8 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -7014,8 +7014,10 @@ const { cleanup, installSpawnEnv } = require('./helpers.cjs'); const { runNode: seamRunNode, runHook: seamRunHook } = require('./helpers/process-seam.cjs'); // Class-norm timeouts, not local literals (CONTRIBUTING: they live in // tests/helpers/timeouts.cjs). The install is the INSTALL class; the emitted -// gate is a short CLI probe against a temp fixture, i.e. the PROBE class. -const { INSTALL_TIMEOUT_MS, PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +// gate below is a bash FAN-OUT (the gate script's `gsd_run` shells out to +// `node` multiple times), not a single CLI probe, so it takes +// HOOK_FANOUT_TIMEOUT_MS rather than PROBE_TIMEOUT_MS. +const { INSTALL_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const INSTALL = path.join(__dirname, '..', 'bin', 'install.js'); @@ -7218,13 +7220,20 @@ test('real install: cursor negotiates --worktree through its own emitted gate an // Seam again — `runHook` documents `interpreter: 'bash'` for a shell // script, so the gate is written to a file rather than passed as `-c`. + // This is a bash FAN-OUT: `gsd_run` shells out to `node` multiple times + // under one `bash` interpreter, the wrong class for `PROBE_TIMEOUT_MS` + // (a single short CLI probe). Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. const gateScript = path.join(proj, 'run-gate.sh'); fs.writeFileSync(gateScript, script); const run = seamRunHook(gateScript, [], { interpreter: 'bash', cwd: proj, env: hermeticEnv, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); assert.strictEqual( run.outcome, 'exited', diff --git a/tests/installer-migration-config-root-marker.test.cjs b/tests/installer-migration-config-root-marker.test.cjs index 846faf607..1b4678d6b 100644 --- a/tests/installer-migration-config-root-marker.test.cjs +++ b/tests/installer-migration-config-root-marker.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/installer-migrations.md is cited only in comments as design rationale; never read. 'use strict'; /** diff --git a/tests/installer-migration-pi-extension-ext.test.cjs b/tests/installer-migration-pi-extension-ext.test.cjs index 26b3c4214..8afb2a61f 100644 --- a/tests/installer-migration-pi-extension-ext.test.cjs +++ b/tests/installer-migration-pi-extension-ext.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/installer-migrations.md is cited only in comments as design rationale; never read. 'use strict'; /** diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index a58d3f922..a30c23cda 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/installer-migrations.md is cited only in a comment; never read. const test = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); diff --git a/tests/kimi-upgrades.test.cjs b/tests/kimi-upgrades.test.cjs index 5da52fcc2..0505bcae3 100644 --- a/tests/kimi-upgrades.test.cjs +++ b/tests/kimi-upgrades.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/reference/host-integration-capability-matrix.md is cited only in a comment; never read. 'use strict'; /** diff --git a/tests/lint-allow-test-rule-refs.test.cjs b/tests/lint-allow-test-rule-refs.test.cjs index b7e60138b..ab38bd406 100644 --- a/tests/lint-allow-test-rule-refs.test.cjs +++ b/tests/lint-allow-test-rule-refs.test.cjs @@ -1,5 +1,9 @@ 'use strict'; +// docs-guard-exempt: 'docs/readme.md' appears only inside literal fixture +// `code` strings written to synthetic files and fed to the no-source-grep +// lint under test — this file never itself reads a real docs/ file off disk. + // Tests for scripts/lint-allow-test-rule-refs.cjs — the guard that (a) // ratchets exemption-marker comments on IDENTITY (uncited comments must // carry a tracking-issue ref or be grandfathered), (b) ratchets EFFECTIVE diff --git a/tests/lint-docs-command-form.test.cjs b/tests/lint-docs-command-form.test.cjs index cf91f4fc4..072475fb2 100644 --- a/tests/lint-docs-command-form.test.cjs +++ b/tests/lint-docs-command-form.test.cjs @@ -1,5 +1,9 @@ 'use strict'; +// docs-guard-exempt: this file only WRITES synthetic 'docs/...' fixtures into +// a throwaway temp git repo (writeFile()) to exercise scripts/lint-docs-command-form.cjs's +// own behavior — it never reads real shipped docs/ content. + /** * TDD tests for scripts/lint-docs-command-form.cjs (#2903). * diff --git a/tests/lint-docs-required.test.cjs b/tests/lint-docs-required.test.cjs index 6f58521d3..95b2f53d1 100644 --- a/tests/lint-docs-required.test.cjs +++ b/tests/lint-docs-required.test.cjs @@ -1,4 +1,7 @@ 'use strict'; +// docs-guard-exempt: isDocsFile('docs/...') calls below spot-check a pure +// path-classifier predicate with string literals — no real docs/ file is +// ever read or asserted on for content. process.env.GSD_TEST_MODE = '1'; const { test, describe } = require('node:test'); diff --git a/tests/lint-source-test-name-collision.test.cjs b/tests/lint-source-test-name-collision.test.cjs new file mode 100644 index 000000000..94f6756d1 --- /dev/null +++ b/tests/lint-source-test-name-collision.test.cjs @@ -0,0 +1,212 @@ +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const { + checkSourceTestNameCollisions, +} = require('../scripts/lint-source-test-name-collision.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); + +function writeFile(root, relPath, content = '// fixture\n') { + const full = path.join(root, relPath); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); +} + +function violationFiles(result) { + return result.violations.map((v) => v.file); +} + +describe('lint-source-test-name-collision', () => { + test('the real repo tree passes (post-rename)', () => { + const result = checkSourceTestNameCollisions({ root: REPO_ROOT }); + assert.equal( + result.ok, + true, + `expected 0 violations, got: ${JSON.stringify(result.violations, null, 2)}` + ); + // Not vacuous: the scan must have actually walked a nontrivial number of + // files, or "0 violations" would mean nothing. + assert.ok( + result.scanned.length > 50, + `expected the scan to cover a substantial file count, got ${result.scanned.length}` + ); + }); + + test('a deliberately-colliding fixture DOES fail (proves the lint can fail)', () => { + const root = createTempDir('lint-collision-fail-'); + try { + writeFile(root, 'src/test-oops.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + assert.deepEqual(violationFiles(result), ['src/test-oops.cts']); + } finally { + cleanup(root); + } + }); + + test('regression pin: test-home-guard.cts under a source dir is flagged', () => { + // Incident: unmodified `next` tip 622f43353 ran 37199 passed / 1 failed, + // `throw · src/test-home-guard.cts` — a SOURCE module collected and + // executed as a test by the remote push-gate runner. Two further live + // instances found by this same trap before they were renamed: + // scripts/test-failure-reasons.cjs (now scripts/gsd-test-gate-reasons.cjs) + // scripts/lint-fix-has-regression-test.cjs (now + // scripts/lint-fix-has-regression-tests.cjs) + const root = createTempDir('lint-collision-incident-'); + try { + writeFile(root, 'src/test-home-guard.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + assert.deepEqual(violationFiles(result), ['src/test-home-guard.cts']); + } finally { + cleanup(root); + } + }); + + test('a colliding basename under an EXEMPT dir (tests/) passes', () => { + const root = createTempDir('lint-collision-exempt-'); + try { + writeFile(root, 'tests/test-oops.cts'); + // Only 'src' is scanned — tests/ is never a configured scan dir, exactly + // as in the real DEFAULT_SCAN_DIRS. + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, true); + assert.deepEqual(result.violations, []); + } finally { + cleanup(root); + } + }); + + test('the identical basename under a SOURCE dir fails (contrast case)', () => { + const root = createTempDir('lint-collision-source-'); + try { + writeFile(root, 'src/test-oops.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + assert.deepEqual(violationFiles(result), ['src/test-oops.cts']); + } finally { + cleanup(root); + } + }); + + describe('boundary coverage: test-*.EXT', () => { + test('test-foo.cts FAILS', () => { + const root = createTempDir('lint-b1-'); + try { + writeFile(root, 'src/test-foo.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + } finally { + cleanup(root); + } + }); + + test('testfoo.cts PASSES', () => { + const root = createTempDir('lint-b2-'); + try { + writeFile(root, 'src/testfoo.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, true); + } finally { + cleanup(root); + } + }); + }); + + describe('boundary coverage: *-test.EXT', () => { + test('foo-test.cts FAILS', () => { + const root = createTempDir('lint-b3-'); + try { + writeFile(root, 'src/foo-test.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + } finally { + cleanup(root); + } + }); + + test('footest.cts PASSES', () => { + const root = createTempDir('lint-b4-'); + try { + writeFile(root, 'src/footest.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, true); + } finally { + cleanup(root); + } + }); + }); + + describe('boundary coverage: *.test.EXT', () => { + test('foo.test.cts FAILS', () => { + const root = createTempDir('lint-b5-'); + try { + writeFile(root, 'src/foo.test.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + } finally { + cleanup(root); + } + }); + + test('foo.tests.cts PASSES', () => { + const root = createTempDir('lint-b6-'); + try { + writeFile(root, 'src/foo.tests.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, true); + } finally { + cleanup(root); + } + }); + }); + + describe('boundary coverage: test.EXT (bare)', () => { + test('test.cts FAILS', () => { + const root = createTempDir('lint-b7-'); + try { + writeFile(root, 'src/test.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, false); + } finally { + cleanup(root); + } + }); + + test('tests.cts PASSES', () => { + const root = createTempDir('lint-b8-'); + try { + writeFile(root, 'src/tests.cts'); + const result = checkSourceTestNameCollisions({ root, dirs: ['src'] }); + assert.equal(result.ok, true); + } finally { + cleanup(root); + } + }); + }); + + test('an unreadable scan directory fails closed, not silently green', () => { + const root = createTempDir('lint-unreadable-'); + try { + // No 'src' dir created at all under a DIFFERENT configured dir name so + // fs.existsSync short-circuits it (not this lint's problem, per its own + // contract) — instead force a real read failure by pointing `dirs` at a + // path that exists as a FILE, not a directory, so readdirSync throws. + writeFile(root, 'not-a-dir', '// not a directory\n'); + const result = checkSourceTestNameCollisions({ root, dirs: ['not-a-dir'] }); + assert.equal(result.ok, false); + assert.ok( + result.violations.some((v) => /cannot read scan directory/.test(v.reason)), + `expected an unreadable-dir violation, got: ${JSON.stringify(result.violations)}` + ); + } finally { + cleanup(root); + } + }); +}); diff --git a/tests/manifest-version-sync.test.cjs b/tests/manifest-version-sync.test.cjs index 3b3d10bbb..a52896ec4 100644 --- a/tests/manifest-version-sync.test.cjs +++ b/tests/manifest-version-sync.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/' appears only as an excluded-prefix string in a guard's exclusion list, never read. 'use strict'; /** diff --git a/tests/milestone-archive.test.cjs b/tests/milestone-archive.test.cjs index c0ecc3c7e..affbdfa97 100644 --- a/tests/milestone-archive.test.cjs +++ b/tests/milestone-archive.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/TESTING-SUITES.md is cited only in a placement-note comment; never read. 'use strict'; /** diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index b6e184232..9e09dc9ee 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/TESTING-SUITES.md is cited only in a header comment; never read. 'use strict'; /** diff --git a/tests/new-project-mvp-prompt.test.cjs b/tests/new-project-mvp-prompt.test.cjs index 9bd1f3eaf..926be5fb1 100644 --- a/tests/new-project-mvp-prompt.test.cjs +++ b/tests/new-project-mvp-prompt.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/CONFIGURATION.md is cited only in a header comment; never read. /** * new-project workflow — MVP mode prompt contract test * Verifies the workflow markdown documents the Vertical MVP / Horizontal Layers diff --git a/tests/onboard-command.test.cjs b/tests/onboard-command.test.cjs index b941087e5..3300079ab 100644 --- a/tests/onboard-command.test.cjs +++ b/tests/onboard-command.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/adr/0001-runtime.md fixtures are synthetic files written into a tmpDir, never the shipped docs/ tree. // allow-test-rule: source-text-is-the-product (see #1990) // Command/workflow markdown is deployed runtime product; source-text assertions // below verify the installed command contract. CLI assertions exercise real diff --git a/tests/opencode-command-dir-plural.test.cjs b/tests/opencode-command-dir-plural.test.cjs index 3989101a7..5c052a674 100644 --- a/tests/opencode-command-dir-plural.test.cjs +++ b/tests/opencode-command-dir-plural.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: opencode.ai/docs/... is an external URL citation in comments, not a repo path. 'use strict'; /** diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 7371c3f7d..6ad77b877 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/adr/3524-...md is cited only in a References comment; never read. // allow-test-rule: source-text-is-the-product // Reads .md/.json/.yml product files whose deployed text IS what the // runtime loads — testing text content tests the deployed contract. diff --git a/tests/pr-branch-planning-filter.test.cjs b/tests/pr-branch-planning-filter.test.cjs index 587b422dd..67ba55177 100644 --- a/tests/pr-branch-planning-filter.test.cjs +++ b/tests/pr-branch-planning-filter.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/readme.md' is a synthetic non-planning-path fixture, never read as content. 'use strict'; process.env.GSD_TEST_MODE = '1'; diff --git a/tests/precommit-alias-drift-hook.test.cjs b/tests/precommit-alias-drift-hook.test.cjs index 2794fdffd..8fb6f90ad 100644 --- a/tests/precommit-alias-drift-hook.test.cjs +++ b/tests/precommit-alias-drift-hook.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/adr/0174-...md' and 'docs/' are synthetic fixture path/prefix values, never read as content. 'use strict'; const { describe, test } = require('node:test'); @@ -8,7 +9,7 @@ const fc = require('fast-check'); const { runHook } = require('./helpers/process-seam.cjs'); const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { stagedSourcePaths } = require('../scripts/lib/alias-drift-families.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -80,6 +81,13 @@ function runPreCommit(t, stagedLines) { `#!/usr/bin/env bash\nprintf 'call %s\\n' "$*" >> "$GSD_TEST_NPM_MARKER"\n`, ); + // This IS the fan-out class HOOK_FANOUT_TIMEOUT_MS documents: the + // pre-commit hook runs under `bash` and shells out to `git diff`, `tr`, + // `grep`, and conditionally `npm` (which itself runs node). Same class as + // the observed CI failures in tests/quick-branching.test.cjs (PR #3787 run + // 32668773524) and tests/worktree-safety.test.cjs (`next` run + // 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for + // the class rationale. const result = runHook(HOOK_PATH, [], { interpreter: 'bash', cwd: ROOT, @@ -89,7 +97,7 @@ function runPreCommit(t, stagedLines) { NPM_OVERRIDE: mockNpm, GSD_TEST_NPM_MARKER: marker, }, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); const npmCalls = fs.existsSync(marker) diff --git a/tests/quick-branching.test.cjs b/tests/quick-branching.test.cjs index a98dab5af..8230a3ede 100644 --- a/tests/quick-branching.test.cjs +++ b/tests/quick-branching.test.cjs @@ -21,6 +21,7 @@ const path = require('node:path'); const { cleanup, readFileNormalized } = require('./helpers.cjs'); const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const QUICK_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'quick.md'); @@ -146,7 +147,14 @@ function runStep(bash, cwd, branchName) { const script = `#!/usr/bin/env bash\nset -uo pipefail\nbranch_name="${branchName}"\n${bash}\n`; fs.writeFileSync(scriptPath, script, { mode: 0o755 }); try { - const r = runHook(scriptPath, [], { interpreter: 'bash', cwd, env: GIT_ENV, timeoutMs: GIT_TIMEOUT_MS }); + // Step 2.5's script is a bash FAN-OUT (multiple git commands in sequence + // under one `bash` interpreter), not a single git plumbing call — the + // wrong class for GIT_TIMEOUT_MS. CI hit exactly this at 15000ms on PR + // #3787 run 32668773524 (`full test (windows-latest, 24, shard 3/3)`, + // `new quick-task branch branches off origin/main (#2916)`): SIGTERM, + // outcome=timed_out exitCode=null. See HOOK_FANOUT_TIMEOUT_MS in + // ./helpers/timeouts.cjs for the class rationale. + const r = runHook(scriptPath, [], { interpreter: 'bash', cwd, env: GIT_ENV, timeoutMs: HOOK_FANOUT_TIMEOUT_MS }); throwIfFailed(r, `runStep: bash ${scriptPath}`); return r.stdout; } finally { diff --git a/tests/removed-but-needed-lint.test.cjs b/tests/removed-but-needed-lint.test.cjs index 28ae7f115..0fe89757a 100644 --- a/tests/removed-but-needed-lint.test.cjs +++ b/tests/removed-but-needed-lint.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/getting-started.md and docs/setup.md are synthetic { file, content } corpus fixtures fed to a lint checker, not real repo docs. 'use strict'; process.env.GSD_TEST_MODE = '1'; diff --git a/tests/repo-invariants.test.cjs b/tests/repo-invariants.test.cjs index 08ce3b7f8..71ecd30e4 100644 --- a/tests/repo-invariants.test.cjs +++ b/tests/repo-invariants.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/ is explicitly named in this file's own comments as a directory it deliberately excludes from its RUNTIME_SURFACES scan. 'use strict'; // Repo-wide invariant scans. diff --git a/tests/require-issue-link-policy.test.cjs b/tests/require-issue-link-policy.test.cjs index ed9fb80cf..b3c062082 100644 --- a/tests/require-issue-link-policy.test.cjs +++ b/tests/require-issue-link-policy.test.cjs @@ -1,5 +1,9 @@ 'use strict'; +// docs-guard-exempt: allPathsAreTestsOrDocs(['tests/a.cjs', 'docs/b.md']) below +// spot-checks a pure path-classifier predicate with string literals — no real +// docs/ file is ever read or asserted on for content. + /** * Tests for scripts/require-issue-link-policy.cjs (#3211, preserving #1389). * diff --git a/tests/reviewer-manifest-body.test.cjs b/tests/reviewer-manifest-body.test.cjs index d730de6d1..db4c74cae 100644 --- a/tests/reviewer-manifest-body.test.cjs +++ b/tests/reviewer-manifest-body.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: record.docs is a metadata field string comparison, not a real docs/ file read. 'use strict'; process.env.GSD_TEST_MODE = '1'; diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index 4cb8a6bb7..100886070 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/TESTING-SUITES.md is cited only in a header comment; never read. // allow-test-rule: pending-migration-to-typed-ir [#3090] // run-tests.cjs is a CLI test harness with no --json/structured output mode; // these tests regex/substring-match its human-readable stderr (usage errors, diff --git a/tests/runtime-artifact-layout-surface.test.cjs b/tests/runtime-artifact-layout-surface.test.cjs index 25439d6e7..bdc8a4c71 100644 --- a/tests/runtime-artifact-layout-surface.test.cjs +++ b/tests/runtime-artifact-layout-surface.test.cjs @@ -1140,7 +1140,7 @@ describe('skills-kind destination parity: installer vs surface-apply (#2911)', ( function withFakeHome(fakeHome, fn) { const savedHome = process.env.HOME; const savedUserProfile = process.env.USERPROFILE; - // #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails + // #3712: record WHICH home this sandboxed to. src/real-home-guard.cts fails // closed on hosts with no readable passwd entry, and this is what proves a // genuinely-sandboxed caller there. Without it these calls would be refused. const savedMarker = process.env.GSD_TEST_HOME_SANDBOX; @@ -1272,7 +1272,7 @@ describe('codex skills-kind destination: home override (#2911)', () => { function withFakeHome(fakeHome, fn) { const savedHome = process.env.HOME; const savedUserProfile = process.env.USERPROFILE; - // #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails + // #3712: record WHICH home this sandboxed to. src/real-home-guard.cts fails // closed on hosts with no readable passwd entry, and this is what proves a // genuinely-sandboxed caller there. Without it these calls would be refused. const savedMarker = process.env.GSD_TEST_HOME_SANDBOX; diff --git a/tests/runtime-name-policy.test.cjs b/tests/runtime-name-policy.test.cjs index a1cfc940c..d4b1bca60 100644 --- a/tests/runtime-name-policy.test.cjs +++ b/tests/runtime-name-policy.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: kilo.ai/docs/... is an external URL citation in a comment, not a repo path. 'use strict'; const { describe, test } = require('node:test'); diff --git a/tests/security-prompt-injection.security.test.cjs b/tests/security-prompt-injection.security.test.cjs index 8924b2fc1..f86cdb854 100644 --- a/tests/security-prompt-injection.security.test.cjs +++ b/tests/security-prompt-injection.security.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: '/proj/docs/notes.md' is a synthetic tool_input fixture path for a prompt-injection probe, never real repo content. // allow-test-rule: structural-regression-guard // #3596 calls out "secret-looking values in inputs, logs, stdout, stderr, and // thrown errors" as required negative-proof cases. The only way to assert diff --git a/tests/shipped-reference-cites.test.cjs b/tests/shipped-reference-cites.test.cjs index e98d367aa..e38dc93ea 100644 --- a/tests/shipped-reference-cites.test.cjs +++ b/tests/shipped-reference-cites.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: this file's own header comment states docs/ is deliberately OUT of scope for its citation scan. 'use strict'; // allow-test-rule: source-text-is-the-product (#3576) — this gate reads shipped diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 15c61fa4d..d9e135396 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: docs/CONFIGURATION.md is cited only in a comment; never read. // allow-test-rule: source-text-is-the-product // Reads .md/.json/.yml product files whose deployed text IS what the // runtime loads — testing text content tests the deployed contract. diff --git a/tests/test-failure-reasons.test.cjs b/tests/test-failure-reasons.test.cjs index 1d81d8732..bbd571f2c 100644 --- a/tests/test-failure-reasons.test.cjs +++ b/tests/test-failure-reasons.test.cjs @@ -1,7 +1,7 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); -const { TEST_GATE_REASON, classifyTestGateResult } = require('../scripts/test-failure-reasons.cjs'); +const { TEST_GATE_REASON, classifyTestGateResult } = require('../scripts/gsd-test-gate-reasons.cjs'); describe('test failure reason classification', () => { test('classifies pass on exitCode 0', () => { diff --git a/tests/unreachable-shell-guard.test.cjs b/tests/unreachable-shell-guard.test.cjs index 563022763..4e7be5806 100644 --- a/tests/unreachable-shell-guard.test.cjs +++ b/tests/unreachable-shell-guard.test.cjs @@ -58,7 +58,7 @@ const path = require('node:path'); const { createTempDir, cleanup, readWorkflowCombined } = require('./helpers.cjs'); const { runHook, OUTCOME } = require('./helpers/process-seam.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const PLAN_PHASE_PATH = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'plan-phase.md'); @@ -209,7 +209,15 @@ describe('#3409 G1/G2 — plan-phase.md Walking Skeleton gate observes a real su MVP_MODE: 'true', padded_phase: '01', }, - timeoutMs: PROBE_TIMEOUT_MS, + // Bash FAN-OUT: the sourced `_runtime-launcher.snippet.sh` preamble + // defines the real `gsd_run` function, which the extracted snippet + // then calls — a bash + node invocation, not a single CLI probe. Same + // class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); assert.equal(result.outcome, OUTCOME.EXITED, `gate script did not exit cleanly: ${result.stderr}`); assert.equal(result.exitCode, 0, `gate script exited non-zero: ${result.stderr}`); @@ -271,10 +279,13 @@ test('#3409 G3: an empty phase_req_ids falls back to TBD, not the empty string', block, 'echo "GSD_TEST_PHASE_REQ_IDS=$PHASE_REQ_IDS"', ].join('\n'); + // Bash FAN-OUT: same class as runWalkingSkeletonGate above — the sourced + // launcher's `gsd_run` shells out to node. See HOOK_FANOUT_TIMEOUT_MS in + // ./helpers/timeouts.cjs for the class rationale. const result = runBashScript(t, script, { cwd: root, env: { ...process.env, RUNTIME_DIR: REPO_ROOT, PHASE: '01' }, - timeoutMs: PROBE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); assert.equal(result.outcome, OUTCOME.EXITED, `PHASE_REQ_IDS script did not exit cleanly: ${result.stderr}`); assert.equal(result.exitCode, 0, `PHASE_REQ_IDS script exited non-zero: ${result.stderr}`); diff --git a/tests/workflow-guard.test.cjs b/tests/workflow-guard.test.cjs index 7c4ffe067..e55aa42cf 100644 --- a/tests/workflow-guard.test.cjs +++ b/tests/workflow-guard.test.cjs @@ -25,7 +25,7 @@ const os = require('node:os'); const path = require('node:path'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); const { throwIfFailed } = require('./helpers/git-fixture.cjs'); -const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { cleanup } = require('./helpers.cjs'); @@ -322,10 +322,17 @@ describe('#3504: global-flag spellings of git add -f reach the shared classifier before(() => { const mk = (branch) => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-workflow-guard-bypass-')); + // Bash FAN-OUT: three chained git commands under one `bash` + // interpreter, not a single git plumbing call — the wrong class for + // `PROBE_TIMEOUT_MS`. Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. const initResult = runHookSeam( '-c', [`git init -q -b ${branch} && git config user.email t@t && git config user.name t`], - { interpreter: 'bash', cwd: dir, timeoutMs: PROBE_TIMEOUT_MS }, + { interpreter: 'bash', cwd: dir, timeoutMs: HOOK_FANOUT_TIMEOUT_MS }, ); throwIfFailed(initResult, `bash -c `); return dir; diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs index 76ebbba05..a00e6541d 100644 --- a/tests/worktree-cleanup.test.cjs +++ b/tests/worktree-cleanup.test.cjs @@ -1505,8 +1505,7 @@ const REPO_ROOT = path.join(__dirname, '..'); const EXECUTE_PHASE_PATH = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'execute-phase.md'); // #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs. -// The guard itself keeps its separately-justified 30000ms (see runGuard below). -const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { GIT_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // --------------------------------------------------------------------------- // Extract the cwd-drift guard bash block from execute-phase.md @@ -1587,15 +1586,19 @@ function extractCwdGuardBash() { * Returns { status, stderr }. */ function runGuard(guardBash, cwd) { - // 30000ms: previously UNBOUNDED (no `timeout` option was passed to - // spawnSync). This is the same execute-phase.md cwd-drift guard snippet - // exercised by tests/execute-phase-worktree-guard.test.cjs, which already - // bounds the identical guard at 30s (a handful of git plumbing calls - // against a small fixture repo) — matched here for consistency. + // Bash FAN-OUT: this is the same execute-phase.md cwd-drift guard snippet + // exercised by tests/execute-phase-worktree-guard.test.cjs — a sequence of + // git plumbing calls (rev-parse, log, status) under one `bash` + // interpreter, not a single git call. 30000ms was sized for the wrong + // class. Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class + // rationale. const result = runHook('-c', [guardBash], { interpreter: 'bash', cwd, - timeoutMs: 30_000, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }); return { status: result.exitCode, stderr: result.stderr || '' }; } diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index 49ecff62e..a16a31f71 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -1,3 +1,4 @@ +// docs-guard-exempt: 'docs/SUMMARY.md' is a synthetic fixture path and a predicate-check literal (isSummaryArtifactRelPath), never read as content. 'use strict'; /** @@ -24,6 +25,7 @@ const { createTempDir, cleanup } = require('./helpers.cjs'); const { createFixture } = require('./fixtures/index.cjs'); const { makeFaultyGit } = require('./helpers/faulty-deps.cjs'); const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // 30000ms: this file's single named bound for every migrated subprocess call // below (git plumbing on small mkdtemp fixtures, gsd-tools.cjs/hook CLI runs, @@ -5836,15 +5838,18 @@ const GATE_SNIPPET = [ ].join('\n'); function runGate(cwd, env) { - // 30000ms: previously UNBOUNDED (execFileSync had no `timeout` option). - // The snippet is pure shell string/array parsing plus one `git config - // --file .gitmodules` lookup against a small fixture repo — matched to the - // 30s bound already established for the other bash guard snippets in this - // suite for consistency, though it does substantially less work than those. + // This is a bash FAN-OUT: the `-c` snippet runs shell string/array parsing + // plus a `git config --file .gitmodules` subprocess under one bash + // interpreter, not a single plumbing call — 30000ms was the wrong CLASS, + // not a slow machine. It timed out on `next` itself, run 32608945654, + // `full test (windows-latest, 24, shard 1/3)`, test `plan touching only + // src/ in a submodule project keeps worktree isolation ENABLED`: + // `outcome=timed_out` exitCode null. See HOOK_FANOUT_TIMEOUT_MS in + // ./helpers/timeouts.cjs for the class rationale. const r = seamRunHookGate('-c', [GATE_SNIPPET], { interpreter: 'bash', cwd, - timeoutMs: 30_000, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, env: { ...process.env, ...env }, }); if (r.exitCode !== 0) { diff --git a/tests/worktree.test.cjs b/tests/worktree.test.cjs index ecc1c66ab..4a64a085f 100644 --- a/tests/worktree.test.cjs +++ b/tests/worktree.test.cjs @@ -28,6 +28,7 @@ const os = require('node:os'); const { cleanup } = require('./helpers.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); +const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); /** * Bound for every subprocess in this file: git plumbing/worktree commands @@ -208,19 +209,28 @@ const DISCOVERY_PIPELINE = 'grep "^worktree " | grep "\\.claude/worktrees/agent-" | sed \'s/^worktree //\''; function runDiscoveryAgainstFixture(porcelain) { + // Bash FAN-OUT: a `grep | grep | sed` pipeline under one `bash` + // interpreter, not a single git plumbing call — the wrong class for + // `WORKTREE_TIMEOUT_MS`. Same class as the observed CI failures in + // tests/quick-branching.test.cjs (PR #3787 run 32668773524) and + // tests/worktree-safety.test.cjs (`next` run 32608945654). See + // HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for the class rationale. const out = runHook('-c', [DISCOVERY_PIPELINE], { interpreter: 'bash', input: porcelain, - timeoutMs: WORKTREE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }).stdout; return out.split('\n').filter((l) => l.length > 0); } function runDiscoveryAgainstRepo(repoCwd) { + // Bash FAN-OUT: `git worktree list` piped through `grep | grep | sed` + // under one `bash` interpreter — same class rationale as + // `runDiscoveryAgainstFixture` above. const out = runHook('-c', [`git worktree list --porcelain | ${DISCOVERY_PIPELINE}`], { interpreter: 'bash', cwd: repoCwd, - timeoutMs: WORKTREE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }).stdout; return out.split('\n').filter((l) => l.length > 0); } @@ -726,11 +736,15 @@ while IFS= read -r WT; do printf 'ITER:%s\\n' "$WT" done < <(${DISCOVERY_PIPELINE}) `; - // bash needed for process substitution `< <(...)`. + // bash needed for process substitution `< <(...)`. FAN-OUT: the + // `while/read` loop drives the `grep | grep | sed` discovery + // pipeline under one `bash` interpreter — same class rationale as + // runDiscoveryAgainstRepo above; see HOOK_FANOUT_TIMEOUT_MS in + // ./helpers/timeouts.cjs. const out = runHook('-c', [script], { interpreter: 'bash', input: porcelain, - timeoutMs: WORKTREE_TIMEOUT_MS, + timeoutMs: HOOK_FANOUT_TIMEOUT_MS, }).stdout; const iterations = out .split('\n')