From 5b5d473e1329e74a62b7aaa7a5c4aa9c87cef0aa Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 13:10:49 -0400 Subject: [PATCH] chore(#3465): remove 22 verified-vestigial allow-test-rule markers (#3494) Phase 1 of #3464. Removes the `// allow-test-rule:` marker from 22 test files where it is provably vestigial, and tightens the ratchet ceiling in scripts/lint-allow-test-rule-refs.ceiling.json from 305 to 285. Eligibility is decided by two independent AST discriminators, both conservative (any doubt => keep): (a) Read-target type. Every readFileSync/readFile call in the file resolves statically to a prose/config extension (.md/.json/.yml/.yaml/.toml/.txt), or the file performs no reads at all. Any read of a source extension (.cjs/.js/.mjs/.ts/.cts/.mts/.jsx/.tsx), any dynamic/unresolvable path, and any other extension all disqualify the file. (b) Marker context. Every `allow-test-rule:` occurrence is a genuine comment node, never string- or template-literal payload. A marker that lives inside a RuleTester `code:` fixture is test DATA, not a suppression directive; stripping it corrupts the test. tests/eslint-rules.test.cjs is the one such fixture host and is deliberately untouched. An earlier attempt at this phase classified markers by "strip it and see if local/no-source-grep still passes" and was reverted in full before commit. That oracle is unsound: the rule only fires on a literal .cjs/.js/.ts path containing a quoted bin/lib/gsd-core/src segment, tracked one hop from the binding, so files that genuinely source-grep real JavaScript pass it silently -- tests/no-unbounded-spawn-allowlist.test.cjs (reads test sources through a listTestFiles() walk) and tests/claude-imperative-reference.test.cjs (matches bin/install.js through an intermediate variable) both cleared it while being real source-greps. The rule's implementation is narrower than its intent, so it cannot adjudicate whether an exemption is load-bearing. Scope is limited to comment deletions: the diff over the test tree is 100% line removals with zero insertions, and no executable line is altered. On the ceiling value. The measured count at this HEAD is 283, so 285 leaves 2 slack -- deliberate, and well inside the documented grace band of 3. Pinning the ceiling to the exact count makes this change effectively unmergeable: any concurrent PR that lands one marker-bearing test file re-reds it. That race fired twice while preparing this branch (once mid-rebase taking the count 304->305 on next, once between rebase and the verification run taking it 282->283), and it is the same race that broke next in #3461. A ceiling of actual+2 preserves a merge window while still ratcheting 305 -> 285. Known limit, disclosed rather than papered over: this clears 22 of 303 markers and does not reach #3464's trend-to-zero goal. Most of the remaining markers sit on dynamic-path reads, commonly a hoisted `const p = path.join(tmpDir, 'STATE.md')` whose target is prose but is unresolvable to this classifier. A stricter one-hop const resolution would flip an estimated 95 more; that is deliberately left to a follow-up so it can be reviewed on its own evidence. Marker discovery reads bytes rather than shelling out to grep: tests/security-prompt-injection.security.test.cjs carries a literal NUL byte (an intentional injection fixture) that makes grep treat it as binary and skip it, which is why the true marked-file count is 303 and not the 302 a shell scan reports. Closes #3465 Co-authored-by: sim Co-authored-by: Claude Opus 5 --- scripts/lint-allow-test-rule-refs.allowlist.json | 11 ----------- scripts/lint-allow-test-rule-refs.ceiling.json | 2 +- tests/agent-size-budget.test.cjs | 1 - tests/artifacts.test.cjs | 1 - tests/claude-skills-migration.test.cjs | 1 - tests/codebase-mapper-date-restamp.test.cjs | 1 - tests/concurrency-safety.test.cjs | 1 - tests/config-defaults-runtime-exclusion.test.cjs | 1 - tests/coverage-uat-routing.test.cjs | 1 - tests/discuss-all-flag.test.cjs | 1 - tests/gsd2-import.test.cjs | 1 - tests/installer-migration-report.test.cjs | 1 - tests/kimi-variant-disambiguation.test.cjs | 1 - tests/legacy-cleanup.test.cjs | 1 - tests/lockfile-cve-audit.test.cjs | 1 - tests/orphan-worktree-detection.test.cjs | 1 - tests/phase-completion-single-owner.test.cjs | 1 - tests/phase-dependency-levels.test.cjs | 1 - tests/prohibition-probe.schema.test.cjs | 1 - tests/prohibition-probe.verify-tier.test.cjs | 1 - tests/quick-session-management.test.cjs | 1 - tests/review-default-reviewers-workflow.test.cjs | 4 ---- tests/spike-manifest-scoping.test.cjs | 1 - tests/thread-session-management.test.cjs | 1 - 24 files changed, 1 insertion(+), 37 deletions(-) diff --git a/scripts/lint-allow-test-rule-refs.allowlist.json b/scripts/lint-allow-test-rule-refs.allowlist.json index 0376ff9a5..1db215e85 100644 --- a/scripts/lint-allow-test-rule-refs.allowlist.json +++ b/scripts/lint-allow-test-rule-refs.allowlist.json @@ -2,7 +2,6 @@ "tests/agent-classification-parity.test.cjs :: source-text-is-the-product", "tests/agent-frontmatter.test.cjs :: source-text-is-the-product", "tests/agent-required-reading-consistency.test.cjs :: source-text-is-the-product", - "tests/agent-size-budget.test.cjs :: source-text-is-the-product", "tests/agent-skills-awareness.test.cjs :: source-text-is-the-product", "tests/ai-evals.test.cjs :: source-text-is-the-product", "tests/analyze-dependencies.test.cjs :: source-text-is-the-product", @@ -18,7 +17,6 @@ "tests/changeset-cli.test.cjs :: source-text-is-the-product", "tests/check-update-config-dir.test.cjs :: structural-regression-guard", "tests/claude-md.test.cjs :: source-text-is-the-product", - "tests/claude-skills-migration.test.cjs :: source-text-is-the-product", "tests/cleanup-branch-pruning.test.cjs :: source-text-is-the-product", "tests/cline-install.test.cjs :: source-text-is-the-product", "tests/cline-support.test.cjs :: source-text-is-the-product", @@ -30,7 +28,6 @@ "tests/codex-config.test.cjs :: source-text-is-the-product", "tests/command-contract.test.cjs :: source-text-is-the-product", "tests/commands.test.cjs :: source-text-is-the-product", - "tests/concurrency-safety.test.cjs :: source-text-is-the-product", "tests/config-field-docs.test.cjs :: docs-parity", "tests/context-enrichment.test.cjs :: source-text-is-the-product", "tests/contributor-standards.test.cjs :: source-text-is-the-product", @@ -38,7 +35,6 @@ "tests/cursor-hooks.test.cjs :: source-text-is-the-product", "tests/cursor-reviewer.test.cjs :: source-text-is-the-product", "tests/debug-session-management.test.cjs :: source-text-is-the-product", - "tests/discuss-all-flag.test.cjs :: source-text-is-the-product", "tests/discuss-checkpoint.test.cjs :: source-text-is-the-product", "tests/discuss-mode.test.cjs :: structural-implementation-guard", "tests/discuss-phase-power.test.cjs :: source-text-is-the-product", @@ -65,7 +61,6 @@ "tests/gsd-researcher-app-aware.test.cjs :: source-text-is-the-product", "tests/gsd-researcher-flow-diagram.test.cjs :: source-text-is-the-product", "tests/gsd-settings-advanced.test.cjs :: source-text-is-the-product", - "tests/gsd2-import.test.cjs :: source-text-is-the-product", "tests/hermes-skills-migration.test.cjs :: source-text-is-the-product", "tests/import-command.test.cjs :: source-text-is-the-product", "tests/ingest-docs.test.cjs :: source-text-is-the-product", @@ -77,7 +72,6 @@ "tests/intel.test.cjs :: source-text-is-the-product", "tests/inventory-headings-countfree.test.cjs :: source-text-is-the-product", "tests/ios-scaffold-safety.test.cjs :: source-text-is-the-product", - "tests/legacy-cleanup.test.cjs :: integration-test-input", "tests/locking-bugs-1909-1916-1925-1927.test.cjs :: architectural-invariant", "tests/mcp-tool-inheritance.test.cjs :: source-text-is-the-product", "tests/milestone-summary.test.cjs :: source-text-is-the-product", @@ -87,12 +81,10 @@ "tests/next-up-clear-order.test.cjs :: source-text-is-the-product", "tests/no-hardcoded-home-gsd-tools.test.cjs :: source-text-is-the-product", "tests/opencode-permissions.test.cjs :: architectural-invariant", - "tests/orphan-worktree-detection.test.cjs :: architectural-invariant", "tests/orphaned-hooks.test.cjs :: structural-regression-guard", "tests/package-legitimacy-gate.test.cjs :: source-text-is-the-product", "tests/parallel-dependent-plans.test.cjs :: source-text-is-the-product", "tests/path-replacement.test.cjs :: source-text-is-the-product", - "tests/phase-dependency-levels.test.cjs :: source-text-is-the-product", "tests/phase.test.cjs :: source-text-is-the-product", "tests/phase6-capability-docs.test.cjs :: source-text-is-the-product", "tests/phase6-capstone-conformance.test.cjs :: source-text-is-the-product", @@ -110,7 +102,6 @@ "tests/profile-output.test.cjs :: source-text-is-the-product", "tests/progress-forensic.test.cjs :: source-text-is-the-product", "tests/prompt-thinning.test.cjs :: source-text-is-the-product", - "tests/quick-session-management.test.cjs :: source-text-is-the-product", "tests/qwen-skills-migration.test.cjs :: source-text-is-the-product", "tests/read-guard.test.cjs :: source-text-is-the-product", "tests/reapply-patches.test.cjs :: source-text-is-the-product", @@ -119,7 +110,6 @@ "tests/release-tarball-smoke-workflow.test.cjs :: source-text-is-the-product", "tests/release-tarball-smoke.install.test.cjs :: integration-test-input", "tests/research-agent-profiles.test.cjs :: source-text-is-the-product", - "tests/review-default-reviewers-workflow.test.cjs :: source-text-is-the-product", "tests/roadmap.test.cjs :: source-text-is-the-product", "tests/runtime-launcher-parity.test.cjs :: structural-regression-guard", "tests/runtime-name-policy.test.cjs :: source-text-is-the-product", @@ -137,7 +127,6 @@ "tests/subagent-timeout.test.cjs :: source-text-is-the-product", "tests/template.test.cjs :: source-text-is-the-product", "tests/thinking-partner.test.cjs :: source-text-is-the-product", - "tests/thread-session-management.test.cjs :: source-text-is-the-product", "tests/ultraplan-phase.test.cjs :: source-text-is-the-product", "tests/verify-health.test.cjs :: source-text-is-the-product", "tests/verify-test-quality.test.cjs :: source-text-is-the-product", diff --git a/scripts/lint-allow-test-rule-refs.ceiling.json b/scripts/lint-allow-test-rule-refs.ceiling.json index d9ef39660..f2371f869 100644 --- a/scripts/lint-allow-test-rule-refs.ceiling.json +++ b/scripts/lint-allow-test-rule-refs.ceiling.json @@ -1,4 +1,4 @@ { - "maxFiles": 305, + "maxFiles": 285, "grace": 3 } diff --git a/tests/agent-size-budget.test.cjs b/tests/agent-size-budget.test.cjs index 580d27f03..18f200306 100644 --- a/tests/agent-size-budget.test.cjs +++ b/tests/agent-size-budget.test.cjs @@ -1,4 +1,3 @@ -// 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. // Per CONTRIBUTING.md exception matrix. diff --git a/tests/artifacts.test.cjs b/tests/artifacts.test.cjs index 4f048e3ad..053c46ba2 100644 --- a/tests/artifacts.test.cjs +++ b/tests/artifacts.test.cjs @@ -97,7 +97,6 @@ describe('isCanonicalPlanningFile', () => { __foldDescribe("folded:enh-2448-artifact-registry (consolidation epic #1969 B3 #1972)", () => { 'use strict'; -// allow-test-rule: source-text-is-the-product (see #2448) // 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/claude-skills-migration.test.cjs b/tests/claude-skills-migration.test.cjs index e1573fed5..3ff5afac0 100644 --- a/tests/claude-skills-migration.test.cjs +++ b/tests/claude-skills-migration.test.cjs @@ -1,4 +1,3 @@ -// 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/codebase-mapper-date-restamp.test.cjs b/tests/codebase-mapper-date-restamp.test.cjs index 61eb9300c..e07320894 100644 --- a/tests/codebase-mapper-date-restamp.test.cjs +++ b/tests/codebase-mapper-date-restamp.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (see #2279) // The gsd-codebase-mapper agent and map-codebase workflow .md files ARE the // contract the model loads at runtime. Regression lock for #2279: on an Update // run the agent must restamp the codebase-doc dates unconditionally, not merely diff --git a/tests/concurrency-safety.test.cjs b/tests/concurrency-safety.test.cjs index a4e48a3fc..a9167c449 100644 --- a/tests/concurrency-safety.test.cjs +++ b/tests/concurrency-safety.test.cjs @@ -1,4 +1,3 @@ -// 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/config-defaults-runtime-exclusion.test.cjs b/tests/config-defaults-runtime-exclusion.test.cjs index 22f815546..bf9e63a2a 100644 --- a/tests/config-defaults-runtime-exclusion.test.cjs +++ b/tests/config-defaults-runtime-exclusion.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (see #2840) // ~/.gsd/defaults.json is machine-wide. The `runtime` key is host-specific // (written by whichever installer ran last). It must NOT be copied into project // configs — on a machine with 2+ runtimes, it poisons every new project. diff --git a/tests/coverage-uat-routing.test.cjs b/tests/coverage-uat-routing.test.cjs index 84dbfe2c1..d46b9b2ef 100644 --- a/tests/coverage-uat-routing.test.cjs +++ b/tests/coverage-uat-routing.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (see #1602) // verify-work.md / execute-plan.md / summary*.md are workflow & template text the // runtime loads and executes. Asserting that they wire the deterministic coverage // classifier (and preserve the legacy prose fall-through) tests the deployed diff --git a/tests/discuss-all-flag.test.cjs b/tests/discuss-all-flag.test.cjs index 495f32ee4..5d78fbf97 100644 --- a/tests/discuss-all-flag.test.cjs +++ b/tests/discuss-all-flag.test.cjs @@ -1,4 +1,3 @@ -// 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/gsd2-import.test.cjs b/tests/gsd2-import.test.cjs index 7cd04bbb7..20a4910d1 100644 --- a/tests/gsd2-import.test.cjs +++ b/tests/gsd2-import.test.cjs @@ -1,6 +1,5 @@ 'use strict'; -// 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/installer-migration-report.test.cjs b/tests/installer-migration-report.test.cjs index c726515f8..ab4cb5081 100644 --- a/tests/installer-migration-report.test.cjs +++ b/tests/installer-migration-report.test.cjs @@ -623,7 +623,6 @@ describe('bug #3610: fresh upgrade with leftover bundled hooks does not throw', { const { describe: __foldDescribe } = require('node:test'); __foldDescribe("folded:bug-3628-bundled-hook-classifier-whitelist (consolidation epic #1969 B5 #1974)", () => { -// allow-test-rule: architectural-invariant (see #3628) // classifyPromptUserAction returns a typed result object; this test asserts // on that typed surface (category + choice fields) for both the positive // (shipped) and negative (user-owned / retired) cases. There is no rendered diff --git a/tests/kimi-variant-disambiguation.test.cjs b/tests/kimi-variant-disambiguation.test.cjs index 37240e8c0..b3e813925 100644 --- a/tests/kimi-variant-disambiguation.test.cjs +++ b/tests/kimi-variant-disambiguation.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: pending-migration-to-typed-ir [#3090] // Phase 5 kimi-variant disambiguation is verified via install.js subprocess // output capture (regex on printed notices), since disambiguateKimiVariant // is inline in bin/install.js (not exported) and emits no structured/JSON diff --git a/tests/legacy-cleanup.test.cjs b/tests/legacy-cleanup.test.cjs index 7a7680ef7..5f98a9960 100644 --- a/tests/legacy-cleanup.test.cjs +++ b/tests/legacy-cleanup.test.cjs @@ -7,7 +7,6 @@ * * These tests read files they create themselves in OS temp directories — * not repo source files. The fs reads are test-input reads, not source-grep. - * // allow-test-rule: integration-test-input */ 'use strict'; diff --git a/tests/lockfile-cve-audit.test.cjs b/tests/lockfile-cve-audit.test.cjs index d4ac5cea9..558797cf2 100644 --- a/tests/lockfile-cve-audit.test.cjs +++ b/tests/lockfile-cve-audit.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: structural-implementation-guard (#2765, #3238) 'use strict'; // Regression guard for #2765: the lockfile must pin the patched brace-expansion diff --git a/tests/orphan-worktree-detection.test.cjs b/tests/orphan-worktree-detection.test.cjs index 4b2cfda65..6562669e8 100644 --- a/tests/orphan-worktree-detection.test.cjs +++ b/tests/orphan-worktree-detection.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: architectural-invariant // Structural checks verify the health seam exports worktree inspection capability. // Behavioral tests cover detection flow via validate health output. diff --git a/tests/phase-completion-single-owner.test.cjs b/tests/phase-completion-single-owner.test.cjs index 149de593a..6dea95351 100644 --- a/tests/phase-completion-single-owner.test.cjs +++ b/tests/phase-completion-single-owner.test.cjs @@ -1,7 +1,6 @@ 'use strict'; process.env.GSD_TEST_MODE = '1'; -// allow-test-rule: source-text-is-the-product, see #3186 // F2 below reads gsd-core/workflows/mvp-phase.md and regex-tests its shell // content for the disk-strict OR removal (Decision 4(d)). A workflow .md // file's text IS the deployed prompt-layer artifact the runtime executes — diff --git a/tests/phase-dependency-levels.test.cjs b/tests/phase-dependency-levels.test.cjs index 0c3e0f1c9..8d214fca9 100644 --- a/tests/phase-dependency-levels.test.cjs +++ b/tests/phase-dependency-levels.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product // Tests for the extracted computeDependencyLevels pure function in phase.cjs. // Covers correctness (behavior) and edge cases. The O(V+E) complexity contract // is documented inline in computeDependencyLevels (phase.cjs) above the head-index diff --git a/tests/prohibition-probe.schema.test.cjs b/tests/prohibition-probe.schema.test.cjs index bef484a14..bb31c1509 100644 --- a/tests/prohibition-probe.schema.test.cjs +++ b/tests/prohibition-probe.schema.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (see #644) — the must_haves.prohibitions: block is the // runtime plan-contract surface; this pins its parse/round-trip/projection bijection to the code. // // RED-first schema contract for the `must_haves.prohibitions:` SIBLING block (ADR-550 Decision 3 — diff --git a/tests/prohibition-probe.verify-tier.test.cjs b/tests/prohibition-probe.verify-tier.test.cjs index ed38c57a0..bd728d1bf 100644 --- a/tests/prohibition-probe.verify-tier.test.cjs +++ b/tests/prohibition-probe.verify-tier.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (see #644) — the verify-time disposition of a test-tier // prohibition is the deployed safety contract; this pins its fail-closed default to the code. // // RED-first SAFETY HALF of ADR-550 Decision 5(d) [maintainer decision 2026-06-12 "B-with-guard"]. diff --git a/tests/quick-session-management.test.cjs b/tests/quick-session-management.test.cjs index 5fddfe151..dde48f06a 100644 --- a/tests/quick-session-management.test.cjs +++ b/tests/quick-session-management.test.cjs @@ -1,6 +1,5 @@ 'use strict'; -// 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/review-default-reviewers-workflow.test.cjs b/tests/review-default-reviewers-workflow.test.cjs index daada6b45..ce7df0884 100644 --- a/tests/review-default-reviewers-workflow.test.cjs +++ b/tests/review-default-reviewers-workflow.test.cjs @@ -1,6 +1,5 @@ 'use strict'; -// allow-test-rule: source-text-is-the-product // Workflow markdown is runtime contract; these assertions verify deployed behavior text. const { describe, test } = require('node:test'); @@ -158,7 +157,6 @@ describe('review workflow source-grounding requirement in build_prompt (#1318)', { const { describe: __foldDescribe } = require('node:test'); __foldDescribe("folded:bug-687-agy-timeout (consolidation epic #1969 B4 #1973)", () => { -// allow-test-rule: source-text-is-the-product (see #687) // review.md is a workflow file whose deployed text IS the runtime contract; the // agy -p invocation cannot be run in CI, so we assert on its content (issue #687). 'use strict'; @@ -247,7 +245,6 @@ describe('bug #687 → #2073: agy print mode is bounded, and its fallback chain __foldDescribe("folded:enh-773-codex-exec-automation-flags (consolidation epic #1969 B4 #1973)", () => { 'use strict'; -// allow-test-rule: source-text-is-the-product (see #773) // Workflow markdown is runtime contract; these assertions verify that // automated codex exec invocations carry the correct automation flags. @@ -333,7 +330,6 @@ describe('#1698 regression: codex review is captured via --output-last-message, __foldDescribe('#1936: OpenCode reviewer empty-output hardening', () => { 'use strict'; -// allow-test-rule: source-text-is-the-product (see #1936) // review.md is a workflow file whose embedded bash IS the runtime contract; the // `opencode run` invocation on a large agentic prompt cannot be run in CI, so we // assert on its content. diff --git a/tests/spike-manifest-scoping.test.cjs b/tests/spike-manifest-scoping.test.cjs index a96d874b3..a89437dad 100644 --- a/tests/spike-manifest-scoping.test.cjs +++ b/tests/spike-manifest-scoping.test.cjs @@ -1,4 +1,3 @@ -// allow-test-rule: source-text-is-the-product (#1700) // spike.md / spike-wrap-up.md / artifact-types.md / sketch.md are workflow and reference // markdown — their text IS the contract loaded by the agent at runtime. Testing text content // tests the deployed contract; there is no build step, CLI entrypoint, or generated code to diff --git a/tests/thread-session-management.test.cjs b/tests/thread-session-management.test.cjs index 8c310c4be..27918709e 100644 --- a/tests/thread-session-management.test.cjs +++ b/tests/thread-session-management.test.cjs @@ -1,6 +1,5 @@ 'use strict'; -// 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.