fix(#2486): do not recommend or persist Claude-only worktree isolation on non-Claude runtimes (#2531)
* fix(#2486): runtime-branch the settings worktrees question + W020 health diagnostic On non-Claude runtimes /gsd:settings offered "Yes (Recommended)" for worktree isolation and persisted workflow.use_worktrees: true — the exact value the execution workflows fail closed on (#1521 guards). Branch the question on the same stamped config-get runtime read the guards use: Claude keeps the unchanged question; non-Claude offers only "No (Recommended)" / "Leave unchanged", never persists true, and warns when the config carries an inherited explicit true. /gsd:health gains W020, surfacing such a config with the guards' own predicate before execution-time failure. Docs state the runtime-conditional default. Fixes #2486 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2486): add changeset for PR #2531 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): reassign the health worktrees check W020 -> W024 (verify.cts namespace collision) The workflow-level check collided with the live W020 (git-worktree-list health) emitted by cmdValidateHealth in src/verify.cts — invisible from health.md's error_codes table, which stops at W019 and under-represents the real namespace (W010-W017, W020-W023 all live). W024 verified free. Adds a regression test pinning the chosen code against src/verify.cts so a future assignment cannot silently collide, a table note naming the namespace owner, and the changeset body reworded to house style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): pre-select the recommended repair in the broken-inheritance case Review round 2: at settings.md:142 the pre-selection rule left "Leave unchanged" as the default when the config carried an explicit non-false use_worktrees — the exact broken state the adjacent notice warns about, so accepting the default kept a config that fails closed at execution time. "Leave unchanged" is now the default only when the key is absent (nothing to repair); explicit false AND explicit non-false both pre-select "No (Recommended)", aligning the default, the label, and the notice. Pinned by two source-contract assertions in the #2486 regression block. Goldens (settings.md hash x19) + size baseline regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): gate the worktrees question on dispatch.isolation, not the runtime name Review round 2: #2584 Phase 3 replaced the runtime-name test with a declared `dispatch.isolation` capability, invalidating this PR's premise. cursor declares harness-worktree and codex/opencode/kimi/ kimi-code declare orchestrator-worktree, so a `RUNTIME != claude` gate blocked a supported configuration on five runtimes and false-warned in health. - settings.md + health.md read `query dispatch-isolation` and branch on `ISOLATION = none`; the runtime-name read is gone from both, and the capability read needs no per-runtime stamping (it fail-closes unknown/ undocumented internally) - all "Claude Code-only primitive" prose rewritten, including the two gates the shell-syntax check missed (config-key list, JSON schema comment) - W024 reconciled across health.md + CONFIGURATION.md + planning-config.md (docs still said W020, which collides with a verify.cts code) - health.md error-codes table fixed: the namespace note no longer sits between rows orphaning I001 - the asymmetry note for the two workflows #2584 has not migrated (quick.md, diagnose-issues.md) is enforced by a set-equality test with a self-check table, so it cannot go stale in either direction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(#2486): restore the Executor isolation section clobbered by #2661 `46ba02ac` (feat(#2630), the current next tip) reverted docs/CONFIGURATION.md to a pre-#2584 state: it restored the old "Non-Claude note" wording on the workflow.use_worktrees row and deleted the whole "Executor isolation per runtime" section. The change is unrelated to that PR's phase-estimation feature and looks like a stale-copy edit. This PR's use_worktrees row links to #executor-isolation-per-runtime, so the deletion leaves a dangling anchor. Restored byte-for-byte froma40ee8a5(the text #2584 Phase 3 originally shipped). No link/anchor checker exists in scripts/ or tests/, so CI would not have caught the dead link. The reverted row wording is outside this PR's scope and is still wrong on next; reported upstream as #2668. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(#2486): acknowledge the emitted size growth for settings.md and health.md The #2723 differential attribution check (epic #2719 Phase 3) landed on next after this branch opened and gates emitted-file growth behind a committed acknowledgment. Both grown files are attributable to source this PR changes; the ack names them and says why, per ADR-2719 §3. Verified load-bearing: removing tests/emitted-drift-ack.json reproduces the same failure; restoring it passes 59/59. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#2486): reword the W024 remediation so it names no bare gsd-tools call The W024 warning told the user to run `gsd-tools query config-set`, a command-position bare invocation that fails with "command not found" on a shim-only install (#2751) — the diagnostic sent the user into a second, more confusing error than the one it reported. The remediation now names only `/gsd:settings` and the config key itself, both of which work on every install layout, and the #2751 command-position gate goes green. Fixes #2486 * fix(#2486): drop the Known-asymmetry note and its guard test — #2728 migrates both workflows Review Major 2: once #2728 lands, the note describes a gap that no longer exists and instructs maintainers not to do the thing that was just done — with the set-equality guard test pinning the stale prose green. This PR now depends on #2728 (declared in the PR body), so the note and its guard go. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(#2486): make the #2728 merge order enforceable instead of advisory settings.md recommends and persists `workflow.use_worktrees: true` for every runtime whose declared dispatch.isolation is not `none` — cursor (harness-worktree) and codex/opencode/kimi/kimi-code (orchestrator-worktree). quick.md and diagnose-issues.md consume that value at dispatch time and, while they still gate on the runtime NAME, FATAL for all five. Merged first, this PR reintroduces #2486's own shape for the exact runtimes it exists to help — on the path it now labels "Recommended". The dependency was stated only as prose in the PR body. A merge-order note is not a gate: an automated batch merge never reads it. This adds the check that makes the ordering structural — red while either sibling is still name-gated, green the moment #2728 lands. The predicate matches a RUNTIME-vs-"claude" comparison, not the legitimate runtime-identity read, and accepts either the inline canonical dispatch-isolation read or a reference to dispatch-isolation-gate.md, which is the shape #2728 gives quick.md. Verified both directions against real sources: red against the current workflows, green against #2728's. This also closes W024's coverage window. W024 fires only when ISOLATION is `none`, so it is structurally blind to these five runtimes — /gsd:health would report healthy right up until quick.md FATALs. W024 cannot see the hazard, so the hazard is prevented by making the unsafe ordering unmergeable rather than by warning after the fact. Separately, the W-code namespace-collision test now declares its source read instead of passing lint silently: verify.cts emits its codes as inline string literals across ~25 addIssue() calls and exports no enumerable registry, so there is nothing to require() and assert against. The gap, and what it costs, are stated in the test. * docs(#2486): use the hyphen command form for /gsd-health in CONFIGURATION.md docs/ is never passed through the install-time slash-form converters, so the colon form names a command no runtime registers. Clears the sole lint-docs-command-form violation attributable to this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#2486): resolve isolation via new side-effect-free inspect-dispatch-isolation query; scope the settings change to isolation-none runtimes Review round 4: - B1: /gsd:health and /gsd:settings no longer call the recording dispatch-isolation query — on current next it persists the resolved decision to the executor-isolation sentinel as an unconditional #3045 side effect, letting a read-only diagnostic hard-block executor dispatch for the sentinel's lifetime across sessions. Both surfaces now use inspect-dispatch-isolation, a new read-only verb sharing the exact resolution implementation (extracted resolveDispatchIsolationDecision) with zero writes. Behavioral tests pin: no sentinel write, per-runtime parity with the recording verb, recording knobs ignored, --json shape. - B2: the #2728 merge-order interlock test is deleted — a repo test cannot sequence merges; it only made this PR unmergeable on its own schedule. The settings behavior change is scoped entirely to the ISOLATION=none branch, which needs nothing from #2728; the != none path is base behavior unchanged. - M1: the W024-vs-verify.cts namespace test (an admitted source-grep) is deleted per RULESET.TESTS.delete-bad-tests, without a standing exemption; the namespace claim lives as guidance in health.md. - M2: remaining allow-test-rule exemptions re-derived into documented categories (source-text-is-the-product, integration-test-input) with issue refs per ADR-456. - Minor: settings.md's current-value read drops the stampable --default/fallback so key-absence stays distinguishable from an explicit false on non-Claude emits (the pre-selection rule depends on the tri-state); docs now present inspect-dispatch-isolation as the inspection command and name dispatch-isolation as the recording resolver. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): put the install-marker rung in the canonical runtime resolver Round-7 review (independent cross-AI pass over the whole PR). BLOCKER — the gate did not fire in the default case. `resolveRuntime` stopped at GSD_RUNTIME > config.runtime > 'claude', and `config-new-project` writes NO `runtime` key — so on a real non-Claude install every consumer believed it was on Claude: isolation reported `harness-worktree`, /gsd:settings still offered "Yes (Recommended)" and W024 stayed silent. That is #2486's own symptom, surviving the fix meant to remove it. Verified end-to-end on a real `--qwen` install with a runtime-neutral config and GSD_RUNTIME unset: pre-fix `harness-worktree`, fixed `none`. The rung lives in `resolveRuntime` (src/runtime-slash.cts), the ONE canonical resolver, not in the isolation call site. A per-consumer fix forks precedence: `inspect-dispatch-isolation` would answer `cursor` while `dispatch-should-flatten` and `resolve-dispatch-type` still answered `claude` for the same install. All three now agree. `readInstallRuntimeMarker`, its cache and its test seams MOVED from model-resolver.cts to runtime-slash.cts, with model-resolver re-exporting the seams — one marker read and one cache, not two that drift. Deliberately NOT `resolveActiveRuntime`/`loadConfig`: an intermediate revision of this fix routed through `loadConfig`, which normalizes and rewrites legacy keys back to disk. That gave `inspect-dispatch-isolation` — the verb whose entire purpose is being side-effect-free — a write side effect, which is the defect the verb exists to avoid. `resolveRuntime` reads .planning/config.json directly. Re-verified: the inspect query against a real install creates no `.gsd/`. Tests, both of which were too weak in the first attempt and are now fail-first proven: - The W024 behavioral stub returned success-with-empty-output for an absent key. Real `config-get` EXITS NON-ZERO, and the `|| echo "true"` fallback only triggers on failure — so reintroducing the fallback would have passed. The stub now returns 1 for the absent case. - The marker regression asserted on the exported helper, so reverting gsd-tools.cjs to a marker-blind resolver still passed. It now drives a real `--qwen` install through the shipped `inspect-dispatch-isolation` query. (GSD_TEST_MODE must be cleared for that child, or install.js no-ops while still exiting 0 — a green test over an install that wrote nothing.) Spawned through `installSpawnEnv()` so ambient GSD_HOME cannot leak in. Also: the three PR-added `try/finally` test bodies converted to `t.after()` per CONTRIBUTING; CONTEXT.md's `worktree create` UNCONSUMED claim corrected (Phase 3 calls it in executor-isolation-dispatch.md); docs/CONFIGURATION.md now states the current non-Claude `use_worktrees` default rather than describing capability-scoped stamping that lands with #2652; resolver precedence comments updated to name the marker rung. Refs #2486 Refs #2668 * fix(#2486): split the marker rung out; make the use_worktrees doc row order-independent Round-8 review. trek-e's Blocker was procedural — commit 10fba7b8 moved the per-install .gsd-runtime marker rung into the canonical resolver, a large blast-radius change that arrived undisclosed and unreviewed. They offered two remedies; taking the second: SPLIT IT OUT. src/runtime-slash.cts and src/model-resolver.cts are reverted to their next state and the marker regression test is removed. What remains is what this PR was filed for: the settings.md / health.md / gsd-tools.cjs isolation query, W024, and the #2668 docs restoration. COST, stated plainly rather than buried: without that rung this PR's gate resolves 'claude' on a non-Claude install whose project config carries no `runtime` key — which is every config config-new-project writes. On those installs W024 stays quiet and /gsd-settings still offers Worktrees. The gate is correct whenever the runtime IS resolvable (GSD_RUNTIME set, or an explicit config.runtime). That is why `Fixes #2486` is already downgraded to `Refs` — #2486 must not close until the residual lands. A KNOWN LIMITATION comment at the resolver call site names #2395 so this does not read as an oversight. The rung itself belongs to #2395, which reports this exact defect and was closed by #2446 — a PR that touched only bin/install.js and fixtures and never runtime-slash.cts, so it persisted the identity into ~/.gsd/defaults.json, a tier resolveRuntime does not read. Evidence and a reopen request are posted there. That same tier is #2566's B1. DOCS — the reviewer flagged that this row and #2728's are order-dependent: whichever merges second falsifies the other. Removed the dependency instead of picking an order. The "Current default … until #2652" paragraph is now a plain troubleshooting note, true before and after #2728 lands. CONFLICT — one hunk in CONTEXT.md: next added the #2596 scope-conformance interface to the same Worktree-Safety paragraph where this PR corrected the `worktree create` UNCONSUMED claim. Resolved keeping both. Verified: lint:ci green. Full suite clean apart from the pre-existing #1160 installed-runtime capability surface. (emitted-attribution also failed until the fork's stale next was fast-forwarded — it defaults to origin/next, which was 131 commits behind; 175/175 against the current base.) Refs #2486 Refs #2668 * fix(#2486): address round-9 review — distinguish "cannot resolve" from "declares none", reject recording-only args on the read verb Codex review of the whole PR, five findings, all verified against source first. Major 3 (the one real defect). Both surfaces read isolation as `ISOLATION=$(… || echo "none")`, so a resolver failure became indistinguishable from a genuine `dispatch.isolation: none` declaration — and W024's text then asserts the latter, telling a user their runtime declares no executor-isolation primitive when GSD simply could not find out. health.md and settings.md now capture the raw value and track ISOLATION_RESOLVED, the same shape references/dispatch-isolation-gate.md already uses (#2652 review). W024 gained a second message for the unresolved case; settings still fails closed there — it must never persist a `true` it cannot justify — but reports what happened rather than a verdict it never reached. Major 4. `dispatch-isolation` applies --force-isolation AFTER the shared resolver returns; inspection accepted the flag and silently ignored it, so the same argv yielded 'none' from one verb and the declared capability from the other. inspect-dispatch-isolation now rejects --force-isolation/--phase/--plan as usage errors. Verified by mutation: disabling the guard reds exactly the three new rejection tests. Major 1. settings.md claimed this flow and the execution guards "always reach the same verdict". False while quick.md still gates on the runtime name: an orchestrator-worktree host can be offered a `true` that /gsd:quick rejects as fatal, W024 silent because isolation is not none. Scoped the claim to the capability gate, named #2728 as the conversion, and added the same caveat to the CONFIGURATION.md use_worktrees row. Not a behavior change — the `!= none` branch is untouched base behavior. Major 2. "side-effect-free" overstated it: every gsd-tools invocation runs the shared bootstrap, and getActiveWorkstream unlinks a stale workstream pointer. That is pre-existing, verb-independent and cannot block a dispatch; writing the sentinel can. Renamed the claim to "sentinel-free" everywhere and documented precisely what is and is not asserted. Minor 5. The JSON parity test asserted against a handwritten key list and never invoked the recording verb, so the two could diverge and stay green. It now runs `dispatch-isolation --json` in a separate project dir and deep-compares, with a control asserting that verb DID write a sentinel. Added the orchestrator exec branch (--cwd-target) the registry parity test never covered. runtime-converters.test.cjs pinned the old `|| echo "none"` line as canonical — that literal was the Major 3 defect. Repinned as two invariants (the raw read is present; the collapsing fallback is absent) plus an ISOLATION_RESOLVED requirement, so reformatting does not fail the test but a semantic regression does. settings.md sits at 40777 bytes against the 40960 DEFAULT hard cap. The round-9 prose was compressed to fit rather than raising the cap. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next @33fca50d(#1160 _resolveManifest, and the #3053 quick_id tests, which compare a local-time expectation against a TZ=UTC child and so only pass on a UTC host). * fix(#2486): second review pass — drop a dangling reference, correct the whole unresolved branch, pin the branch behaviorally Codex re-review of the full PR after the first round-9 pass. It confirmed Majors 1, 2 and 4 and Minor 5 fixed, and found seven more. All verified against source. Minor 5 was mine and the worst of them: settings.md and health.md pointed at `gsd-core/references/dispatch-isolation-gate.md`, which exists only on the #2728 branch — not in this PR and not on the merge-base. Merging this alone would have shipped three dangling canonical-source references. Removed; the surrounding text now stands on its own. Major 2. The unresolved branch corrected only the two option descriptions. The pre-selection rationale still claimed an absent key already resolves to false on this runtime, and the explicit-true notice still stated the runtime declares no primitive — both capability verdicts that were never reached. The substitution rule now covers every place in that branch that asserts one. Major 3. The repinned invariants could not catch the mutation they existed to catch: flipping the shipped block's ISOLATION_RESOLVED=true to false left all three green, since they only assert the raw read is present, the token appears, and the collapsing fallback is gone. Added a behavioral test that drives the shipped W024 block twice — resolver answering vs resolver exiting non-zero — and asserts the two emit different text, that the unresolved branch says "could not resolve", and that it does NOT claim the runtime has no primitive. Verified: the true->false mutation now reds it. Major 1. The verb fail-closes an unknown or undeclared runtime to `none` and exits 0, so ISOLATION_RESOLVED=true means "the query answered", not "the runtime published a declaration" — the shell cannot see an internal fallback. Exposing provenance is an API change and out of scope here, so W024's resolved-case text is instead written to be true of every path that reaches it ("no usable executor-isolation primitive — declared or fail-closed from an unknown value"), and both workflows state the limit of the signal explicitly. Minor 4. The rejection tests regex-matched human prose, so swapping ERROR_REASON.USAGE for UNKNOWN would have kept them green while breaking machine consumers. They now pass --json-errors and assert reason === 'usage' on the parsed envelope. Minor 6. CONTEXT.md still described the verb as side-effect-free. Now sentinel-free, consistent with the router comment and the other two surfaces. Minor 7. 183 bytes of headroom under the 40960 hard cap was called unacceptable, and it was — a routine 184-byte edit would have failed CI. Compressed this PR's own settings.md prose (the #2486 block was carrying ~7.1KB of rationale) to 40499 bytes, 458 free. Two phrases other tests pin verbatim were restored after the first compression pass reworded them. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next @33fca50d. * fix(#2486): third review pass — placeholder table replaces the fragile substitution rule Codex round 3 blocked the push on two Majors. Both verified and real. Major 2 was the substantive one and my error. The round-2 fix told the model to find and replace the string "this runtime declares no executor-isolation primitive (dispatch.isolation: none)" — text that does not appear anywhere in the branch. The first option description has no parenthetical, the second asserts "absent, it already resolves to false on this runtime" (not covered at all), and the pre-selection rationale and notice carried three more unsupported assertions. A model following the instruction literally would have found no match and changed nothing. Replaced with three named placeholders — {FINDING}, {CONSEQUENCE}, {ABSENCE} — and a two-column table giving each one's resolved and unresolved wording. Every assertion in the branch now flows from the table, so none of them can outrun what was actually established, and there is no string-matching to get wrong. It is also shorter than the prose it replaced. Major 1: the resolver catches thrown errors and returns none successfully, a path the round-2 wording ("declared, or unknown/undeclared") did not cover. Both surfaces now say "declared as none, or fail-closed because the capability could not be determined", which is true of the thrown path too, and both state that an internal resolution error is among the things the verb fail-closes. Still not provenance — the verb cannot distinguish these for the caller — but no longer a claim the code contradicts. Minor 3: the behavioral test asserted only that the two branches differ and that the resolved one omits "could not resolve", so arbitrary resolved text stayed green. Now pins what it must positively say: the capability finding, the fail-closed consequence, the offending key, and (both branches) the repair command. Minor 4: two stale artifacts the round-2 sweep missed — a test comment still citing the #2728-only references/dispatch-isolation-gate.md, and the emitted drift ack still describing inspection as side-effect-free. Also aligned the two remaining router comments to sentinel-free. settings.md 40420 bytes, 540 free under the cap. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next @33fca50d. * fix(#2486): fourth review pass — stop recommending a repair whose effect depends on the emit Codex round 4, one Major and three Minors. The Major is a real hole and worth the round. W024 advised "remove the key from .planning/config.json so the runtime default (false) applies". That default is not false everywhere: execute-phase.md:102, quick.md:155 and diagnose-issues.md:62 all read the key as `|| echo "true"`, and only `_stampNonClaudeRuntimeDefaults` rewrites them to `--default false` on a non-Claude emit. So on an un-stamped emit the advised repair leaves use_worktrees resolving to TRUE against isolation none — exactly the state W024 exists to flag. Reachable in the round-3 gap: a caught internal resolver error returns none with rc=0, so ISOLATION_RESOLVED is true and the confident branch fires on a host whose emit was never stamped. Fixed by removing the dependence rather than the symptom: both surfaces now say to set workflow.use_worktrees false explicitly, and say why deleting the key is not equivalent. The {ABSENCE} placeholder no longer claims absence resolves to false unconditionally — it is scoped to an emit that stamped that default. Minor: the notice and W024 hardcoded .planning/config.json, which is the wrong file in an active workstream (settings itself resolves $GSD_CONFIG_PATH correctly). Both now say "the project config", naming the workstream case. Minor: the new repair pin required the literal /gsd:settings, a form documented as no longer routable. Relaxed to accept the canonical slash and $-prefixed forms so a correct rewording cannot fail the test. Nit: two test descriptions still said side-effect-free. Not fixed, deliberately: the verb still cannot tell a caught resolver error from a declared none, so ISOLATION_RESOLVED remains a signal about the CALL, not the declaration. Both workflows now state that limit outright. Exposing provenance is a change to the query contract that belongs with the runtime-resolution work in #2395, not here. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next @33fca50d. The only change after that suite run was removing a redundant regex escape flagged by eslint; that test was re-run individually and eslint is clean. * fix(#2486): fifth review pass — stop recommending "leave it absent" as the safe default Codex round 5, one Major: the round-4 fix corrected the {ABSENCE} wording but left the pre-selection rule built on the assumption it had just falsified. An absent key still pre-selected "Leave unchanged" on the rationale that there was nothing to repair. Absence resolves to false only where the emit stamped that default; on an un-stamped emit it reads as true, so accepting the recommended default could preserve the exact isolation-none-plus-true state this branch exists to prevent. The isolation-none branch now pre-selects "No (Recommended)" in every case, including an absent key. Writing an explicit false is correct under either emit; "Leave unchanged" stays available for the deliberate shared-config case but is never the default. The test that pinned the old rule pinned a falsified assumption, so it now pins the new one and asserts the old sentence is gone. Also tightened the round-4 repair regex, which had been relaxed far enough to accept `$gsd:settings` and `/gsd:settings-bogus`. It now matches only the canonical `/gsd-settings`, `/gsd:settings` and `$gsd-settings` forms. settings.md 40685 bytes, 275 free. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next @33fca50d. * fix(#2486): make the inspection exemption block-scoped, and drop the last pre-#2728 claim Codex review of the conflict resolution: one Major, two Minors, all real. Major. The inspection-surface exemption I added to #2728's "every dispatch-site degrade block re-records" guard was FILE-wide. Codex probed it by adding an unrecorded executor block to health.md: the exemption assertions still passed, the whole file was skipped, and the offender went unreported. That is the same hole the hand-listed revision of this guard had — a promise of "every dispatch site" with a silent carve-out. Now scoped per block. A block earns the exemption only by resolving through `inspect-dispatch-isolation` AND carrying no dispatch primitive (`Agent(`, `harnessFlag`/`HARNESS_FLAG`, `isolation="worktree"`, or the recording verb). The file-level assertions remain as a precondition on top. Verified with Codex's own probe: the appended dispatch block is now reported at health.md:285, while the legitimate inspection block still passes. Minor. settings.md still carried the caveat saying `quick.md` gates on the runtime name and "#2728 converts that surface". #2728 has landed. Same obsolete premise already removed from docs/CONFIGURATION.md in the merge commit; this was the copy I missed. Removing it also returns 413 bytes, so settings.md now has 688 free under the 40960 cap rather than 275. Minor. Four scratch-directory prefixes still read `w024` after the renumber. Non-functional, but the whole point of the rename was that W024 now means someone else's warning. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next. * fix(#2486): name the diagnostics' state INSPECTED_ISOLATION and delete the exemption Codex probed the per-block exemption and it was still escapable: DISPATCH_PRIMITIVE matched only `Agent(`, two harness-flag spellings, `isolation="worktree"` and the recording verb, so `Task(`, `spawn_agent(`, a `codex exec` process dispatch, and — unfixably by any regex — the repo's normal split shape (isolation resolved in one bash fence, dispatch in a later one) all still qualified as exempt. Took Codex's suggested fix, which is better than the one it replaces. The diagnostics now name their state `INSPECTED_ISOLATION` / `INSPECTED_RESOLVED` / `_INSPECTED_RAW` instead of reusing the dispatch-site names. #2728's guard scans for a literal `ISOLATION=none`, so health.md and settings.md fall outside it BY CONSTRUCTION and the exemption is deleted outright — no carve-out to escape, and a diagnostic that ever writes a real `ISOLATION=none` is caught like any other site. The name is also just more accurate: an inspection result is not a dispatch decision. Pinned so the reasoning cannot be lost: the #2486 suite now asserts neither diagnostic assigns the bare `ISOLATION` name, with the rationale in the comment. Verified by mutation — renaming back makes #2728's guard flag health.md:107 AND fails the new naming pin. Net effect on the guard's coverage is positive: before this PR it scanned two fewer files by exemption; now it scans everything. settings.md 40352 bytes, 608 free. Validated: lint:ci clean; full suite green except the two failures that reproduce identically on pristine next. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Behruz Nassre Esfahani
committed by
GitHub
GitHub
parent
77374cfc32
commit
f0abdb1b89
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"settings.md": "#2486 \u2014 the Worktrees question now branches on the runtime's declared dispatch.isolation capability (resolved via the sentinel-free inspect-dispatch-isolation query) instead of offering Claude-only worktree isolation everywhere. Growth is the isolation-none branch, its explanatory notice, and the tri-state pre-selection rule for the broken-inheritance repair."
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"health.md": "#2573 registers W024 (STATE.md written many commits ago — treat its contents as approximate) in the health workflow's <error_codes> table, so the advisory health now emits is documented where every other W-code is listed. The growth is that single table row written inline, not relocated into an eagerly @-imported reference (ADR-1610 Decision 4). 12246 -> 12348 bytes (+102), DEFAULT tier, cap 40960."
|
||||
"health.md": "#2573 registers W024 (STATE.md written many commits ago \u2014 treat its contents as approximate) in the health workflow's <error_codes> table, so the advisory health now emits is documented where every other W-code is listed. The growth is that single table row written inline, not relocated into an eagerly @-imported reference (ADR-1610 Decision 4). 12246 -> 12348 bytes (+102), DEFAULT tier, cap 40960.\n\nAlso acknowledged here because the seam permits exactly one ack source per path and #2573 landed on next first (#2486 rebase, 2026-08-11): #2486 \u2014 adds the W025 diagnostic that detects a persisted workflow.use_worktrees:true on a runtime whose declared isolation cannot honor it, resolved via the sentinel-free inspect-dispatch-isolation query. Growth is the new check, its prose, and the error_codes row."
|
||||
}
|
||||
}
|
||||
|
||||
@@ -874,3 +874,201 @@ describe('#3045 MINOR — writer/reader sentinel path derivation now agrees for
|
||||
assert.equal(read.isolation, 'harness-worktree');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#2486 regression: inspect-dispatch-isolation is the sentinel-free read', () => {
|
||||
// /gsd:health (W025) and /gsd:settings (Worktrees branching) must be able to
|
||||
// learn the negotiated isolation WITHOUT recording it: the #3045 recording
|
||||
// verb stamps a phase:null/plan:null sentinel the guard hooks then enforce
|
||||
// against real executor dispatches — letting a read-only diagnostic
|
||||
// hard-block execution for the sentinel's lifetime, across sessions.
|
||||
|
||||
test('inspect-dispatch-isolation resolves the declared capability and writes NO sentinel', (t) => {
|
||||
const dir = createTempProject('gsd-2486-inspect-');
|
||||
t.after(() => cleanup(dir));
|
||||
assert.equal(fs.existsSync(sentinelFile(dir)), false, 'precondition: no sentinel yet');
|
||||
const result = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--raw'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: 'claude', HOME: dir },
|
||||
);
|
||||
assert.equal(result.success, true, result.error);
|
||||
assert.equal(result.output.trim(), 'harness-worktree');
|
||||
assert.equal(
|
||||
fs.existsSync(sentinelFile(dir)),
|
||||
false,
|
||||
'inspection must not create .gsd/dispatch-isolation-sentinel.json',
|
||||
);
|
||||
assert.equal(fs.existsSync(path.join(dir, '.gsd')), false, 'inspection must not even create the .gsd dir');
|
||||
});
|
||||
|
||||
|
||||
test('parity: inspect resolves byte-identically to the recording verb for every registry runtime', (t) => {
|
||||
// Same negotiation implementation by construction (shared helper) — this
|
||||
// pins the contract so a future edit cannot fork the two verbs apart.
|
||||
for (const runtimeId of Object.keys(runtimes)) {
|
||||
const dir = createTempProject('gsd-2486-parity-');
|
||||
t.after(() => cleanup(dir));
|
||||
const inspected = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--raw'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: runtimeId, HOME: dir },
|
||||
);
|
||||
assert.equal(inspected.success, true, inspected.error);
|
||||
assert.equal(
|
||||
fs.existsSync(sentinelFile(dir)),
|
||||
false,
|
||||
`${runtimeId}: inspect must not write the sentinel`,
|
||||
);
|
||||
|
||||
const dispatched = runGsdTools(
|
||||
['query', 'dispatch-isolation', '--raw'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: runtimeId, HOME: dir },
|
||||
);
|
||||
assert.equal(dispatched.success, true, dispatched.error);
|
||||
assert.equal(
|
||||
inspected.output.trim(),
|
||||
dispatched.output.trim(),
|
||||
`${runtimeId}: the two verbs must resolve the same isolation`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// #2486 review, Major 4: silently IGNORING these was the defect. The
|
||||
// recording verb applies --force-isolation after the shared helper returns,
|
||||
// so accepting-and-ignoring it here means the same argv yields 'none' from
|
||||
// dispatch and the declared capability from inspect — a caller gets a
|
||||
// different answer with no signal. Rejecting turns that into a loud usage
|
||||
// error. This test fails if the verb ever goes back to accepting them.
|
||||
for (const [flag, value] of [['--force-isolation', 'none'], ['--phase', '9'], ['--plan', 'p1']]) {
|
||||
test(`inspect REJECTS the recording-only argument ${flag}`, (t) => {
|
||||
const dir = createTempProject('gsd-2486-inspect-args-');
|
||||
t.after(() => cleanup(dir));
|
||||
// --json-errors so the assertion is on the TYPED reason, not on human
|
||||
// prose: swapping ERROR_REASON.USAGE for UNKNOWN would keep a message
|
||||
// regex green while breaking every machine consumer (#2486 review,
|
||||
// round-9 Minor 4).
|
||||
const result = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--raw', flag, value, '--json-errors'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: 'claude', HOME: dir },
|
||||
);
|
||||
assert.equal(result.success, false, `${flag} must be a usage error, not a silently ignored argument`);
|
||||
const envelope = JSON.parse(result.error.trim().split('\n').filter(Boolean).pop());
|
||||
assert.equal(
|
||||
envelope.reason,
|
||||
'usage',
|
||||
`${flag}: must be typed as a usage error — got ${JSON.stringify(envelope.reason)}`,
|
||||
);
|
||||
assert.match(
|
||||
envelope.message || '',
|
||||
/recording-only/,
|
||||
`${flag}: the message must say why the argument has no read-path meaning`,
|
||||
);
|
||||
assert.equal(fs.existsSync(sentinelFile(dir)), false, 'a rejected inspection still records nothing');
|
||||
});
|
||||
}
|
||||
|
||||
test('the divergence that rejection prevents: dispatch DOES honour --force-isolation', (t) => {
|
||||
// Pins the asymmetry that makes rejection necessary rather than pedantic.
|
||||
// If a future edit moved the override into the shared helper, inspect could
|
||||
// safely accept the flag — and this test would still pass, correctly, while
|
||||
// the rejection tests above would then be the ones to revisit.
|
||||
const dir = createTempProject('gsd-2486-force-divergence-');
|
||||
t.after(() => cleanup(dir));
|
||||
const dispatched = runGsdTools(
|
||||
['query', 'dispatch-isolation', '--raw', '--force-isolation', 'none'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: 'claude', HOME: dir },
|
||||
);
|
||||
assert.equal(dispatched.success, true, dispatched.error);
|
||||
assert.equal(dispatched.output.trim(), 'none', 'force is honoured on the recording verb');
|
||||
|
||||
const inspected = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--raw'],
|
||||
dir,
|
||||
{ GSD_RUNTIME: 'claude', HOME: dir },
|
||||
);
|
||||
assert.equal(inspected.success, true, inspected.error);
|
||||
assert.equal(
|
||||
inspected.output.trim(),
|
||||
'harness-worktree',
|
||||
'inspection reports the DECLARED capability, which is what the two would disagree on',
|
||||
);
|
||||
});
|
||||
|
||||
// #2486 review, Minor 5: asserting inspection against a HANDWRITTEN key list
|
||||
// does not test parity at all — changing the recording verb's JSON
|
||||
// independently would leave it green. Compare against the real thing.
|
||||
test('--json shape matches the recording verb, compared against its actual output', (t) => {
|
||||
const inspectDir = createTempProject('gsd-2486-inspect-json-');
|
||||
const dispatchDir = createTempProject('gsd-2486-dispatch-json-');
|
||||
t.after(() => cleanup(inspectDir));
|
||||
t.after(() => cleanup(dispatchDir));
|
||||
|
||||
const inspected = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--json'],
|
||||
inspectDir,
|
||||
{ GSD_RUNTIME: 'cursor', HOME: inspectDir },
|
||||
);
|
||||
assert.equal(inspected.success, true, inspected.error);
|
||||
const inspectedJson = JSON.parse(inspected.output);
|
||||
|
||||
// Separate project dir: the recording verb writes a sentinel, and the
|
||||
// inspection assertion below must not be able to see it.
|
||||
const dispatched = runGsdTools(
|
||||
['query', 'dispatch-isolation', '--json'],
|
||||
dispatchDir,
|
||||
{ GSD_RUNTIME: 'cursor', HOME: dispatchDir },
|
||||
);
|
||||
assert.equal(dispatched.success, true, dispatched.error);
|
||||
const dispatchedJson = JSON.parse(dispatched.output);
|
||||
|
||||
assert.deepEqual(
|
||||
Object.keys(inspectedJson).sort(),
|
||||
Object.keys(dispatchedJson).sort(),
|
||||
'consumers written against the recording verb JSON must be able to switch verbatim',
|
||||
);
|
||||
assert.deepEqual(
|
||||
inspectedJson,
|
||||
dispatchedJson,
|
||||
'same runtime, same declared capability — every field must agree, not just the key set',
|
||||
);
|
||||
assert.equal(inspectedJson.runtime, 'cursor');
|
||||
assert.equal(inspectedJson.isolation, 'harness-worktree');
|
||||
assert.equal(fs.existsSync(sentinelFile(inspectDir)), false, 'no sentinel from a --json inspection either');
|
||||
assert.equal(fs.existsSync(sentinelFile(dispatchDir)), true, 'control: the recording verb DID write one');
|
||||
});
|
||||
|
||||
// #2486 review, Minor 5 (second half): the registry parity test compares raw
|
||||
// isolation only, so it never exercises the orchestrator `exec` branch that
|
||||
// --cwd-target populates. Compare the full JSON there too.
|
||||
test('parity holds on the orchestrator exec branch (--cwd-target), not just raw isolation', (t) => {
|
||||
const inspectDir = createTempProject('gsd-2486-inspect-cwd-');
|
||||
const dispatchDir = createTempProject('gsd-2486-dispatch-cwd-');
|
||||
t.after(() => cleanup(inspectDir));
|
||||
t.after(() => cleanup(dispatchDir));
|
||||
|
||||
const inspected = runGsdTools(
|
||||
['query', 'inspect-dispatch-isolation', '--json', '--cwd-target', 'wt'],
|
||||
inspectDir,
|
||||
{ GSD_RUNTIME: 'codex', HOME: inspectDir },
|
||||
);
|
||||
assert.equal(inspected.success, true, inspected.error);
|
||||
const dispatched = runGsdTools(
|
||||
['query', 'dispatch-isolation', '--json', '--cwd-target', 'wt'],
|
||||
dispatchDir,
|
||||
{ GSD_RUNTIME: 'codex', HOME: dispatchDir },
|
||||
);
|
||||
assert.equal(dispatched.success, true, dispatched.error);
|
||||
|
||||
const inspectedJson = JSON.parse(inspected.output);
|
||||
assert.deepEqual(
|
||||
inspectedJson,
|
||||
JSON.parse(dispatched.output),
|
||||
'the exec branch must resolve identically on both verbs',
|
||||
);
|
||||
assert.equal(inspectedJson.isolation, 'orchestrator-worktree', 'precondition: codex is the orchestrator-worktree case');
|
||||
assert.ok(inspectedJson.exec, 'precondition: this branch actually populates exec, so the comparison means something');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2480,6 +2480,15 @@ describe('#2728 B1 — isolation degrades re-record through the single write pat
|
||||
'delegate or make those sites re-record inline',
|
||||
);
|
||||
|
||||
// #2486 note: the runtime-neutral diagnostics (health.md, settings.md)
|
||||
// resolve isolation for a READ, not a dispatch, and deliberately name their
|
||||
// state `INSPECTED_ISOLATION` rather than `ISOLATION`. That keeps them out
|
||||
// of this scan by construction. An earlier revision exempted those two
|
||||
// files instead; the exemption was file-wide, so any real dispatch block
|
||||
// added to either one would have inherited it and escaped a guard whose
|
||||
// name promises "every dispatch site" (#2486 review). Renaming the variable
|
||||
// removes the carve-out entirely — a diagnostic that ever writes a literal
|
||||
// `ISOLATION=none` is caught here like any other site.
|
||||
for (const file of scan) {
|
||||
const rel = path.relative(REPO_ROOT, file).replace(/\\/g, '/');
|
||||
if (DELEGATED_TO_PER_PLAN_GATE.has(rel)) continue;
|
||||
|
||||
@@ -1530,6 +1530,465 @@ test('manager.md and autonomous.md no longer contain old "not claude" background
|
||||
});
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// #2486: settings.md must not recommend or persist worktree isolation on
|
||||
// non-Claude runtimes, and health.md must surface an inherited explicit
|
||||
// use_worktrees=true before execution fails closed on it. Both rely on the
|
||||
// #1521 stamping machinery, so the canonical runtime/use_worktrees reads in
|
||||
// those two files are part of the runtime contract surface.
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
{
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const conversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs');
|
||||
const { NON_CLAUDE_RUNTIMES } = conversion;
|
||||
|
||||
const RUNTIME_BRANCH_WORKFLOWS = ['settings.md', 'health.md'];
|
||||
const CLAUDE_RUNTIME_LINE = 'config-get runtime --default claude --raw 2>/dev/null || echo "claude"';
|
||||
const TRUE_WT_LINE = 'config-get workflow.use_worktrees --raw 2>/dev/null || echo "true"';
|
||||
// #2486 review round 2 (#2584): the capability read that replaced the runtime-name
|
||||
// gate. Round 4: `inspect-dispatch-isolation`, the side-effect-free inspection verb —
|
||||
// the recording `dispatch-isolation` verb stamps the executor-dispatch sentinel as an
|
||||
// unconditional #3045 side effect, which an inspection surface must never do.
|
||||
// Round 9 (#2486 review, Major 3): this used to pin the single-line
|
||||
// `ISOLATION=$(… || echo "none")` read. That shape was the defect — `|| echo
|
||||
// "none"` collapses "this runtime declares no primitive" into "the resolver
|
||||
// failed", and both surfaces then assert the former, which is false. The
|
||||
// canonical read now captures the raw value and tracks whether a verdict was
|
||||
// actually learned, the same shape the execution-side isolation gate uses.
|
||||
// Pinned as two invariants rather than one long literal so that reformatting
|
||||
// the block does not fail the test while a semantic regression still does.
|
||||
const ISOLATION_LINE = '_INSPECTED_RAW=$(gsd_run query inspect-dispatch-isolation --raw 2>/dev/null)';
|
||||
const ISOLATION_RESOLVED_FLAG = 'INSPECTED_RESOLVED';
|
||||
const ISOLATION_COLLAPSING_FALLBACK = /inspect-dispatch-isolation --raw 2>\/dev\/null \|\| echo/;
|
||||
const readWorkflow = (wf) =>
|
||||
fs.readFileSync(path.join(__dirname, '..', 'gsd-core', 'workflows', wf), 'utf8');
|
||||
|
||||
// Behavioral W025 coverage (#2486 Major 2) executes the shipped block, so it
|
||||
// needs a subprocess and a CRLF-safe read. `readFileNormalized` normalizes at
|
||||
// the READ boundary — a `\r?\n` fence regex alone leaves embedded CR inside
|
||||
// the captured body, which bash then treats as part of the token
|
||||
// (DEFECT.WINDOWS-CRLF-TEST-PORTABILITY).
|
||||
//
|
||||
// The subprocess goes through the process seam, never a hand-rolled
|
||||
// spawnSync (CONTRIBUTING "Spawning a subprocess: use the process seam").
|
||||
// `runHook` already documents `interpreter: 'bash'` for running a shell
|
||||
// script, so the harness is written to a file rather than passed as `-c`.
|
||||
const { runHook } = require('./helpers/process-seam.cjs');
|
||||
const { readFileNormalized, createTempDir, cleanup: cleanupDir } = require('./helpers.cjs');
|
||||
// Skipped on Windows, where there is no bash. Checked by platform rather than
|
||||
// by shelling out to `which`, which is itself non-portable.
|
||||
const NO_BASH = process.platform === 'win32';
|
||||
|
||||
describe('#2486 regression: settings/health worktrees isolation branch', () => {
|
||||
// Review round 2 (#2584 Phase 3): isolation is a DECLARED CAPABILITY, not a
|
||||
// runtime name. cursor declares harness-worktree and codex/opencode/kimi/
|
||||
// kimi-code declare orchestrator-worktree, so a `RUNTIME != claude` gate
|
||||
// would block a supported configuration on five runtimes and false-warn in
|
||||
// health. Both surfaces branch on `dispatch-isolation` instead.
|
||||
test('settings.md and health.md contain NO runtime-name gate for the worktrees branch', () => {
|
||||
// allow-test-rule: source-text-is-the-product (#2486)
|
||||
// Workflow .md text IS what the runtime loads — asserting on it tests the deployed contract.
|
||||
for (const wf of RUNTIME_BRANCH_WORKFLOWS) {
|
||||
const src = readWorkflow(wf);
|
||||
assert.ok(
|
||||
!src.includes(CLAUDE_RUNTIME_LINE),
|
||||
`${wf}: the worktrees branch must not read the runtime name — gate on dispatch-isolation (#2584)`,
|
||||
);
|
||||
assert.ok(
|
||||
!/RUNTIME"?\s*(!=|=)\s*"?claude/.test(src),
|
||||
`${wf}: residual RUNTIME-vs-claude comparison — execute-phase forbids a runtime-name fan-out`,
|
||||
);
|
||||
// Prose gates count too: the shell-syntax check above missed two
|
||||
// sentences that still asserted the obsolete non-Claude premise
|
||||
// (the config-key list and the emitted-JSON schema comment).
|
||||
assert.ok(
|
||||
!/non-Claude (installs?|runtimes?)\b[^.]{0,120}\b(fail closed|default it to `?false)/i.test(src),
|
||||
`${wf}: prose still states the obsolete "non-Claude cannot honor worktrees" premise — gate on dispatch.isolation`,
|
||||
);
|
||||
assert.ok(
|
||||
!/never written as true on a non-Claude runtime/i.test(src),
|
||||
`${wf}: emitted-JSON comment still claims a runtime-name persistence rule`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// Round 4 note: an earlier revision carried a test here that encoded the
|
||||
// #2728 merge order as a red assertion against quick.md/diagnose-issues.md.
|
||||
// Deleted: a repo test cannot sequence merges — it only made this change
|
||||
// unmergeable on its own schedule. The settings behavior change is instead
|
||||
// scoped entirely to the `ISOLATION = none` branch (below), which needs
|
||||
// nothing from any sibling PR; the `!= none` path is base behavior unchanged.
|
||||
|
||||
test('settings.md and health.md resolve isolation with the sentinel-free inspection read', () => {
|
||||
// allow-test-rule: source-text-is-the-product (#2486)
|
||||
// Workflow .md text IS what the runtime loads — asserting on it tests the deployed contract.
|
||||
for (const wf of RUNTIME_BRANCH_WORKFLOWS) {
|
||||
const src = readWorkflow(wf);
|
||||
assert.ok(
|
||||
src.includes(ISOLATION_LINE),
|
||||
`${wf}: missing the canonical inspect-dispatch-isolation read (fail-closes unknown/undocumented to none)`,
|
||||
);
|
||||
// Major 3: fail-closed is right; reporting the failure AS a capability
|
||||
// verdict is not. Both surfaces must be able to tell the two apart.
|
||||
// #2486 review: the diagnostics name their state INSPECTED_ISOLATION,
|
||||
// not ISOLATION. That is load-bearing, not cosmetic — #2728's
|
||||
// "every dispatch-site degrade block re-records" guard scans for a
|
||||
// literal `ISOLATION=none` in any workflow bash block, and a read-only
|
||||
// surface has no sentinel to re-record. Naming the read differently
|
||||
// keeps these files out of that scan BY CONSTRUCTION; the alternative
|
||||
// was exempting the two files, which silently covered any future
|
||||
// dispatch block added to them.
|
||||
assert.ok(
|
||||
!/^\s*ISOLATION=/m.test(src),
|
||||
`${wf}: a read-only diagnostic must not assign the dispatch-site variable name ISOLATION — use INSPECTED_ISOLATION so #2728's re-record guard does not have to carve out this file`,
|
||||
);
|
||||
assert.ok(
|
||||
src.includes(ISOLATION_RESOLVED_FLAG),
|
||||
`${wf}: must track ISOLATION_RESOLVED — a resolver failure is not a declaration of 'none' (#2486 review, Major 3)`,
|
||||
);
|
||||
assert.ok(
|
||||
!ISOLATION_COLLAPSING_FALLBACK.test(src),
|
||||
`${wf}: '|| echo "none"' on the inspect read collapses "could not resolve" into "declares none" — capture the raw value and branch on ISOLATION_RESOLVED instead`,
|
||||
);
|
||||
// #2486 round 4 (B1): the RECORDING resolver is dispatch-only. On
|
||||
// current next, `query dispatch-isolation` persists its decision to
|
||||
// .gsd/dispatch-isolation-sentinel.json as an unconditional #3045 side
|
||||
// effect, and the isolation guard hooks hard-fail dispatches that
|
||||
// disagree with the recorded sentinel. An inspection surface calling it
|
||||
// would let /gsd:health or /gsd:settings hard-block executor dispatch
|
||||
// for the sentinel's lifetime — across sessions, since the sentinel
|
||||
// root resolves linked worktrees to the main checkout.
|
||||
assert.ok(
|
||||
!src.includes('query dispatch-isolation'),
|
||||
`${wf}: calls the RECORDING dispatch-isolation verb — inspection surfaces must use inspect-dispatch-isolation, which never writes the sentinel`,
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('"$INSPECTED_ISOLATION" = "none"') || src.includes('`INSPECTED_ISOLATION` = `none`'),
|
||||
`${wf}: must gate on ISOLATION = none, the only value that cannot honor worktrees`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('the isolation read is runtime-neutral — no per-runtime stamping rewrites it', () => {
|
||||
// inspect-dispatch-isolation resolves the runtime internally and fail-closes,
|
||||
// so unlike the #1521 runtime read it must survive every emit byte-identical.
|
||||
// allow-test-rule: integration-test-input (#2486)
|
||||
// The workflow source is fed to _applyRuntimeRewrites as real fixture input;
|
||||
// the assertion is on the transformation's output.
|
||||
for (const rt of NON_CLAUDE_RUNTIMES) {
|
||||
for (const wf of RUNTIME_BRANCH_WORKFLOWS) {
|
||||
const out = conversion._applyRuntimeRewrites(readWorkflow(wf), rt, `$HOME/.${rt}/`, true, undefined);
|
||||
assert.ok(
|
||||
out.includes(ISOLATION_LINE),
|
||||
`${rt}/${wf}: the inspect-dispatch-isolation read must not be rewritten by per-runtime stamping`,
|
||||
);
|
||||
}
|
||||
// The settings tri-state read must survive stamping too: if
|
||||
// _stampNonClaudeRuntimeDefaults ever matched the bare (no-fallback)
|
||||
// shape, absence would again collapse into an explicit false and the
|
||||
// pre-selection rule would go dead on that runtime.
|
||||
const settingsOut = conversion._applyRuntimeRewrites(readWorkflow('settings.md'), rt, `$HOME/.${rt}/`, true, undefined);
|
||||
assert.ok(
|
||||
settingsOut.includes('USE_WORKTREES_CURRENT=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null)'),
|
||||
`${rt}/settings.md: the bare tri-state worktrees read must survive per-runtime stamping byte-identical`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('claude emit of settings.md and health.md keeps the recommended Yes option unchanged', () => {
|
||||
const settingsOut = conversion._applyRuntimeRewrites(readWorkflow('settings.md'), 'claude', '$HOME/.claude/', true, undefined);
|
||||
assert.ok(
|
||||
settingsOut.includes('{ label: "Yes (Recommended)", description: "Each parallel executor runs in its own worktree branch — no conflicts between agents." }'),
|
||||
'claude/settings.md: the worktree-capable Worktrees question must be unchanged',
|
||||
);
|
||||
});
|
||||
|
||||
test('settings.md source carries the isolation-none branch: no enabling option, never persist true', () => {
|
||||
const src = readWorkflow('settings.md');
|
||||
assert.ok(
|
||||
src.includes('**Conditional options — Worktrees (#2486):**'),
|
||||
'settings.md: missing the conditional-options block for the Worktrees question',
|
||||
);
|
||||
// Round 4: the current-value read carries NO --default/fallback on purpose.
|
||||
// _stampNonClaudeRuntimeDefaults rewrites the canonical fallback line to
|
||||
// `--default false || echo "false"` on every non-Claude emit, which made
|
||||
// key-absence indistinguishable from an explicit false — and the
|
||||
// "pre-select Leave unchanged only when absent" rule dead there. The bare
|
||||
// read signals absence as empty output and matches no stamp pattern.
|
||||
assert.ok(
|
||||
src.includes('USE_WORKTREES_CURRENT=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null)'),
|
||||
'settings.md: current worktrees value must use the bare tri-state read (empty = absent)',
|
||||
);
|
||||
assert.ok(
|
||||
!src.includes(`USE_WORKTREES_CURRENT=$(gsd_run query ${TRUE_WT_LINE})`),
|
||||
'settings.md: the stampable fallback read collapses absent into false on non-Claude emits',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('NEVER write `workflow.use_worktrees: true` from this workflow when the runtime declares no isolation primitive'),
|
||||
'settings.md: missing the never-persist-true instruction for isolation-none runtimes',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('{ label: "No (Recommended)", description: "Write use_worktrees: false.'),
|
||||
'settings.md: isolation-none branch must recommend No',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('{ label: "Leave unchanged", description: "Do not write the key.'),
|
||||
'settings.md: isolation-none branch must offer leaving the key untouched for shared configs',
|
||||
);
|
||||
// Round 2: in the broken-inheritance case (explicit non-false value) the
|
||||
// pre-selected default must be the repair, not "Leave unchanged".
|
||||
// Round-5 review: this used to pin `Pre-select "Leave unchanged" only
|
||||
// when the key is absent`. That assumed an absent key is already safe,
|
||||
// which holds only on an emit that stamped the default to false —
|
||||
// execute-phase/quick/diagnose read it as `|| echo "true"` otherwise. The
|
||||
// recommended default must now be the explicit repair in every case.
|
||||
assert.ok(
|
||||
src.includes('Pre-select "No (Recommended)" in every case, including an absent key'),
|
||||
'settings.md: the recommended default must be the explicit false — an absent key is safe only on a stamped emit',
|
||||
);
|
||||
assert.ok(
|
||||
!/Pre-select "Leave unchanged" only when the key is absent/.test(src),
|
||||
'settings.md: the old absent-key-is-safe pre-selection rule is falsified on un-stamped emits (#2486 round-5 review)',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('when it is an explicit non-false value — the broken-inheritance case'),
|
||||
'settings.md: the broken-inheritance case must pre-select the recommended repair',
|
||||
);
|
||||
});
|
||||
|
||||
test('health.md source carries the W025 isolation/worktrees compatibility check', () => {
|
||||
const src = readWorkflow('health.md');
|
||||
assert.ok(
|
||||
src.includes('W025:'),
|
||||
'health.md: missing the W025 diagnostic line',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('Status: DEGRADED'),
|
||||
'health.md: a config the execution workflows fail closed on must degrade the reported status',
|
||||
);
|
||||
// #2486 review Major 1: W025's correctness must NOT depend on
|
||||
// `_stampNonClaudeRuntimeDefaults` having rewritten the read. A
|
||||
// `|| echo "true"` fallback makes an ABSENT key look like an explicit
|
||||
// `true`, so the check fires on a config that never set it — correct only
|
||||
// on a stamped emit, wrong on the un-stamped source/Claude emit. Pin the
|
||||
// stamp-independent shape settings.md already uses.
|
||||
assert.ok(
|
||||
src.includes('USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null)'),
|
||||
'health.md: the W025 worktrees read must be bare — a --default/|| fallback collapses key-absent into explicit-true (#2486 Major 1)',
|
||||
);
|
||||
assert.ok(
|
||||
!/workflow\.use_worktrees --raw 2>\/dev\/null \|\| echo/.test(src),
|
||||
'health.md: the W025 read reintroduced a fallback, so it depends on install-time stamping again (#2486 Major 1)',
|
||||
);
|
||||
assert.ok(
|
||||
src.includes('[ -n "$USE_WORKTREES" ]'),
|
||||
'health.md: W025 must require the key to be PRESENT before warning that the config "sets" it',
|
||||
);
|
||||
});
|
||||
|
||||
// #2486 review Major 2: the coverage above is still text-matching. Execute
|
||||
// the SHIPPED block so the predicate is proven by behavior — Major 1 would
|
||||
// have been caught by any one of these cases.
|
||||
test('W025 fires only on an explicit non-false key under isolation=none (behavioral, #2486 Major 2)', { skip: NO_BASH }, (t) => {
|
||||
const scratch = createTempDir('gsd-w025-');
|
||||
t.after(() => cleanupDir(scratch));
|
||||
const src = readFileNormalized(
|
||||
path.join(__dirname, '..', 'gsd-core', 'workflows', 'health.md'),
|
||||
);
|
||||
const block = [...src.matchAll(/```bash\r?\n([\s\S]*?)```/g)]
|
||||
.map(m => m[1])
|
||||
.find(b => b.includes('W025:'));
|
||||
assert.ok(block, 'health.md: no ```bash block containing the W025 diagnostic');
|
||||
|
||||
/** Run the shipped block with `gsd_run` stubbed to the given answers. */
|
||||
const fire = (isolation, worktreesOut) => {
|
||||
const harness = [
|
||||
'set -u',
|
||||
'gsd_run() {',
|
||||
' case "$*" in',
|
||||
` *inspect-dispatch-isolation*) printf '%s' ${JSON.stringify(isolation)} ;;`,
|
||||
// Faithful to the real CLI: `config-get` EXITS NON-ZERO for an absent
|
||||
// key, it does not print empty and succeed. A stub that succeeds here
|
||||
// would let `|| echo "true"` be reintroduced and still pass — the
|
||||
// fallback only triggers on failure (#2486 round-7 review, Major 4).
|
||||
worktreesOut === ''
|
||||
? ' *"config-get workflow.use_worktrees"*) return 1 ;;'
|
||||
: ` *"config-get workflow.use_worktrees"*) printf '%s' ${JSON.stringify(worktreesOut)} ;;`,
|
||||
" *) printf '' ;;",
|
||||
' esac; }',
|
||||
block,
|
||||
].join('\n');
|
||||
const scriptPath = path.join(scratch, `w025-${isolation}-${worktreesOut || 'absent'}.sh`);
|
||||
fs.writeFileSync(scriptPath, harness);
|
||||
const res = runHook(scriptPath, [], { interpreter: 'bash' });
|
||||
assert.equal(
|
||||
res.outcome, 'exited',
|
||||
`W025 block did not complete cleanly: outcome=${res.outcome} ${res.stderr || ''}`,
|
||||
);
|
||||
assert.equal(res.exitCode, 0, `W025 block exited ${res.exitCode}: ${res.stderr}`);
|
||||
return res.stdout.includes('W025:');
|
||||
};
|
||||
|
||||
// The reviewer's exact repro: rt=qwen, source (un-stamped) emit, cfg={}.
|
||||
// The key is absent, so `config-get` prints nothing. This FIRED before.
|
||||
assert.equal(
|
||||
fire('none', ''),
|
||||
false,
|
||||
'W025 fired on an ABSENT use_worktrees key — the warning claims the config "sets" a non-false value, and it does not (#2486 Major 1 repro)',
|
||||
);
|
||||
// The defect W025 actually exists for.
|
||||
assert.equal(
|
||||
fire('none', 'true'),
|
||||
true,
|
||||
'W025 stayed quiet on an explicit true under isolation=none — that is the config execute-phase/quick fail closed on',
|
||||
);
|
||||
assert.equal(
|
||||
fire('none', 'false'), false, 'W025 fired on an explicit false — nothing to repair',
|
||||
);
|
||||
// A host that CAN isolate is never the subject of this warning.
|
||||
for (const iso of ['harness-worktree', 'orchestrator-worktree']) {
|
||||
assert.equal(
|
||||
fire(iso, 'true'),
|
||||
false,
|
||||
`W025 fired on ${iso}, which declares an isolation primitive — the gate is the declared capability, not the runtime name (#2584)`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// #2486 round-9 review, Major 3: the source-text pins above assert only
|
||||
// that ISOLATION_RESOLVED is *mentioned*, so flipping the shipped block's
|
||||
// `ISOLATION_RESOLVED=true` to `false` left every one of them green. What
|
||||
// has to be pinned is the BRANCH: a resolver that answered and a resolver
|
||||
// that failed must produce different W025 text, because the whole point is
|
||||
// to stop reporting a failed query as a capability verdict. This test
|
||||
// drives the shipped block twice and fails on the mutation.
|
||||
test('W025 distinguishes a declared none from an unresolvable query (behavioral, #2486 Major 3)', { skip: NO_BASH }, (t) => {
|
||||
const scratch = createTempDir('gsd-w025-provenance-');
|
||||
t.after(() => cleanupDir(scratch));
|
||||
const src = readFileNormalized(
|
||||
path.join(__dirname, '..', 'gsd-core', 'workflows', 'health.md'),
|
||||
);
|
||||
const block = [...src.matchAll(/```bash\r?\n([\s\S]*?)```/g)]
|
||||
.map(m => m[1])
|
||||
.find(b => b.includes('W025:'));
|
||||
assert.ok(block, 'health.md: no ```bash block containing the W025 diagnostic');
|
||||
|
||||
// `resolves` false = the inspect call exits non-zero, the real shape of a
|
||||
// shim that cannot resolve. The worktrees key is an explicit true in both
|
||||
// runs, so the ONLY difference is whether the capability query answered.
|
||||
const runBlock = (resolves) => {
|
||||
const harness = [
|
||||
'set -u',
|
||||
'gsd_run() {',
|
||||
' case "$*" in',
|
||||
resolves
|
||||
? " *inspect-dispatch-isolation*) printf 'none' ;;"
|
||||
: ' *inspect-dispatch-isolation*) return 1 ;;',
|
||||
" *\"config-get workflow.use_worktrees\"*) printf 'true' ;;",
|
||||
" *) printf '' ;;",
|
||||
' esac; }',
|
||||
block,
|
||||
].join('\n');
|
||||
const scriptPath = path.join(scratch, `w025-resolves-${resolves}.sh`);
|
||||
fs.writeFileSync(scriptPath, harness);
|
||||
const res = runHook(scriptPath, [], { interpreter: 'bash' });
|
||||
assert.equal(res.outcome, 'exited', `block did not complete: ${res.stderr || ''}`);
|
||||
assert.equal(res.exitCode, 0, `block exited ${res.exitCode}: ${res.stderr}`);
|
||||
return res.stdout;
|
||||
};
|
||||
|
||||
const resolved = runBlock(true);
|
||||
const unresolved = runBlock(false);
|
||||
|
||||
// Both must warn — an explicit true is worth reporting either way.
|
||||
assert.match(resolved, /W025:/, 'a resolved none with an explicit true must still warn');
|
||||
assert.match(unresolved, /W025:/, 'an unresolvable capability with an explicit true must still warn');
|
||||
|
||||
// ...but they must not say the same thing.
|
||||
assert.notEqual(
|
||||
resolved.trim(),
|
||||
unresolved.trim(),
|
||||
'W025 emitted identical text whether or not the capability resolved — that is the Major 3 conflation',
|
||||
);
|
||||
assert.match(
|
||||
unresolved,
|
||||
/could not resolve/i,
|
||||
'the unresolved branch must report that the query failed, not assert a capability verdict',
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
unresolved,
|
||||
/declares no executor-isolation primitive|has no usable executor-isolation primitive/i,
|
||||
'the unresolved branch must NOT claim the runtime has no primitive — nothing established that',
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
resolved,
|
||||
/could not resolve/i,
|
||||
'the resolved branch must state the capability finding, not a resolution failure',
|
||||
);
|
||||
// Round-3 review: asserting only "differs and omits the other phrase"
|
||||
// would stay green if the resolved text were replaced with anything at
|
||||
// all. Pin what it must positively say — the capability finding and the
|
||||
// consequence a user acts on.
|
||||
assert.match(
|
||||
resolved,
|
||||
/no usable executor-isolation primitive/i,
|
||||
'the resolved branch must state the capability finding it actually reached',
|
||||
);
|
||||
assert.match(
|
||||
resolved,
|
||||
/fail closed/i,
|
||||
'the resolved branch must state the consequence — that is what makes W025 actionable',
|
||||
);
|
||||
assert.match(
|
||||
resolved,
|
||||
/use_worktrees/,
|
||||
'the resolved branch must name the offending key',
|
||||
);
|
||||
// Both branches must still route the user to the same repair.
|
||||
for (const [name, out] of [['resolved', resolved], ['unresolved', unresolved]]) {
|
||||
assert.match(out, /(?:\/gsd[:-]settings|\$gsd-settings)\b/, `${name}: W025 must name the repair command`);
|
||||
}
|
||||
});
|
||||
|
||||
test('W025 is documented consistently across health.md and both config references', () => {
|
||||
// The rename W020 -> W025 landed in health.md only; the two docs kept
|
||||
// saying W020, which collides with a code src/verify.cts already emits.
|
||||
for (const rel of ['gsd-core/workflows/health.md', 'docs/CONFIGURATION.md', 'gsd-core/references/planning-config.md']) {
|
||||
const text = fs.readFileSync(path.join(__dirname, '..', rel), 'utf8');
|
||||
assert.ok(text.includes('W025'), `${rel}: must document the worktrees warning as W025`);
|
||||
assert.ok(
|
||||
!/\bW020\b[^)]{0,80}worktree/i.test(text),
|
||||
`${rel}: stale W020 reference for the worktrees warning`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// Round 4 note: an earlier revision carried a W025-vs-src/verify.cts
|
||||
// namespace-collision test here that read verify.cts as raw text — the
|
||||
// source-grep shape RULESET.TESTS.delete-bad-tests says to delete, not
|
||||
// exempt. Deleted without a behavioral replacement: verify.cts exposes no
|
||||
// enumerable W-code registry to assert against, and building one is a
|
||||
// shared-module refactor outside this fix. The namespace claim lives as
|
||||
// guidance in health.md's error-codes note instead of as a fake test.
|
||||
|
||||
test('the health.md error-codes table is not broken by the namespace note', () => {
|
||||
// The note was inserted BETWEEN two rows, which terminates the GFM table
|
||||
// and orphans the I001 row into literal pipe-delimited text.
|
||||
const src = readWorkflow('health.md');
|
||||
const w025 = src.indexOf('| W025 |');
|
||||
const i001 = src.indexOf('| I001 |');
|
||||
const note = src.indexOf('Note: the `W0NN` warning-code namespace');
|
||||
assert.ok(w025 > -1 && i001 > -1 && note > -1, 'health.md: expected W025, I001 and the namespace note');
|
||||
assert.ok(i001 > w025, 'health.md: I001 row must follow the W025 row');
|
||||
assert.ok(
|
||||
note > i001,
|
||||
'health.md: the namespace note must come AFTER the final table row — placing it between rows ends the table and orphans I001',
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// Folded from tests/bug-2876-skill-frontmatter-quote.test.cjs — consolidation epic #1969 (B8 #1977)
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user