fix(test-timeouts): size the spawn-heavy hook suites to what they actually do - #1363
Merged
Conversation
…ually do `packages/core/hooks/getff-work.test.ts` was flaky/red on the vitest 5000ms default: every case spawns a real `bash scripts/getff-work.sh`, which does `git worktree add` + node_modules wiring + a package-manager install inside the new worktree. On clean staging c1848e8 it ran 4 failed / 3 passed, all four `Test timed out in 5000ms`. In CI it was marginal — CC-DEFERRAL failed on PR #1359's first run and went green on re-run, having already blocked one unrelated comment-only PR. Timing the wrapper directly on a cold worktree measured 30.8s total (dominant leg: the step-2 `npm ci` inside the new worktree), so the 30_000 SLOW_SHELL_MS value the sibling suites use is itself too tight here. getff-work gets 60_000 — ~2x the measured cold cost, with precedent in principles/20-bundle-classification .paired-negative.test.ts and the root vitest.config.ts, and well under that file's documented 120_000 ceiling. Its inner spawnSync guard is named SPAWN_GUARD_MS so the "test timeout must outlive the spawn guard" inequality is visible. A full-suite sweep (`vitest run hooks/ skills/`) showed the same defect class across the siblings: 70 failed, 69 of them `timed out in 5000ms`. Fixed at 30_000 (the existing SLOW_SHELL_MS convention): hooks/create-worktree.test.ts, hooks/worktree-setup.test.ts, hooks/dup-detect-empty-arg.test.ts, hooks/priority-score-branch-matcher.test.ts, hooks/priority-score-reconstruct-stub.test.ts, skills/dup-detect.test.ts, skills/dup-detect-skip-closed.test.ts plus two CI-gated files that carried only PARTIAL per-test coverage — the untimed cases (C1/C2 tier-resolution; the fake-SHA paired-negative) were the ones going red, and `test:hooks` does not go green under load without them: hooks/validate-prompt.test.ts, hooks/check-worker-dispatch-channel.test.ts Verification (clean tree, macOS, 2026-08-10): - getff-work.test.ts alone: 7 passed (7), slowest 32.5s - `vitest run hooks/ skills/`: hooks/ failures 15 -> 0; total 70 -> 38 - `tsc --noEmit`: clean Timeout configuration only — no assertion, test-body, or `it()` line changed; `git diff` contains zero touched expect/toBe/toMatch/toContain lines. Known residue, deliberately NOT in this diff (no drive-by per CLAUDE.md «PR strategy»): the 38 remaining timeouts live in four skills/ files outside the audited set — planner-discovery (12), plan-currency-check (11), -digest (6), -kickoff (5), run-helper (4). They are the same defect class, but skills/ is gated by no workflow (CI runs test:hooks / test:principles / ir / backends / composition / live-generation / render), so they block no PR. Prior-art: skipped — test timeout configuration only, no new capability, no new dependency and no new module; the values reuse the SLOW_SHELL_MS convention that already exists in this package.
artyhoo
enabled auto-merge (squash)
August 10, 2026 09:07
artyhoo
pushed a commit
that referenced
this pull request
Aug 10, 2026
…245) Merge-forward per .claude/rules/git-conflict-merge-forward.md (rebase / force-push is a dead end on a published branch). Sole conflict: this branch and #1362 both appended an SSOT row numbered 244. Resolution — staging's #1362 row KEEPS 244 (already merged, its own trailer cites that ID); this branch's GNU-make-resolver row is renumbered to 245 and ordered after it. Verified post-resolve: no conflict markers, no duplicate IDs across the table, sequence ends 243 · 244 · 245, markdownlint 0 errors, and the diff against staging is a single added line (the first resolve attempt also ate the blank line before the closing `---`, which turned the whole table into a setext heading — MD003 at :186; restored). CORRECTION to 4e4231a's trailer: it cites `prior-art-evaluations.md#244`, which after this merge is #1362's actionlint row. The correct citation for principle 36 is **#245**. Principle 11 F1 still resolves via the verbatim SSOT path match (row 245's "Capability matched" cell names packages/core/principles/36-make-target-claim-liveness.test.ts), and the squash body at merge time carries #245. Two staging commits landed mid-branch and each retires something this PR asserted: - #1362 wired `shipped-prettier` + `framework-fresh-install-validate-multistack` into `ci-success` `needs:` and gated it — exactly the GAP-FOUND in this PR's §1.7 Backward-check surface 2. Now CLOSED upstream; the PR body is updated so it does not go on asserting a defect that no longer exists. - #1363 sized the spawn-heavy hook suites to their real runtime — the local `test:hooks` flake this branch diagnosed (12 failures, all "timed out in 5000ms", all under hooks/; the same file passes 5/5 in 22.76s under --no-file-parallelism --testTimeout=30000). Note for the reviewer: #1362 independently BUILT a needs-completeness gate for the sibling surface while this PR built the make-target-claim gate. Two instances of one class — "an artifact asserts a falsifiable property about a channel it does not own" — were gated within an hour of each other, by different sessions, neither aware of the other. That is corroboration that the class is real, not a one-off. Prior-art: prior-art-evaluations.md#245 (GNU make `-n` as target-recipe resolver + the absence of any tool verifying a script's prose claim about which build target invokes it — HYBRID: ADOPT the resolver, BUILD only the claim-extraction slice).
artyhoo
pushed a commit
that referenced
this pull request
Aug 10, 2026
…ger, and gate registrability Completes the half #1368 declared and deferred. That PR marked `discipline-self-check.yml`'s job `# required-context: no` and named the exact precondition in the marker: «To make it required, the path filter must first move from `on.pull_request.paths` into the job body … so the context always reports.» Operator ruling 2026-08-10 requires this context on staging, so the precondition had to be met. Met by the stronger route: the `paths:` filter is REMOVED, not reimplemented in the job body. §1.7 already has a declared out-of-scope mechanism — the `### §1.7 Skipped: <≥60 chars>` marker, which pull_request_template.md:61 emits as the DEFAULT for every PR — so a second scope list in the job body would have been a third hand-maintained copy of «what counts as discipline-bearing», in a PR whose whole subject is that such copies drift. Measured before removing it: of the 40 most recent merged base=staging PRs, 38 already carry a valid §1.7 section (11 Skipped markers, 27 Forward/Backward pairs); #1363 and #1337 are the only two that would newly redden, each needing the one template-default line. Also adds `if: base.ref != 'main'` — a promote PR aggregates ~90 squashed commits and has no single meaningful forward/backward pair, so today it gets a guaranteed RED. Deliberately not the siblings' `== 'staging'`: theirs is subject-scoped (a kickoff's fidelity verdict, a stale-base rebuild against staging), whereas §1.7 also applies to the base=epic/* PRs CLAUDE.md:121 permits agents to merge, and `== 'staging'` would silently drop that coverage. A job-level `if:` is safe where a trigger filter is not — «if a job is skipped due to a conditional, it reports success». Both derived lists gain the newly-declared context. The sweep's REGISTRATION STATE line is rewritten to name contexts instead of «the last three» / «the first two» — appending a sixth entry above it had silently falsified both phrases, which is the same positional-drift shape principle 37 exists to catch. The gate, arms (i)-(k). Principle 37's existing arms assert the declared set is restated consistently; none asserts a declared entry is safe to register at all. `hasPullRequestTrigger` checks only that a `pull_request:` trigger EXISTS, never that it is unfiltered — so flipping a marker to `yes` inside a `paths:`-filtered workflow passes every existing arm and then freezes every PR matching none of the filter. That is precisely what #1368 wrote in prose and deferred, and prose is not a mechanism (attention-is-not-a-mechanism.md §1). Arm (i) asserts it, (j) is the paired negative (re-add the removed filter → RED; also run on disk against the real tree, not only in-parser), (k) pins the parser's scope: `pull_request:` only (workflow-integrity.yml filters both push: and pull_request:), inline `on:` forms resolve to [] rather than a false «no trigger» RED, and job-level `if:` is never flagged. Added to principle 37 rather than shipped as a parallel principle: that file already owns the declaration model and its parsers, so a second principle would have had to duplicate them (`#sync-by-copy-paste`). Prior-art: prior-art-evaluations.md#247 (actionlint RuleGlob / zizmor / poseidon/wait-for-status-checks / blend/require-conditional-status-checks / GitHub-native required_status_checks API — verdict BUILD, narrow complement to #245. actionlint validates `paths:` glob SYNTAX but per DeepWiki on rhysd/actionlint, 2026-08-10, «does not analyze the implications for branch protection»; the marketplace answers to skipped-but-required are RUNTIME waiters that add a job to wait on other jobs, never a static assertion that a declaration is registrable.)
artyhoo
added a commit
that referenced
this pull request
Aug 10, 2026
…ger, and gate registrability (#1371) ## Summary Completes the half #1368 declared and explicitly deferred, and closes the gap that deferral left in the gate. #1368 (merged 10:26Z today) built the declaration model: every job declares `# required-context: yes|no` at the job, and principle 37 asserts both in-repo lists equal the declared set. It marked `discipline-self-check.yml`'s job `no`, naming the precondition in the marker itself: «To make it required, the path filter must first move from `on.pull_request.paths` into the job body … so the context always reports.» The 2026-08-10 operator ruling requires that context on `staging`, so the precondition had to be met. Two things here: the §1.7 context becomes registrable and is declared required, and principle 37 gains the arm that makes «declared required ⇒ actually reportable» a mechanism instead of a comment. **Scope note, stated plainly:** this branch was opened before #1368 existed (my in-flight probe ran at 09:33Z; #1368 was created at 09:39Z) and originally carried its own parallel list-gating design. That work is discarded — #1368's declare-at-the-job model is better, and this PR is rebased onto it. The net diff is 5 files. ## Changes **1. `discipline-self-check.yml` — registrable, and declared required.** The `paths:` trigger filter is removed, and the marker flips to `# required-context: yes`. GitHub's guidance is the reason the filter and the requirement are incompatible: «Workflows skipped due to path filtering, branch filtering, or commit messages will remain in a pending state and block merging. To avoid this, do not require workflows that can be skipped.» Removed rather than reimplemented in the job body. §1.7 already has a declared out-of-scope mechanism — the `### §1.7 Skipped: <≥60 chars>` marker, which `.github/pull_request_template.md:61` emits as the DEFAULT for every PR. A scope list inside the job body would have been a third hand-maintained copy of «what counts as discipline-bearing», in a PR whose subject is that such copies drift. **Measured before removing it:** of the 40 most recent merged base=staging PRs, 38 already carry a valid §1.7 section (11 Skipped markers, 27 Forward/Backward pairs). #1363 and #1337 are the only two that would newly redden, each needing the one template-default line. Also adds `if: base.ref != 'main'`: a promote PR aggregates ~90 squashed commits and has no single meaningful forward/backward pair, so today it gets a guaranteed RED. Deliberately not the siblings' `== 'staging'` — theirs is subject-scoped (a kickoff's fidelity verdict, a stale-base rebuild against staging), whereas §1.7 also applies to the base=`epic/*` PRs `CLAUDE.md:121` permits agents to merge, and `== 'staging'` would have silently dropped that coverage. Safe because a job skipped by `if:` reports `skipped`, which protection accepts — «if a job is skipped due to a conditional, it reports success». **2. Both derived lists gain the newly-declared context**, per #1368's rule that the three move together. The sweep's `REGISTRATION STATE` line is also rewritten to name contexts instead of «the last three» / «the first two»: appending a sixth entry above it silently falsified both phrases — the same positional-drift shape principle 37 exists to catch. **3. Principle 37 arms (i)–(k) — registrability.** The existing arms assert the declared set is *restated consistently*. None asserts a declared entry is *safe to register at all*: `hasPullRequestTrigger` (`:112`) checks only that a `pull_request:` trigger EXISTS, never that it is unfiltered. So flipping any marker to `yes` inside a `paths:`-filtered workflow passes every existing arm and then freezes every PR matching none of the filter. That is exactly the hazard #1368 wrote into the `discipline-self-check.yml` marker and deferred — and prose is not a mechanism (`.claude/rules/attention-is-not-a-mechanism.md:1`). - **(i)** every `required-context: yes` job sits behind an unfiltered `pull_request:` trigger (no trigger at all is equally an offender). - **(j)** paired negative — re-adding the removed filter must be detected. - **(k)** parser scope: `pull_request:` only (`workflow-integrity.yml` filters *both* `push:` and `pull_request:`, so a file-wide scan would look identical here); inline `on: [push, pull_request]` resolves to `[]` rather than a false «no trigger» RED; job-level `if:` is never flagged, because flagging it would forbid the promote-flow guards all three PR-body gates rely on. Added to principle 37 rather than shipped as a parallel principle: that file already owns the declaration model and its parsers, so a second principle would have had to duplicate them. ## Operator action required **Order matters — register only AFTER this merges.** `discipline-self-check.yml` on `staging` is still `paths:`-filtered until then; registering its context first deadlocks every non-matching PR. Live state re-read 2026-08-10: `{"strict":false,"checks":[{"app_id":15368,"context":"ci-success"},{"app_id":15368,"context":"fidelity-verdict-in-pr-body"},{"app_id":15368,"context":"stale-revert-in-pr-diff"}]}` — three registered against six now declared. ```bash gh api -X PATCH repos/artyhoo/getff/branches/staging/protection/required_status_checks --input - <<'JSON' { "strict": false, "checks": [ {"context": "Template render probes — P1/P4/P6 (deterministic)", "app_id": 15368}, {"context": "capability PR carries Prior-art line in PR body (squash-survival)", "app_id": 15368}, {"context": "ci-success", "app_id": 15368}, {"context": "fidelity-verdict-in-pr-body", "app_id": 15368}, {"context": "stale-revert-in-pr-diff", "app_id": 15368}, {"context": "§1.7 forward+backward sections present in PR description", "app_id": 15368} ] } JSON ``` - Payload generated from the declared set, not retyped; `strict: false` + `app_id: 15368` reproduce the current pinning. Passing bare `contexts[]=` would drop the app pinning. - This registers all six declared contexts, including the two #1368 left declared-but-unregistered. Register a subset if you want them staged separately — but every unregistered declared context is one that can go red while the PR merges. - **PRs open at registration time will not report the new checks until they get a fresh PR event** (a push or a body edit — all the relevant workflows carry `types: [… edited …]`). - Verify: `gh api repos/artyhoo/getff/branches/staging/protection --jq '.required_status_checks.contexts[]'` → 6 lines. **Effect on the 6 currently-open PRs, measured (`gh pr list --state open`):** #1366 carries a valid §1.7 section. The other five — #1331 plus four Dependabot bumps (#1230, #1217, #1216, #1149) — do not, and will show a red §1.7 once this merges. This does **not** create a new blocked class: those four are *already* blocked by an existing required check — `gh pr checks 1230` shows `fidelity-verdict-in-pr-body fail` today, because Dependabot's body carries no `## Fidelity verdict` either, and Dependabot cannot edit its own body. Whatever already handles that for these PRs handles this too. Worth an explicit decision eventually; out of scope here. ## Test plan - [x] `npx vitest run packages/core/principles/37-required-context-completeness.test.ts` → 11/11 (8 pre-existing + 3 new). - [x] **Paired-negative on disk, not just in-parser.** The `paths:` filter was re-added to the real `discipline-self-check.yml` and the suite re-run: arms (i) and (j) both RED, with the deadlock message naming the job. Restored → 11/11 green. - [x] `npm --prefix packages/core run test:principles` → 40 files, 381 passed, 1 skipped, 0 failed. - [x] `bash scripts/run-local-ci-sweep.test.sh` → ALL PASS (the sweep's own paired-negative, after editing its header block). - [x] `actionlint` on both changed workflows → clean; `bash -n scripts/run-local-ci-sweep.sh` → clean; `npm run format:check` → clean. - [x] Full `bash scripts/run-local-ci-sweep.sh` on the pre-merge tree: PASS through `vitest-composition`; `format-check`, `rule-index-check`, `alwayson-budget`, `test:render`, `test:ir` also green individually. - [x] Live branch protection re-read before writing the operator command. **Pre-existing failures on this host, NOT introduced here** (both reproduced on a clean `staging` checkout, so out of scope per CLAUDE.md «PR strategy»): - `packages/core/hooks/getff-work.test.ts` — 3 failures (`CC-DEFERRAL`, `NON-CC-PRINT`, `NO-LAUNCH-FLAG`, all `expected 1 to be +0`). Verified identical on `/Users/art/code/rules-as-tests-aif` at `staging`. - `packages/core/principles/20-bundle-classification.test.ts` — one 30 s timeout under full-suite parallel load; passes in isolation in 45 s (~3.3 s/test), and green in the 40-file run above. Same spawn-heavy class #1363 addressed. ## Provenance n/a — not a stage PR. Direct implementation of the 2026-08-10 operator ruling, rebased onto #1368 after that PR merged mid-flight. ## Review findings n/a — not a stage PR. Three findings came from this branch's own pre-handoff self-review and were fixed before handoff: the `!= 'main'` guard scoping (`== 'staging'` would have dropped epic/* coverage); the inline `on: [push, pull_request]` grammar in the arm-(i) parser (an unhandled inline form would have false-REDded the one unconditionally safe shape); and the sweep's positional «last three» claim, which this PR's own sixth list entry falsified. **Also surfaced, not fixed** (no drive-by, per CLAUDE.md «PR strategy»): `origin/staging` currently carries **two** principle-37 files — `37-make-target-claim-liveness.test.ts` (#1366) and `37-required-context-completeness.test.ts` (#1368) — merged within minutes of each other. Nothing asserts principle-slot uniqueness, which is the same missing-mechanism class as everything else in this PR. ## Fidelity verdict FIDELITY: skipped — no kickoff or spec governs this change; it implements a direct operator ruling recorded in the task brief, not a stage of a planned umbrella. ## Parked questions n/a — not a stage PR. ## §1.7 Self-discipline check (REQUIRED if PR touches discipline-bearing files) ### §1.7 Forward-check applied **`no-paid-llm-in-ci.md §2`** — arms (i)–(k) are `readFileSync` + regex, zero API-billed calls; the workflow edits remove a trigger filter and add no step (`packages/core/principles/37-required-context-completeness.test.ts:71` imports only `node:fs`/`node:path`/`node:url`). **`ci-tool-pinning.md §1`** — no bare `run:` tool install added or altered; `.github/workflows/discipline-self-check.yml:79` still has no install step. **`attention-is-not-a-mechanism.md §1`** — this is the rule's own case: the deferred fix lived as a prose marker comment on a job, i.e. detection resting on someone reading it; arm (i) replaces it with a deterministic gate, and the surviving best-effort job at `workflow-integrity.yml:32` stays explicitly labelled non-load-bearing. **`rule-enforcement-channel-selection.md §3`** — mechanically detectable → gate; a principle test is the earliest firing channel (`packages/core/hooks/pre-push.ts:1269` runs `test:principles` at pre-push, CI as backstop). **`build-first-reuse-default.md §3`** — DeepWiki `rhysd/actionlint` + context7 `/github/docs` ×3 phrasings + WebSearch ×2 ran before the BUILD verdict; recorded as SSOT #247 in `docs/meta-factory/prior-art-evaluations.md:319` and cited by the commit trailer. The same rule drove the *shape*: the arms were folded into the existing principle 37 rather than shipped as a parallel principle, reusing its declaration model and parsers instead of duplicating them. **`language-discipline.md §1`** — all added comments, test prose and commit bodies are English. **CLAUDE.md capability-commit gate** — the diff adds no new file ≥80 LOC and no dependency; the trailer cites the SSOT entry anyway, since the capability is real even though the artifact is an extension. ### §1.7 Backward-check applied Delegated cold to `agents/backward-sweep-auditor.md` per T21 — handed only the two class predicates, never the diff, branch or PR, so it could not restate this change. (Dispatched against the pre-merge tree; both classes are structural and unaffected by the rebase, and finding B-2 below is about files this PR does touch, so it was re-checked by hand against the merged state.) **Class A — «a hand-maintained list whose correct contents are determined by a population declared elsewhere, with nothing asserting the mirror».** Population 13, all reached. SWEPT-CLEAN: `packages/core/principles/27-*` (install.sh copy-list ⊇ import graph), `.claude/rules/00-rule-index.md` + AGENTS.md (`scripts/render-rule-index.mjs --check`, `packages/core/hooks/pre-push.ts:1253`), agents/rules/skills header coverage (`packages/core/principles/09-doc-authority-hierarchy.ts:228`), `tests/agnosticism` probes (`run-audit.sh:14` globs, no hand list), `.claude/hooks/**` channel markers. **Six GAP-FOUND, none in this diff:** (1) the Tier-0 core set exists in three copies — `packages/core/principles/31-rule-channel-declaration.ts:58` (3 entries) vs `scripts/render-rule-index.mjs:56` and `scripts/render-rule-channels.mjs:75` (2 each), the latter's comment at `:70` claiming a «verbatim» mirror while differing, and `--check` regenerates *from* the stale copy so it stays green; (2) `packages/core/principles/09-doc-authority-hierarchy.ts:28` `REQUIRED_HEADER_DOCS` is static, and 5 `docs/meta-factory/*.md` carry no header; (3) `tests/install-sh/meta-all-wired.test.sh:23` loops only `tests/install-sh/*.test.sh`, leaving 29 of 127 tracked `*.test.sh` unasserted, 13 invoked by nothing; (4) `setup.d/LAYERS.md:3` declares itself authoritative for the layer list but carries 13 rows against 16 globbed by `install.sh:1142`; (5) `setup.d/10-skills.sh:108` names 12 of 14 skills, in sync but unasserted; (6) `scripts/run-local-ci-sweep.sh:123` gate table (low severity — a stale row yields a false LOCAL green and gates no merge). **Class B — «a declared gate whose channel cannot fire where it is relied upon».** Population 9, all reached. SWEPT-CLEAN: `guard-liveness-fullsweep.yml:20`, `pr-body-prior-art.yml:28`, `audit-self.yml:549` lychee vacuity guard, `audit-self.yml:1058` toolchain non-vacuity, `scripts/host-verify-coverage.sh:4`. `workflow-integrity.yml`'s own `paths:` filter is clean precisely because it is declared `required-context: no` — which arm (i) now asserts rather than assumes. **Three GAP-FOUND:** (1) 113 of 262 `packages/core` test files execute at no channel — `packages/core/vitest.config.ts:9` allowlists 22 dirs, pre-push runs 4 `test:*` scripts and CI 7, so `validator/gate-conflict.test.ts` and six other self-described gates never run; `audit-self.yml:389` states the class in the repo's own words and `:394` defers it; (2) all three PR-body required gates skip on base=`epic/*` while `CLAUDE.md:121` permits agents to merge there; (3) changed-markdown link integrity is pre-push-only and degrades to a warning when lychee is absent (`packages/core/hooks/pre-push.ts:1499`), while `link-checker.yml:26` is paths-filtered to `lychee.toml`. **Acted on within scope:** B-2 is the one finding this PR's own files carry, and it is fixed here for the §1.7 gate — `discipline-self-check.yml:77` uses `!= 'main'`, so base=`epic/*` stays covered. The cold agent reached that defect independently, on the two sibling gates, without seeing the diff; the two siblings keep `== 'staging'` and are **not** changed here, since narrowing them is a different subject with a recorded prior decision (`docs/superpowers/specs/2026-07-23-acceptance-contour-design.md:161`). The other eight findings are sibling surfaces outside this umbrella and are surfaced, not fixed — the Tier-0 three-copy set is the closest structural twin of what principles 36 and 37 each fixed once. Prior-art: prior-art-evaluations.md#247 (actionlint RuleGlob / zizmor / poseidon/wait-for-status-checks / blend/require-conditional-status-checks / GitHub-native required_status_checks API — verdict BUILD, narrow complement to #245. actionlint validates `paths:` glob SYNTAX but per DeepWiki on `rhysd/actionlint`, 2026-08-10, «does not analyze the implications for branch protection» and «does not have a feature to cross-check a list of status-check context strings against job `name:` fields»; the marketplace answers to skipped-but-required are RUNTIME waiters that add a job to wait on other jobs, never a static assertion that a declaration is registrable.)
artyhoo
added a commit
that referenced
this pull request
Aug 10, 2026
…de-allowlist so it cannot drift again (#1370) * fix(ci-coverage): run the 87 unwired vitest tests, and gate the include-allowlist so it cannot drift again `packages/core/vitest.config.ts` `include:` is the real test population; the `principles-meta-tests` job invoked a HAND-WRITTEN list of npm scripts and paths. Nothing tied the two together, so the list fell behind: 87 `*.test.ts` files across 12 directories had ZERO CI invocation — skills 22, research 22, validator 16, eslint-rules 7, detector 7, installer 4, scenario-generator 2, diff 2, diagnostics 2, detector-v0 1, spec-validation 1, python-starter 1 — plus partial coverage of synthesizer (3/23 files), install (2/5) and audit-self (3/6). audit-self.yml already admitted the hole in prose («the install/ vitest dir is otherwise un-gated»); a comment is not a mechanism. Reproduced independently before acting: principle 38's own resolver, run against the pre-wiring tree, reported exactly those 12 directories at exactly those counts. WIRED (all green, run as CI runs them): test:units 905 passed / 39 skipped, 7s — audit-self detector detector-v0 diagnostics diff eslint-rules install installer python-starter research scenario-generator validator test:skills 211 passed, 39s — split out: every case spawns a real bash helper against a sandbox tree test:spec-validation 9 passed — split out: the only network suite (shells to `gh`); GH_TOKEN passed so assertions are real rather than uniformly rate-limited into the tolerated exit-2 branch test:synthesizer 186 passed / 23 files — supersedes the 3-file test:live-generation step DELIBERATELY EXCLUDED: none. COVERAGE_ALLOWLIST ships empty — every gap the sweep found is now wired. The allowlist exists for files that are genuinely not merge gates (toolchain CI lacks, manual probes); "slow" alone is not a reason, it earns its own step. Placement is load-bearing, and is what made the old scoping unnecessary: validator/gate-*.ts, install/rule-bootstrap-cli.ts and synthesizer/generate-react-spa.test.ts import `@rules-as-tests/preset-next-15-canonical`, a workspace sibling absent from packages/core's package.json AND its standalone lock. It resolves only after the root `npm install` links it into the repo-root node_modules. The old comment blamed scoping and called the fix "a separate task"; the fix was placement — the new steps run after that install. PRE-EXISTING FAILURES FOUND BY ACTUALLY RUNNING THEM (fixed here, timeout config only): 15 tests across 6 files failed on the vitest 5000ms default while the underlying script succeeded — the #1363 defect class, in the four skills/ files #1363 named as deliberate residue plus two it did not reach. Fixed at 30_000, the existing SLOW_SHELL_MS convention, applied per #1363's precedent: skills/plan-currency-check{,-digest,-kickoff}.test.ts, skills/planner-discovery.test.ts, skills/run-helper.test.ts, synthesizer/run-generated-rule-mutation-skip.test.ts, spec-validation/validate-batch-spec.test.ts (its 2 untimed --soft cases; the 3 integration siblings already carried 30_000 for the same `gh` round-trip) No assertion, test-body or `it()` line changed: `git diff` has zero touched expect/toBe/toMatch/toContain lines. THE MECHANISM: packages/core/principles/38-vitest-include-ci-coverage.test.ts. Per-FILE, not per-directory — a directory named by one step is not evidence its other 19 files run (the synthesizer 3/23 case). Runs at pre-push AND in CI, so it fails at the earliest reachable channel. Arm (d) is the paired-negative: it deletes the real test:skills wiring from the real workflow text and re-runs the real resolver, proving RED; arms (e)/(f) cover a new-unwired-dir and a fail-closed unparsable config. Filter semantics are modelled on the runner, not assumed: vitest positional args are SUBSTRING matches, and the trailing slash is load-bearing. Verified against the real binary — `vitest list hooks/` selects 0 hooks-tree-guard files, `vitest list hooks` selects 6. Normalising `hooks/`→`hooks` would have credited the hooks step with covering audit-self/hooks-tree-guard.test.ts: false coverage, the exact bug this gate exists to catch. Arm (c) pins it. Also wired into scripts/run-local-ci-sweep.sh: CI-only wiring would reproduce the same one-channel defect a layer up, and the sweep is the pre-push mirror. Its self-test passes. Prior-art: prior-art-evaluations.md#246 (Vitest `test.include`/`globTestFiles`/`collect`; orphan finders git-orphaned-files + dependency-cruiser --orphan; in-repo meta-all-wired.test.sh + principles 36/27 — verdict ADAPT). DeepWiki `vitest-dev/vitest`: «no explicit feature to reconcile the declared `include` globs with the files that are actually run»; `FilesNotFoundError` fires only when NO file is found. Orphan finders reason about the import graph, to which a test file is a root by construction. ADAPT not BUILD: this ports the meta-all-wired / principle-36 shape onto a third population/registry pair, zero new dependencies. * fix(ecosystem-cargo): make the traversal paired-negatives tmpdir-depth independent Wiring research/ into CI (this PR's point) immediately caught a real, host-dependent defect the suite had carried since it shipped: on ubuntu-latest, two of the cargo containment paired-negatives died with `EACCES: permission denied, mkdir '/etc/foo'` and `mkdir '/evil-member'`. Root cause is tmpdir depth, not the guard under test. `makeRoot()` put `root` directly at `mkdtempSync(os.tmpdir())`, and the traversal tests plant a real Cargo.toml at `root/../..` — the target a malicious `../../etc/foo` dep name or `members = ["../../evil-member"]` resolves to. os.tmpdir() depth is platform-dependent: macOS `/var/folders/<x>/<y>/T/` leaves that target writable (green locally), Linux `/tmp` makes it `/` (EACCES). The tests could never have passed on a Linux runner; nothing noticed because research/ ran at no CI channel. Fix: nest `root` two levels inside the sandbox (`<mkdtemp>/nested/root`), so every escape target stays inside a directory the test owns, on every platform. Intent preserved exactly — the planted manifest is still genuinely OUTSIDE `root`, so the containment guard is still the only reason these go green (.claude/rules/research-source-trust.md §5 item 2 — the BLOCKER paired-negatives). Non-vacuity re-proven after the change, not assumed: neutering `resolvedWithinRoot` behind a temporary env flag turned 8 containment tests RED (both BLOCKER exploits, all three symlink-escape branches, both absolute-path escapes); restoring it turned them green. Verified both ways: TMPDIR=/tmp (reproduces the Linux runner): 31 passed default macOS tmpdir: 31 passed TMPDIR=/tmp vitest run research/: 239 passed (22 files) Swept the rest of the newly-wired set under TMPDIR=/tmp for the same defect class: test:units 905 passed, test:synthesizer 186, test:skills 211, test:spec-validation 9 — no other tmpdir-depth assumption found. Prior-art: skipped — test-fixture path portability only; no new capability, no new dependency, no new module, and no change to the guard under test or to any assertion. --------- Co-authored-by: Test <test@example.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
packages/core/hooks/getff-work.test.tswas flaky/red on the vitest 5000ms default. Every case spawns a realbash scripts/getff-work.sh, which doesgit worktree add+ node_modules wiring + a package-manager install inside the new worktree — multi-second runtimes are inherent, not a regression. On clean stagingc1848e88feit ran 4 failed / 3 passed, all fourTest timed out in 5000ms. In CI it was marginal:CC-DEFERRALfailed on #1359's first run and went green on re-run, having already blocked one unrelated comment-only PR.Timeout configuration only. No assertion, test body, or
it()line changed —git diffcontains zero touchedexpect/toBe/toMatch/toContainlines.Why 60_000 for getff-work and not the usual 30_000
Timing the wrapper directly on a cold worktree:
The dominant leg is the script's own step 2 —
npm ciinside the freshly created worktree. Under vitest the three worktree-creating cases measured 31.4 / 33.2 / 31.4 s, so the siblingSLOW_SHELL_MS = 30_000is itself too tight here. 60_000 is ~2× the measured cold cost, has precedent (principles/20-bundle-classification.paired-negative.test.ts; the rootvitest.config.ts), and stays well under that file's documented 120_000 ceiling. The innerspawnSyncguard is namedSPAWN_GUARD_MSso the "test timeout must outlive the spawn guard" inequality is visible to the next editor.Siblings — same defect, wider than expected
A full-suite sweep (
vitest run hooks/ skills/) found 70 failed, 69 of themtimed out in 5000ms. Fixed at 30_000 (existingSLOW_SHELL_MSconvention):hooks/create-worktree.test.ts,hooks/worktree-setup.test.tshooks/dup-detect-empty-arg.test.ts,skills/dup-detect.test.ts,skills/dup-detect-skip-closed.test.tshooks/priority-score-branch-matcher.test.ts,hooks/priority-score-reconstruct-stub.test.tsPlus two CI-gated files carrying only partial per-test coverage — the untimed cases (C1/C2 tier-resolution; the fake-SHA paired-negative) were exactly the ones going red, and
test:hooksdoes not go green under load without them:hooks/validate-prompt.test.ts,hooks/check-worker-dispatch-channel.test.tsVerification (clean tree, macOS, 2026-08-10)
vitest run hooks/getff-work.test.tsvitest run hooks/ skills/—hooks/failuresvitest run hooks/ skills/— total failurestsc --noEmitKnown residue — deliberately NOT in this diff
Per CLAUDE.md «PR strategy» (no drive-by scope expansion), the remaining 38 timeouts are left alone. They live in four
skills/files outside the audited set —planner-discovery(12),plan-currency-check(11),-digest(6),-kickoff(5),run-helper(4). Same defect class, butskills/is gated by no workflow (CI runstest:hooks/test:principles/ir/backends/composition/live-generation/render), so they block no PR. Happy to take them in a follow-up.Two further observations surfaced while reproducing, also not fixed here (they are behaviour, not timeout config):
getff-work.test.tspollutes the working repo.runScriptpasses nocwd, andgetff-work.shresolves the repo root from its own location, so the first three cases create real worktrees.claude/worktrees/smoke-{cc,noncc,nolaunch}+ branchesworktree-smoke-*on every run.afterEachcleans only the temp repo. A leftover set from a prior run in another worktree is what madeNO-LAUNCH-FLAGfail with exit 1 (not a timeout) on first repro — the branch was checked out elsewhere, so bothgit worktree addattempts failed. Stale worktrees removed by hand (clean, 0 commits ahead oforigin/staging); the three branches remain.NO-LAUNCH-FLAGis ambient-env sensitive — it does not isolateCLAUDE_CODE_SESSION_ID, so inside a Claude Code session it takes the CC-deferral branch and fails on assertion. All runs above usedenv -u CLAUDE_CODE_SESSION_IDto emulate a plain shell and CI.Fidelity verdict
FIDELITY: skipped — not a stage PR and not an acceptance-contour deliverable: this diff carries no substrate kickoff/spec to be faithful to. It is a test-timeout configuration change only (no assertion, test body, or
it()line touched), so there is nothing for agents/fidelity-auditor.md to judge against.