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')