diff --git a/.changeset/sturdy-birds-chatter.md b/.changeset/sturdy-birds-chatter.md new file mode 100644 index 000000000..6f1ffe8f6 --- /dev/null +++ b/.changeset/sturdy-birds-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2531 +--- +**Settings no longer offer worktree isolation on runtimes that cannot honor it, and health warns before execution fails closed** — previously `/gsd-settings` recommended "Yes" and persisted `workflow.use_worktrees: true` on every runtime, handing installs whose declared `dispatch.isolation` capability is `none` the exact value `/gsd-execute-phase` and `/gsd-quick` fail closed on. On those runtimes the Worktrees question now offers "No (Recommended)" / "Leave unchanged" (never an enabling option), warns when the config carries an inherited explicit `true`, and `/gsd-health` surfaces such a config as new warning W025 with a DEGRADED status before execution-time failure. Runtimes that declare `harness-worktree` or `orchestrator-worktree` are unaffected — the gate is the declared capability, never the runtime name. Both surfaces resolve isolation through the new `inspect-dispatch-isolation` query, a sentinel-free sibling of `dispatch-isolation`: the dispatch verb records its decision to the executor-isolation sentinel by design, which a read-only diagnostic must never trigger. The inspection verb rejects `--force-isolation`, `--phase` and `--plan` as usage errors rather than accepting and ignoring them — the recording verb applies `--force-isolation` after resolution, so silently ignoring it would hand the same argv two different answers. Both surfaces also distinguish "this runtime declares no isolation primitive" from "the capability could not be resolved", and say which one happened instead of reporting a resolver failure as a capability verdict. (#2486) diff --git a/CONTEXT.md b/CONTEXT.md index aba850889..f96415a51 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -138,7 +138,7 @@ Diagnostic-output convention for the Resolution Provenance principle (ADR-1411 P Leaf module owning the **out-of-band** half of ADR-1411's "corrupt is not absent" amendment (epic #1879). Where a read already returns a provenance envelope the cause is named in-band (`ConfigResolution.reason`, #1880); where a read returns a bare sentinel or a plausible default it cannot extend, the return value is preserved exactly and the cause is surfaced here instead. Interface: `UNUSABLE_REASON` (frozen reason enum — one entry per condition that has an emitting call site; adding a reason is three coordinated changes: enum + call site + the test locking `Object.keys(...).sort()`), `warnUnusableInput({reason, source?, content?}) → boolean` (returns whether this call actually wrote, so tests assert emission *counts* on a typed surface rather than scraping stderr), plus the `_resetUnusableInputWarningsForTests` / `_unusableInputWarningCountForTests` seams. Dedup key is `\0` — **both halves are load-bearing**: keying on the path alone would let a second, different fault on the same file go unreported, and keying on message prose would couple the guard to wording (ADR-1411 dedup clause). Path separators are deliberately **not** normalized: an earlier revision folded backslashes to `/` so two spellings of one Windows path would not double-report, but a backslash is a legal filename character on Linux and macOS, so that folding collapsed two genuinely distinct POSIX files onto one key and swallowed the second file's diagnostic. The trade is now one-directional — two spellings of one Windows path may report twice (noise), but two distinct files can never silence each other (lost signal), and ADR-1411 ranks the swallow the worse failure; ASCII control characters are stripped from the source before it is keyed or written, because the key separator is NUL (a crafted path could otherwise forge a collision) and because a path carrying ANSI escapes would replay into the operator's terminal. Callers with no path (in-memory content) fall back to a short content digest so *different* bad inputs still key differently. The diagnostic is **unconditional** — a deliberate divergence from ADR-227's never-implemented `GSD_DEBUG` opt-in, since "an opt-in nobody sets is indistinguishable from the silence #1879 is about" — and **never throws**: a failed stderr write is swallowed so a degraded read is never escalated into a crash. Adopted by `extractFrontmatter` (#1882, `frontmatter_unterminated`) and by `getRoadmapPhaseInternal`/`getMilestoneInfo` (#1881, `roadmap_unreadable`); `planning-workspace`/`verify` (#1883) follow. #1881 detects on the errno alone: `platformReadSync` returns `null` for ENOENT and its callers convert that to an errno-less Error, so reporting unconditionally in those catches would flag every project without a ROADMAP.md as corrupt. Exists as a shared seam rather than a per-site copy because four sites need identical behavior and four hand-rolled copies is `RULESET.GENERATIVE-FIX` by construction. Source of truth: `gsd-core/bin/lib/unusable-input.cjs` (generated from `src/unusable-input.cts`). Test anchor: `tests/unusable-input.test.cjs`. See Resolution Provenance, Config Loader Module. ### Worktree Safety Policy Module -CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult` (per-entry gauntlet: branch → base → deletions → **advisory scope conformance (#2596)** → SUMMARY-rescue → clean-worktree → merge → remove; the scope check compares the branch's committed diff against the entry's declared `files_modified` and appends `WAVE_CLEANUP_WARNING`-coded entries to a `warnings` channel WITHOUT touching `ok` — an advisory, not a gate, and skipped entirely with no git call when no scope was declared), `planWaveScopeConformance(changedPaths, declaredFiles, branch) → WaveCleanupWarning[]` (pure; literal-prefix path coverage deliberately mirroring the submodule-intersection gate's glob-prefix rule rather than introducing a second matcher; over-accepts by design because a false alarm costs an advisory more than a miss), `isSummaryArtifactRelPath(relPath) → boolean` (the single definition of "executor-written SUMMARY artifact", shared with `defaultFindSummaryFiles` so the rescue walker and the scope exemption cannot drift), `WAVE_CLEANUP_WARNING` (frozen advisory-code enum: `scope_out_of_declared`, `scope_check_unavailable`), `planWorktreeRecordAgent(manifestRaw, fields) → RecordAgentPlan` (write-strict per-agent manifest append; validates each field at write time via the same `normalizeCleanupManifestEntry` rules the reader enforces; fail-closed on a missing/garbled field or a duplicate `(worktree_path, branch)` the reader would dedup away), `cmdWorktreeRecordAgent(cwd, args, deps) → RecordAgentCmdResult` (thin deps-injectable IO wrapper for the `worktree record-agent` verb), `planWorktreeCreate(fields) → WorktreeCreatePlan` (write-strict `worktree create` planner — same missing-field-hint and `normalizeCleanupManifestEntry` validation as `planWorktreeRecordAgent`, pure/no-git), `executeWorktreeCreatePlan(plan, repoRoot, deps) → WorktreeCreateResult` (bounded `git rev-parse --verify` base check THEN `git worktree add -b `; fail-closed `base_unresolved`/`git_timeout`/`worktree_add_failed`; returns `cwd` — the working directory an executor spawn would use), `cmdWorktreeCreate(cwd, args, deps) → WorktreeCreateCmdResult` (CLI verb: requires `--root` — confinement is mandatory, not opt-in; omitting it fails closed with `reason:'root_required'` before any git side effect, rather than silently creating an unconfined worktree (#3050); plans, creates the worktree, then appends the manifest entry so it is immediately manageable by cleanup-wave/reap-orphans; dedupes by `(worktree_path, branch)`). #2584 ADR-1239 Codex-binding amendment, Phase 2: `worktree create` is the git-worktree-creation primitive for `dispatch.isolation: orchestrator-worktree` hosts — declared and testable but UNCONSUMED (no scheduler calls it yet; Phase 3 wires it). `worktree record-agent` / `worktree create` accept an optional `--files` recording the plan's declared scope, consumed by the advisory scope-conformance check above; a blank or omitted value leaves the 4-field on-disk entry shape unchanged. Source of truth: `gsd-core/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. The `core.cjs` re-export spine was retired in epic #1267: this module absorbed the two thin compositional wrappers that squatted in Core — `resolveWorktreeRoot(cwd, deps) → {root, reason}` (a projection over `resolveWorktreeContext`; returns the `reason` alongside `root` — a `git_timed_out` reason means `root` is a best-effort cwd fallback, not a confirmed resolution, and callers must surface that risk rather than trust it silently, #3050) and `pruneOrphanedWorktrees(...)` (sequences `planWorktreePrune` + `executeWorktreePrunePlan` with a timeout warning) — so callers reach this single worktree-lifecycle seam directly. `gitWorktreeInfoInternal` did NOT move here — worktree-info detection belongs to the Git Query Module. +CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult` (per-entry gauntlet: branch → base → deletions → **advisory scope conformance (#2596)** → SUMMARY-rescue → clean-worktree → merge → remove; the scope check compares the branch's committed diff against the entry's declared `files_modified` and appends `WAVE_CLEANUP_WARNING`-coded entries to a `warnings` channel WITHOUT touching `ok` — an advisory, not a gate, and skipped entirely with no git call when no scope was declared), `planWaveScopeConformance(changedPaths, declaredFiles, branch) → WaveCleanupWarning[]` (pure; literal-prefix path coverage deliberately mirroring the submodule-intersection gate's glob-prefix rule rather than introducing a second matcher; over-accepts by design because a false alarm costs an advisory more than a miss), `isSummaryArtifactRelPath(relPath) → boolean` (the single definition of "executor-written SUMMARY artifact", shared with `defaultFindSummaryFiles` so the rescue walker and the scope exemption cannot drift), `WAVE_CLEANUP_WARNING` (frozen advisory-code enum: `scope_out_of_declared`, `scope_check_unavailable`), `planWorktreeRecordAgent(manifestRaw, fields) → RecordAgentPlan` (write-strict per-agent manifest append; validates each field at write time via the same `normalizeCleanupManifestEntry` rules the reader enforces; fail-closed on a missing/garbled field or a duplicate `(worktree_path, branch)` the reader would dedup away), `cmdWorktreeRecordAgent(cwd, args, deps) → RecordAgentCmdResult` (thin deps-injectable IO wrapper for the `worktree record-agent` verb), `planWorktreeCreate(fields) → WorktreeCreatePlan` (write-strict `worktree create` planner — same missing-field-hint and `normalizeCleanupManifestEntry` validation as `planWorktreeRecordAgent`, pure/no-git), `executeWorktreeCreatePlan(plan, repoRoot, deps) → WorktreeCreateResult` (bounded `git rev-parse --verify` base check THEN `git worktree add -b `; fail-closed `base_unresolved`/`git_timeout`/`worktree_add_failed`; returns `cwd` — the working directory an executor spawn would use), `cmdWorktreeCreate(cwd, args, deps) → WorktreeCreateCmdResult` (CLI verb: requires `--root` — confinement is mandatory, not opt-in; omitting it fails closed with `reason:'root_required'` before any git side effect, rather than silently creating an unconfined worktree (#3050); plans, creates the worktree, then appends the manifest entry so it is immediately manageable by cleanup-wave/reap-orphans; dedupes by `(worktree_path, branch)`). #2584 ADR-1239 Codex-binding amendment, Phase 2: `worktree create` is the git-worktree-creation primitive for `dispatch.isolation: orchestrator-worktree` hosts — consumed since #2584 Phase 3 — `executor-isolation-dispatch.md` calls it to create the worktree an `orchestrator-worktree` host is then process-spawned into. `worktree record-agent` / `worktree create` accept an optional `--files` recording the plan's declared scope, consumed by the advisory scope-conformance check above; a blank or omitted value leaves the 4-field on-disk entry shape unchanged. Source of truth: `gsd-core/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. The `core.cjs` re-export spine was retired in epic #1267: this module absorbed the two thin compositional wrappers that squatted in Core — `resolveWorktreeRoot(cwd, deps) → {root, reason}` (a projection over `resolveWorktreeContext`; returns the `reason` alongside `root` — a `git_timed_out` reason means `root` is a best-effort cwd fallback, not a confirmed resolution, and callers must surface that risk rather than trust it silently, #3050) and `pruneOrphanedWorktrees(...)` (sequences `planWorktreePrune` + `executeWorktreePrunePlan` with a timeout warning) — so callers reach this single worktree-lifecycle seam directly. `gitWorktreeInfoInternal` did NOT move here — worktree-info detection belongs to the Git Query Module. ### Worktree Lifecycle Module Workflow contract seam covering agent worktree lifecycle orchestration rules. The `worktree_branch_check` block lives in one canonical fragment (`gsd-core/references/worktree-branch-check.md`) that `execute-phase.md`, `quick.md`, `diagnose-issues.md`, and `execute-plan.md` embed at dispatch. Key invariants: `worktree_branch_check` is **verify-only and fail-closed** — the orchestrator owns worktree lifecycle and base recovery, so the sub-agent holds no state-correction primitives; HEAD attachment verified via `git symbolic-ref`; positive allow-list `^worktree-agent-*` enforced; `git update-ref` on protected refs is prohibited; on base mismatch the sub-agent halts with `exit 42` and surfaces to the orchestrator (#48); the orchestrator runs a cwd-drift guard at `execute_waves` entry that resolves the worktree root and refuses drift into an agent worktree (#48); #1856: that refusal now also reports what the agent worktree holds — commits ahead of the resolved base and uncommitted files, both with true counts plus a truncation notice — and the commit/switch/merge-or-cherry-pick sequence to integrate them, because `re-run from the orchestrator worktree` alone silently meant abandoning work that lives only on the agent branch. The refusal condition and exit code are unchanged, and every added command is diagnostic and `|| true`-guarded so a failure degrades to the plain refusal; cleanup is manifest-scoped (`WAVE_WORKTREE_MANIFEST`) not global-discovery-based; worktree spawning is sequential (one `run_in_background` at a time to avoid `config.lock` contention). Test anchor: `tests/worktree.test.cjs`. @@ -159,7 +159,7 @@ Module owning runtime identity normalization at runtime-selection seams. Canonic Pure, no-write Module owning the **detection rung** of runtime identity — ADR-2313 Phase 5 (#3245, folded from #2320). The Runtime Name Policy Module normalizes the two *explicit* signals (`GSD_RUNTIME`, `.planning/config.json:runtime`); this module answers the different question those two cannot: *which host is this process actually running inside* when neither is set. Before it, `init` reported `agent_runtime: claude` inside a Codex session, because the ladder ended at a hardcoded default. `detectHostRuntime(deps?) → {runtime, source, signal}` is the typed surface tests assert against (`source` ∈ `session-env|config-home|none`); it probes, in order, the frozen `CODEX_SESSION_ENV_SIGNALS` table (`CODEX_SANDBOX`, `CODEX_SANDBOX_NETWORK_DISABLED` — injected by Codex into shell-tool children per openai/codex `AGENTS.md`; absent under `sandbox_mode = "danger-full-access"`, so best-effort), then an explicitly-exported `CODEX_HOME` whose `config.toml` exists — the marker FILENAME is single-sourced from the Update-Context Module's `inferPreferredRuntime` (`CODEX_CONFIG_MARKER`, re-exported here), while the TRUTHINESS RULE deliberately differs: `inferPreferredRuntime` accepts a bare, unchecked `CODEX_HOME` as sufficient to resolve an update context, whereas this module additionally requires the marker file to exist, because it asserts session identity rather than resolving an update context and needs the stronger signal — a difference pinned by a test in `tests/host-runtime-detection.test.cjs` rather than left implicit. `resolveReportedRuntime(projectDir, deps?)` composes the whole ladder: `GSD_RUNTIME` > config `runtime` > detection > `'claude'`. Three invariants are load-bearing and each has a test: it **never writes** (#2297 shared-defaults poisoning — no `~/.gsd/defaults.json`, no config mutation); it **never shells out**, so there is no subprocess to time-bound and no degraded-on-timeout path to design; and the **default `~/.codex/config.toml` is never probed**, because every machine that has run Codex carries that file and probing it would misreport Claude Code sessions as codex. _Avoid_: calling this "runtime resolution" — `resolveRuntime` (Runtime Slash Module) keeps its own frozen `GSD_RUNTIME > config > 'claude'` contract and its 71 dependents, including `formatGsdSlash`'s command-style decision, are deliberately untouched; only `withProjectRoot`'s reported `agent_runtime` consumes the detection rung. Sources: `src/host-runtime-detection.cts` → `gsd-core/bin/lib/host-runtime-detection.cjs`; the `resolveExplicitRuntime` seam it composes lives in `src/runtime-slash.cts`. See ADR-2313. ### Host-Integration Interface -Pure, additive, no-I/O Module owning the versioned, negotiated contract over the six host-integration interface points (command, dispatch, model, hooks, state, artifact) — ADR-1239 Phase A. Extends the ADR-1016 runtime descriptor with nine closed-vocabulary axes carried under `capability.json` `runtime.hostIntegration`: `embeddingMode` (`imperative|declarative`), `commandSurface` (`slash-file|slash-programmatic|slash-toml|palette|prose-only`), `dispatch` (`{namedDispatch,nested,maxDepth,background,backgroundDispatch,subagentToolkit,isolation}`), `modelMode` (`active|passive`), `hookBus` (`host|engine|none`), `stateIO` (`filesystem|sandboxed-storage|session-log-append`), `transport` (`mcp|native-extension`), `runtime` (`node|bun|sandboxed-web|python|go|rust|electron|other`), `effortSurface` (`argv|none` — how reasoning effort reaches the host; ADR-1239 amendment #2481, the first axis whose consumer is an invocation-time argument rather than an install-time artifact). `dispatch.isolation` (`harness-worktree|orchestrator-worktree|none` — how a host isolates concurrent same-wave executors; ADR-1239 Codex-binding amendment #2584; consumed by the phase scheduler since #2584 Phase 3 and, since #2652, by every single-agent dispatch site — `quick.md`, `diagnose-issues.md`, `execute-plan.md` — which resolve it through the canonical `gsd-core/references/dispatch-isolation-gate.md` rather than branching on a runtime id). `resolveOrchestratorExec(orchestratorExec, cwd) → { ok:true, command, args, cwd } | { ok:false, reason }` (#2584 Phase 2, pure, no I/O — resolves the `runtime.orchestratorExec` descriptor field, a sibling of `runtime.hostBehaviors` in `capability.json` carrying `{command, args?, cwdFlag?}`, into the concrete argv/cwd a process-spawn primitive would use for a `dispatch.isolation: orchestrator-worktree` host; appends `[cwdFlag, cwd]` to `args` when `cwdFlag` is a non-empty string, e.g. codex `exec --cd `, opencode `run --dir `, kimi `--work-dir `; when `cwdFlag` is `null`/absent — kimi-code's process-cwd case — no flag is appended and `cwd` alone is returned for the caller to bind via the subprocess's own working-directory option; fail-closed `missing_command`/`invalid_cwd`/`invalid_args`/`invalid_cwd_flag`; CONSUMED since #2584 Phase 3 — `routeDispatchIsolation` resolves it into the `exec` field of `gsd_run query dispatch-isolation --json`, and `gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md` process-spawns that `command`/`args`/`cwd`; #2652 adds a second consumer, `_negotiatedDispatchIsolation`, which probes it at install time against a placeholder target to decide whether an `orchestrator-worktree` declaration actually resolves). Interface: `negotiateHostCapabilities(host, engine?) → { protocolVersion, effective, points, warnings }` enforcing the trust-boundary invariant `effective ⊆ host-declared ∩ engine-known` (never augment with an undeclared or unknown/future-`protocolVersion` value — fail-closed via the most-restrictive-known `SAFE_DEFAULTS`); `degradationFor(point, axes) → { level, fallback }` (a pure Full/Degraded/Absent ladder table, never throws); `profileOf(axes) → 'programmatic-cli'|'declarative-cli'|'ide'|null`; plus `PROTOCOL_VERSION` (integer, starts at 1 — distinct from the package `version`/`engines.gsd` semver), `HOST_INTEGRATION_AXES` (the frozen closed vocabulary, single source of truth), `PROFILE_BASELINES`, and `shouldFlattenDispatch(dispatch) → boolean` (ADR-1239 Phase B / #1708 — graduates the #853 rule: returns `true` = run the orchestrator inline UNLESS the host is documented to background a nesting-capable orchestrator (`background === true && backgroundDispatch === true`); fail-closed to inline; exposed to the plan/execute workflows via the `gsd_run query dispatch-should-flatten --raw` CLI, which replaced the former scattered `RUNTIME === 'codex'` prose check). The runtime-descriptor validator (`gsd-core/bin/lib/capability-validator.cjs` `validateRuntimeBody`) mirrors the closed vocabulary inline (exported as `_HOST_INTEGRATION_VOCAB`) and is kept in lock-step by the parity guard `tests/host-integration-validator-parity.test.cjs`. Orthogonal axes (resolved explicitly per ADR-1239 Phase A): `commandStyle` (GSD emission style, retained) vs `commandSurface` (host surface type); `hookEvents` dialect vs `hookBus` ownership (a host with `hooksSurface:none` may still be `hookBus:host` — e.g. opencode); `runtimeCompat` (feature→host) vs these negotiated runtime→engine axes. Phase A defined the interface; Phase B (#1679) wires it incrementally — `destSubpath` write-confinement (#1704) and the typed documentation-sourced #853 dispatch-flatten (#1708, the first consumer of a negotiated `dispatch` axis); adapters/MCP/host-bindings remain Phases C–E. Source of truth: `gsd-core/bin/lib/host-integration.cjs` (generated from `src/host-integration.cts`). See ADR-1239 and ADR-1016. +Pure, additive, no-I/O Module owning the versioned, negotiated contract over the six host-integration interface points (command, dispatch, model, hooks, state, artifact) — ADR-1239 Phase A. Extends the ADR-1016 runtime descriptor with nine closed-vocabulary axes carried under `capability.json` `runtime.hostIntegration`: `embeddingMode` (`imperative|declarative`), `commandSurface` (`slash-file|slash-programmatic|slash-toml|palette|prose-only`), `dispatch` (`{namedDispatch,nested,maxDepth,background,backgroundDispatch,subagentToolkit,isolation}`), `modelMode` (`active|passive`), `hookBus` (`host|engine|none`), `stateIO` (`filesystem|sandboxed-storage|session-log-append`), `transport` (`mcp|native-extension`), `runtime` (`node|bun|sandboxed-web|python|go|rust|electron|other`), `effortSurface` (`argv|none` — how reasoning effort reaches the host; ADR-1239 amendment #2481, the first axis whose consumer is an invocation-time argument rather than an install-time artifact). `dispatch.isolation` (`harness-worktree|orchestrator-worktree|none` — how a host isolates concurrent same-wave executors; ADR-1239 Codex-binding amendment #2584; consumed by the phase scheduler since #2584 Phase 3 and, since #2652, by every single-agent dispatch site — `quick.md`, `diagnose-issues.md`, `execute-plan.md` — which resolve it through the canonical `gsd-core/references/dispatch-isolation-gate.md` rather than branching on a runtime id; and, since #2486, by the runtime-neutral diagnostics — `/gsd:settings` gates its Worktrees recommendation and `/gsd:health` raises W025 from this axis, read through the sentinel-free `inspect-dispatch-isolation` verb). `resolveOrchestratorExec(orchestratorExec, cwd) → { ok:true, command, args, cwd } | { ok:false, reason }` (#2584 Phase 2, pure, no I/O — resolves the `runtime.orchestratorExec` descriptor field, a sibling of `runtime.hostBehaviors` in `capability.json` carrying `{command, args?, cwdFlag?}`, into the concrete argv/cwd a process-spawn primitive would use for a `dispatch.isolation: orchestrator-worktree` host; appends `[cwdFlag, cwd]` to `args` when `cwdFlag` is a non-empty string, e.g. codex `exec --cd `, opencode `run --dir `, kimi `--work-dir `; when `cwdFlag` is `null`/absent — kimi-code's process-cwd case — no flag is appended and `cwd` alone is returned for the caller to bind via the subprocess's own working-directory option; fail-closed `missing_command`/`invalid_cwd`/`invalid_args`/`invalid_cwd_flag`; CONSUMED since #2584 Phase 3 — `routeDispatchIsolation` resolves it into the `exec` field of `gsd_run query dispatch-isolation --json`, and `gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md` process-spawns that `command`/`args`/`cwd`; #2652 adds a second consumer, `_negotiatedDispatchIsolation`, which probes it at install time against a placeholder target to decide whether an `orchestrator-worktree` declaration actually resolves). Interface: `negotiateHostCapabilities(host, engine?) → { protocolVersion, effective, points, warnings }` enforcing the trust-boundary invariant `effective ⊆ host-declared ∩ engine-known` (never augment with an undeclared or unknown/future-`protocolVersion` value — fail-closed via the most-restrictive-known `SAFE_DEFAULTS`); `degradationFor(point, axes) → { level, fallback }` (a pure Full/Degraded/Absent ladder table, never throws); `profileOf(axes) → 'programmatic-cli'|'declarative-cli'|'ide'|null`; plus `PROTOCOL_VERSION` (integer, starts at 1 — distinct from the package `version`/`engines.gsd` semver), `HOST_INTEGRATION_AXES` (the frozen closed vocabulary, single source of truth), `PROFILE_BASELINES`, and `shouldFlattenDispatch(dispatch) → boolean` (ADR-1239 Phase B / #1708 — graduates the #853 rule: returns `true` = run the orchestrator inline UNLESS the host is documented to background a nesting-capable orchestrator (`background === true && backgroundDispatch === true`); fail-closed to inline; exposed to the plan/execute workflows via the `gsd_run query dispatch-should-flatten --raw` CLI, which replaced the former scattered `RUNTIME === 'codex'` prose check). The runtime-descriptor validator (`gsd-core/bin/lib/capability-validator.cjs` `validateRuntimeBody`) mirrors the closed vocabulary inline (exported as `_HOST_INTEGRATION_VOCAB`) and is kept in lock-step by the parity guard `tests/host-integration-validator-parity.test.cjs`. Orthogonal axes (resolved explicitly per ADR-1239 Phase A): `commandStyle` (GSD emission style, retained) vs `commandSurface` (host surface type); `hookEvents` dialect vs `hookBus` ownership (a host with `hooksSurface:none` may still be `hookBus:host` — e.g. opencode); `runtimeCompat` (feature→host) vs these negotiated runtime→engine axes. Phase A defined the interface; Phase B (#1679) wires it incrementally — `destSubpath` write-confinement (#1704) and the typed documentation-sourced #853 dispatch-flatten (#1708, the first consumer of a negotiated `dispatch` axis); adapters/MCP/host-bindings remain Phases C–E. Source of truth: `gsd-core/bin/lib/host-integration.cjs` (generated from `src/host-integration.cts`). See ADR-1239 and ADR-1016. ### Statusline Host-integration hook (`hooks/gsd-statusline.js`) that renders the session status line: model name, context-window meter, workspace directory, and the GSD-state segment (`formatGsdState()` projecting `.planning/` STATE.md). `readGsdState()` is workstream-aware (#2850): when the walk-up finds no flat `.planning/STATE.md` but lands on a `.planning/workstreams/` directory, it resolves the active workstream via `resolveActiveWorkstream` (`active-workstream-store.cts`), called with an empty args array — only its env>store precedence applies for this caller, since the CLI leg is inert without argv — and `planningPaths`/`listAvailableWorkstreams` (`planning-workspace.cts`) for path/mode resolution, the same seams every other workstream-aware command uses, and reads that workstream's `STATE.md` instead. The store tier is `peekActiveWorkstream`, a read-only sibling of `getActiveWorkstream` that never deletes a stale/invalid pointer file — a renderer invoked on every prompt must never mutate persistent state as a side effect of drawing a screen (`getActiveWorkstream`'s self-heal is correct for a command, not a render). When workstream mode is detected but nothing resolves, it returns a `{noActiveWorkstream:true}` sentinel that `formatGsdState`/`formatGsdStateCompact` render as `"no active workstream"` — observable, never silent emptiness. Opt-in segments are gated by `.planning/config.json` keys (`statusline.show_last_command`, `statusline.context_position`, plus the approved `statusline.show_context_tokens` and `statusline.state_format`), each registered across `gsd-core/bin/shared/config-schema.manifest.json` + `src/config.cts` + the `loadConfig` whitelist + `docs/CONFIGURATION.md`. The compact GSD-state format consumes the canonical status vocabulary from `normalizeStateStatus()` (STATE.md Document Module) rather than a parallel keyword list. **Data-source boundary (ADR-2164):** the statusline sources only local, read-only data — it refines the stdin payload Claude Code already sends and may add a new *local* source (e.g. `git`), but does not read credentials or call external/network APIs for data; account/usage/platform-level state is out of scope. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 95774c72f..ac5a326c3 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -345,7 +345,7 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin | `workflow.max_discuss_passes` | number | `3` | Maximum number of question rounds in discuss-phase before the workflow stops asking. Useful in headless/auto mode to prevent infinite discussion loops. | | `workflow.skip_discuss` | boolean | `false` | When `true`, `/gsd-autonomous` bypasses the discuss-phase entirely, writing minimal CONTEXT.md from the ROADMAP phase goal. Useful for projects where developer preferences are fully captured in PROJECT.md/REQUIREMENTS.md. Added in v1.28 | | `workflow.text_mode` | boolean | `false` | Replaces AskUserQuestion TUI menus with plain-text numbered lists. Required for Claude Code remote sessions (`/rc` mode) where TUI menus don't render. Can also be set per-session with `--text` flag on discuss-phase. Added in v1.28 | -| `workflow.use_worktrees` | boolean | `true` | When `false`, disables git worktree isolation for parallel execution. Users who prefer sequential execution or whose environment does not support worktrees can disable this. Added in v1.31. **Branch-divergence note:** when your branch has diverged from `origin/HEAD`, GSD auto-degrades to sequential and prints a warning. See [`worktree.baseRef`](#worktree-settings) to restore parallel execution on a diverged branch. **Per-runtime note:** whether this key can be honored depends on the runtime's declared `dispatch.isolation` capability, not on its name (#2584). Runtimes whose own harness isolates each executor (**Claude Code**, **Cursor**) run parallel worktrees natively; runtimes exposing a headless exec with an explicit working directory (**Codex**, **OpenCode**, **Kimi**, **Kimi Code**) get worktrees GSD itself creates and merges — where a dispatch site can only drive the harness model, those hosts degrade to sequential with a warning rather than aborting. Every other runtime declares no isolation primitive, and forcing `use_worktrees: true` there still fails closed before any executor dispatch. See [Executor isolation per runtime](#executor-isolation-per-runtime). | +| `workflow.use_worktrees` | boolean | `true` | When `false`, disables git worktree isolation for parallel execution. Users who prefer sequential execution or whose environment does not support worktrees can disable this. Added in v1.31. **Branch-divergence note:** when your branch has diverged from `origin/HEAD`, GSD auto-degrades to sequential and prints a warning. See [`worktree.baseRef`](#worktree-settings) to restore parallel execution on a diverged branch. **Per-runtime note:** whether this key can be honored depends on the runtime's declared `dispatch.isolation` capability, not on its name (#2584). Runtimes whose own harness isolates each executor (**Claude Code**, **Cursor**) run parallel worktrees natively; runtimes exposing a headless exec with an explicit working directory (**Codex**, **OpenCode**, **Kimi**, **Kimi Code**) get worktrees GSD itself creates and merges — where a dispatch site can only drive the harness model, those hosts degrade to sequential with a warning rather than aborting. Every other runtime declares no isolation primitive, and forcing `use_worktrees: true` there still fails closed before any executor dispatch. `/gsd-health` reports such a value as warning `W025` (#2486). **Default on a non-Claude install:** if a worktree-capable non-Claude host is not isolating as described above, check whether the install stamped this key's default to `false` and set an explicit `use_worktrees: true`. See [Executor isolation per runtime](#executor-isolation-per-runtime). | | `workflow.worktree_skip_hooks` | boolean | `false` | When `true`, executor agents in worktree mode pass `--no-verify` (skipping pre-commit hooks) and post-wave hook validation runs against the merged result instead. Opt-in escape hatch for projects whose hooks cannot run in agent worktrees. Default `false` runs hooks on every commit (#2924). | | `workflow.code_review` | boolean | `true` | Enable `/gsd-code-review` and `/gsd-code-review --fix` commands. When `false`, the commands exit with a configuration gate message. Added in v1.34 | | `workflow.code_review_depth` | string | `standard` | Default review depth for `/gsd-code-review`: `quick` (pattern-matching only), `standard` (per-file analysis), or `deep` (cross-file with import graphs). Can be overridden per-run with `--depth=`. Added in v1.34 | @@ -388,6 +388,26 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin |---------|------|---------|-------------| | `worktree.baseRef` | string | (unset) | Controls which ref the worktree-based parallel executor uses as the base when creating new phase/wave worktrees. When unset, the executor bases new worktrees on the repository default branch (`origin/HEAD`); if the current branch has diverged, execute-phase auto-degrades to sequential execution rather than halting (as of v1.4.0). Set to `"head"` to base new worktrees on the local `HEAD` instead — the appropriate choice when working on a branch that has diverged from the default branch, as it prevents the exit-42 base-mismatch halt and allows wave-based parallel execution to proceed normally. See [Fix the worktree base-mismatch (exit 42) error](how-to/fix-worktree-base-mismatch.md). | +### Executor isolation per runtime + +When `/gsd-execute-phase` runs a wave containing several independent plans, it can execute them concurrently — but only if the runtime can keep each executor isolated. Two executors sharing one checkout race on files, git state, hooks, and `.planning/`. Which runtimes can do this is a **declared capability** (`dispatch.isolation`), not a hardcoded list, so the scheduler behaves the same way for every host that declares the same value. + +| Isolation | Runtimes | What happens | +|---|---|---| +| `harness-worktree` | `claude`, `cursor` | The runtime's own harness creates and binds a git worktree per executor. GSD passes the host's isolation flag and runs no git itself. | +| `orchestrator-worktree` | `codex`, `opencode`, `kimi`, `kimi-code` | The runtime has no harness-native isolation, but exposes a headless exec that accepts a working directory. **GSD** creates the worktree, spawns each executor into it, then validates and merges the result. All git operations are performed by GSD, never by the sandboxed executor. | +| `none` | every other runtime | No isolation primitive — plans in a wave run sequentially. Setting `workflow.use_worktrees: true` here fails closed before any executor is dispatched. | + +You do not configure this directly: set `workflow.use_worktrees` and GSD negotiates the rest. `use_worktrees: false` forces sequential execution on **every** runtime, including the ones that support isolation. An unknown or undeclared isolation value always degrades to sequential — GSD never guesses its way into an unisolated parallel run. + +To see what your current runtime negotiated: + +```bash +gsd-tools query inspect-dispatch-isolation --json +``` + +(`inspect-dispatch-isolation` is the read-only form. The `dispatch-isolation` query is the executor-dispatch resolver: it records its decision to the isolation sentinel as a deliberate side effect, so it is not an inspection command.) + ## Code Quality Settings The `code_quality.*` namespace gates optional structural-analysis tooling that augments `/gsd-code-review`. Settings are additive: each tool is independently opt-in and off by default. diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index fb9a7c321..ae2914932 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1580,7 +1580,8 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load function routeDispatchShouldFlatten({ args, cwd, raw, error }) { // #1708 / #853: typed query replacing the `RUNTIME === 'codex'` prose rule. // - // Resolves the current runtime (GSD_RUNTIME > config.runtime > 'claude'), + // Resolves the current runtime (GSD_RUNTIME > config.runtime > per-install + // .gsd-runtime marker > 'claude'), // looks up registry.runtimes[id].runtime.hostIntegration.dispatch, and // calls shouldFlattenDispatch(dispatch) from host-integration.cjs. // @@ -1634,7 +1635,8 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // #853) for exactly this reason — it replaced a `RUNTIME === 'codex'` // prose rule. // - // Resolves the current runtime (GSD_RUNTIME > config.runtime > 'claude'), + // Resolves the current runtime (GSD_RUNTIME > config.runtime > per-install + // .gsd-runtime marker > 'claude'), // reads registry.runtimes[id].runtime.hostIntegration.dispatch.isolation, // and validates it against the closed vocabulary. // @@ -1669,12 +1671,83 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // `per-plan-worktree-gate.md`) override the naturally-resolved mode // while still going through this single write path. Best-effort: a // sentinel write failure here must never fail the wave. - const VALID_ISOLATION = new Set(['harness-worktree', 'orchestrator-worktree', 'none']); + const decision = resolveDispatchIsolationDecision({ args, cwd }); + const runtimeId = decision.runtimeId; + let { isolation, exec, harnessFlag } = decision; + + // `--force-isolation ` overrides the naturally-resolved mode with + // context this resolver has no way to see on its own (e.g. the #2474 + // per-plan submodule intersection). Invalid/unrecognized values are + // ignored rather than erroring — this is a best-effort recording call, + // not a hard usage gate. Forcing to 'none' clears harnessFlag/exec since + // neither applies to sequential dispatch. + const forceIdx = args.indexOf('--force-isolation'); + const forcedIsolation = forceIdx !== -1 ? args[forceIdx + 1] : undefined; + if (forcedIsolation && DISPATCH_ISOLATION_VOCABULARY.has(forcedIsolation)) { + isolation = forcedIsolation; + if (isolation === 'none') { + harnessFlag = null; + exec = null; + } + } + + const phaseIdx = args.indexOf('--phase'); + const phaseArg = phaseIdx !== -1 && args[phaseIdx + 1] && !args[phaseIdx + 1].startsWith('--') + ? args[phaseIdx + 1] + : null; + const planIdx = args.indexOf('--plan'); + const planArg = planIdx !== -1 && args[planIdx + 1] && !args[planIdx + 1].startsWith('--') + ? args[planIdx + 1] + : null; + + // Side-effect write (#3045 CORE REDESIGN) — see the doc comment above. + // Never allowed to affect this query's own stdout contract or throw. + try { + writeDispatchIsolationSentinel(cwd, { isolation, harnessFlag, phase: phaseArg, plan: planArg }); + } catch { + // writeDispatchIsolationSentinel already swallows its own errors into + // a { recorded: false } result; this catch is defense in depth only. + } + + if (args.indexOf('--json') !== -1) { + output({ runtime: runtimeId, isolation, exec, harnessFlag }, raw); + } else { + process.stdout.write(isolation); + } + } + + const DISPATCH_ISOLATION_VOCABULARY = new Set(['harness-worktree', 'orchestrator-worktree', 'none']); + + /** + * Shared, side-effect-free resolution of the negotiated dispatch isolation: + * runtime (GSD_RUNTIME > config.runtime > per-install .gsd-runtime marker > + * 'claude') → declared + * `dispatch.isolation` → harness-flag / orchestrator-exec degrade rules. + * Extracted (#2486) so `routeDispatchIsolation` (the #3045 recording + * dispatch path) and `routeInspectDispatchIsolation` (the read-only + * inspection path) share exactly one negotiation implementation and cannot + * drift apart. Resolution only — the caller decides whether the decision is + * recorded to the sentinel. + */ + function resolveDispatchIsolationDecision({ args, cwd }) { let isolation = 'none'; let runtimeId = null; let exec = null; let harnessFlag = null; try { + // Deliberately `resolveRuntime`, NOT `resolveActiveRuntime`/`loadConfig`: + // loadConfig normalizes and rewrites legacy keys back to disk, and this + // resolver backs the sentinel-free `inspect-dispatch-isolation` verb, + // which must never write. resolveRuntime reads config.json directly. + // + // KNOWN LIMITATION, tracked separately: resolveRuntime stops at + // GSD_RUNTIME > config.runtime > 'claude' and does not consult the + // per-install `.gsd-runtime` marker, so on a non-Claude install whose + // project config carries no `runtime` key this resolves 'claude'. That is + // open-gsd/gsd-core#2395 — a pre-existing defect in the canonical + // resolver, not introduced here, and deliberately NOT fixed in this PR + // (its blast radius reaches every consumer of that resolver, so it is + // being handled on its own). const { resolveRuntime } = require('./lib/runtime-slash.cjs'); runtimeId = resolveRuntime(cwd); @@ -1683,7 +1756,7 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load ? registry.runtimes[runtimeId] : null; const declared = runtimeEntry?.runtime?.hostIntegration?.dispatch?.isolation ?? null; - if (typeof declared === 'string' && VALID_ISOLATION.has(declared)) { + if (typeof declared === 'string' && DISPATCH_ISOLATION_VOCABULARY.has(declared)) { isolation = declared; } @@ -1726,41 +1799,49 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load exec = null; harnessFlag = null; } + return { runtimeId, isolation, exec, harnessFlag }; + } - // `--force-isolation ` overrides the naturally-resolved mode with - // context this resolver has no way to see on its own (e.g. the #2474 - // per-plan submodule intersection). Invalid/unrecognized values are - // ignored rather than erroring — this is a best-effort recording call, - // not a hard usage gate. Forcing to 'none' clears harnessFlag/exec since - // neither applies to sequential dispatch. - const forceIdx = args.indexOf('--force-isolation'); - const forcedIsolation = forceIdx !== -1 ? args[forceIdx + 1] : undefined; - if (forcedIsolation && VALID_ISOLATION.has(forcedIsolation)) { - isolation = forcedIsolation; - if (isolation === 'none') { - harnessFlag = null; - exec = null; - } + function routeInspectDispatchIsolation({ args, cwd, raw }) { + // #2486: sentinel-free sibling of `dispatch-isolation` for INSPECTION + // surfaces — /gsd:health's W025 check and /gsd:settings' Worktrees + // branching. The dispatch verb above intentionally records its resolved + // decision to the isolation sentinel as an unconditional side effect + // (#3045 CORE REDESIGN): correct for executor dispatch, where the record + // must be structurally unskippable — but wrong for a read-only + // diagnostic. A health check that records a phase:null/plan:null + // sentinel can hard-block every executor dispatch for the sentinel's + // lifetime, across sessions sharing the main checkout. Inspection + // surfaces call this verb instead. Two claims, both narrower than + // "side-effect-free", and both exactly true (#2486 review, Majors 2 & 4): + // + // 1. SENTINEL-FREE, not write-free. This route writes nothing itself, and + // in particular never writes .gsd/dispatch-isolation-sentinel.json — + // the only write that can hard-block a later executor dispatch. It is + // NOT a claim of total filesystem purity: like every gsd-tools + // invocation, it runs the shared bootstrap and active-workstream + // resolution first, and getActiveWorkstream self-heals (unlinks) a + // stale or invalid pointer. That is pre-existing, verb-independent, + // and harmless to dispatch. + // + // 2. SHARED NEGOTIATION, for the arguments this verb accepts. Both verbs + // call resolveDispatchIsolationDecision, so the natural resolution + // cannot drift. It is NOT a claim of byte-identical output for every + // argv: routeDispatchIsolation applies --force-isolation AFTER the + // shared helper returns, so the same argv could otherwise yield + // 'none' there and the declared capability here. Rather than let a + // caller receive a silently different answer, this verb REJECTS the + // recording-only knobs outright — they exist to be recorded, and a + // read has nothing to record. + const RECORDING_ONLY_ARGS = ['--force-isolation', '--phase', '--plan']; + const rejected = RECORDING_ONLY_ARGS.filter((flag) => args.indexOf(flag) !== -1); + if (rejected.length > 0) { + error( + `inspect-dispatch-isolation: ${rejected.join(', ')} ${rejected.length === 1 ? 'is a' : 'are'} recording-only argument${rejected.length === 1 ? '' : 's'} and cannot be used on the inspection verb — it resolves the runtime's DECLARED capability and records nothing. Use 'query dispatch-isolation' if you need the override applied and the decision recorded.`, + ERROR_REASON.USAGE, + ); } - - const phaseIdx = args.indexOf('--phase'); - const phaseArg = phaseIdx !== -1 && args[phaseIdx + 1] && !args[phaseIdx + 1].startsWith('--') - ? args[phaseIdx + 1] - : null; - const planIdx = args.indexOf('--plan'); - const planArg = planIdx !== -1 && args[planIdx + 1] && !args[planIdx + 1].startsWith('--') - ? args[planIdx + 1] - : null; - - // Side-effect write (#3045 CORE REDESIGN) — see the doc comment above. - // Never allowed to affect this query's own stdout contract or throw. - try { - writeDispatchIsolationSentinel(cwd, { isolation, harnessFlag, phase: phaseArg, plan: planArg }); - } catch { - // writeDispatchIsolationSentinel already swallows its own errors into - // a { recorded: false } result; this catch is defense in depth only. - } - + const { runtimeId, isolation, exec, harnessFlag } = resolveDispatchIsolationDecision({ args, cwd }); if (args.indexOf('--json') !== -1) { output({ runtime: runtimeId, isolation, exec, harnessFlag }, raw); } else { @@ -3500,6 +3581,7 @@ const HOST_COMMAND_ROUTERS = { 'normalize-test-command': routeNormalizeTestCommand, 'dispatch-should-flatten': routeDispatchShouldFlatten, 'dispatch-isolation': routeDispatchIsolation, + 'inspect-dispatch-isolation': routeInspectDispatchIsolation, 'record-dispatch-isolation': routeRecordDispatchIsolation, 'resolve-dispatch-type': routeResolveDispatchType, 'agent-skills': routeAgentSkills, @@ -3754,7 +3836,7 @@ const TOP_LEVEL_USAGE = 'Usage: gsd-tools [args] [--raw] [--pick /dev/null) +_ISOLATION_RC=$? +if [ $_ISOLATION_RC -ne 0 ] || [ -z "$_INSPECTED_RAW" ]; then + INSPECTED_ISOLATION=none + INSPECTED_RESOLVED=false # no verdict learned — not the same as "declares none" +else + INSPECTED_ISOLATION="$_INSPECTED_RAW" + INSPECTED_RESOLVED=true +fi +case "$INSPECTED_ISOLATION" in + harness-worktree|orchestrator-worktree|none) ;; + *) INSPECTED_ISOLATION=none; INSPECTED_RESOLVED=false ;; # out of vocabulary is not a verdict either +esac + +USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null) +if [ "$INSPECTED_ISOLATION" = "none" ] && [ -n "$USE_WORKTREES" ] && [ "$USE_WORKTREES" != "false" ]; then + if [ "$INSPECTED_RESOLVED" = "true" ]; then + echo "W025: the project config sets workflow.use_worktrees to a non-false value, but this runtime has no usable executor-isolation primitive — dispatch.isolation resolves to none, declared as none, or fail-closed because the capability could not be determined — so /gsd:execute-phase and /gsd:quick will fail closed. Fix: run /gsd:settings and answer No to Worktrees, or set workflow.use_worktrees: false in the project config (the active workstream's config.json when one is active). Set it explicitly rather than deleting the key — an absent key resolves to false only on an emit whose default was stamped to false, and to true otherwise." + else + echo "W025: the project config sets workflow.use_worktrees to a non-false value, and GSD could not resolve this runtime's executor-isolation capability ('gsd_run query inspect-dispatch-isolation' failed or returned nothing) — so it cannot tell whether /gsd:execute-phase and /gsd:quick will fail closed on that value. This is a report of an unverifiable config, NOT a finding that the runtime declares no primitive. Fix: re-run once the gsd-tools shim resolves; if the warning persists, run /gsd:settings and answer No to Worktrees." + fi +fi +``` + +If the check prints, append it to the Warnings section of the report as `[W025]` with the printed fix, include it in the displayed warning count, and report `Status: DEGRADED` if `validate.health` returned `healthy` (a config the execution workflows fail closed on is not a healthy planning state). It is not auto-repairable: an explicit `true` may be intentional for a worktree-capable install sharing the same `.planning/config.json`, so the remedy is the user's call (#2486). @@ -187,8 +236,11 @@ Report final status. | W018 | warning | MILESTONES.md missing entry for archived milestone snapshot | Yes (`--backfill`) | | W019 | warning | Unrecognized .planning/ root file — not a canonical GSD artifact | No | | W024 | warning | STATE.md was written many commits ago — treat its contents as approximate | No | +| W025 | warning | config.json: workflow.use_worktrees enabled on a runtime whose dispatch.isolation is none (#2486) | No | | I001 | info | Plan without SUMMARY (may be in progress) | No | +Note: the `W0NN` warning-code namespace is owned by `src/verify.cts` (`validate.health`), which also emits codes this table does not list (`W010`–`W017` and `W020`–`W023` as of #2486). `W001`–`W024` are all allocated, so this workflow's isolation warning is `W025`. Before assigning a new code here, grep `src/verify.cts` for the next free number — the table alone under-represents the live namespace, and two PRs in flight can otherwise claim the same code (which is exactly what happened between #2486 and #2573). + diff --git a/gsd-core/workflows/settings.md b/gsd-core/workflows/settings.md index 908024492..6aa4d00b1 100644 --- a/gsd-core/workflows/settings.md +++ b/gsd-core/workflows/settings.md @@ -59,7 +59,7 @@ Parse current values (default to `true` if not present): - `graphify.auto_update` — opt-in: auto-rebuild graph after main HEAD advances (#3347) (default: `false`) - `model_profile` — which model each agent uses (default: `balanced`) - `git.branching_strategy` — branching approach (default: `"none"`) -- `workflow.use_worktrees` — whether parallel executor agents run in worktree isolation (default: `true`) +- `workflow.use_worktrees` — whether parallel executor agents run in worktree isolation (honored when the runtime declares a `dispatch.isolation` primitive — `harness-worktree` or `orchestrator-worktree`; runtimes declaring `none` default it to `false` and fail closed on an explicit `true` — #1521, #2486, #2584) - `model_policy.provider` — provider slug for model policy (default: `null`; known values: anthropic, openai, google, qwen; set via /gsd:config --advanced) - `model_policy.budget` — budget level for model policy (default: `null`; known values: high, medium, low; set via /gsd:config --advanced) - `model_policy.high` — model ID for high-cost tier (default: `null`; set via /gsd:config --advanced) @@ -87,6 +87,30 @@ configure `model_overrides` manually in .planning/config.json to target specific models per agent. ``` +**Isolation resolution for the Worktrees question (#2486):** resolve the runtime's declared executor-isolation primitive and the current worktrees value before presenting the questions. Use `inspect-dispatch-isolation`, the **sentinel-free** inspection verb — never `dispatch-isolation`, whose #3045 contract records the resolved decision to the executor-dispatch sentinel as an unconditional side effect; a settings menu must not be able to stamp a sentinel the isolation guards then enforce against real dispatches. Sentinel-free is the exact claim: the shared CLI bootstrap still runs, but nothing it does can block a dispatch. The worktrees read deliberately carries no `--default`/fallback: an absent key must stay distinguishable from an explicit `false` (empty output = key absent), which the pre-selection rule below depends on. + +A failed query is not a capability verdict, so track the two apart: settings still fails closed, but +says so instead of claiming the runtime declares no primitive. The verb fail-closes an unknown runtime — or an +internal resolution error — to `none` and exits 0, so `INSPECTED_RESOLVED=false` means only "the query did not answer" +(#2486 review): + +```bash +_INSPECTED_RAW=$(gsd_run query inspect-dispatch-isolation --raw 2>/dev/null) +_ISOLATION_RC=$? +if [ $_ISOLATION_RC -ne 0 ] || [ -z "$_INSPECTED_RAW" ]; then + INSPECTED_ISOLATION=none + INSPECTED_RESOLVED=false # no verdict learned — not the same as "declares none" +else + INSPECTED_ISOLATION="$_INSPECTED_RAW" + INSPECTED_RESOLVED=true +fi +case "$INSPECTED_ISOLATION" in + harness-worktree|orchestrator-worktree|none) ;; + *) INSPECTED_ISOLATION=none; INSPECTED_RESOLVED=false ;; # out of vocabulary is not a verdict either +esac +USE_WORKTREES_CURRENT=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null) +``` + Use AskUserQuestion with current values pre-selected. Questions are grouped into six visual sections; the first question in each section carries the section-denoting `header` field (AskUserQuestion renders abbreviated section tags for grouping, max 12 chars). Section layout: @@ -113,6 +137,46 @@ Context Warnings, Research Qs **Conditional visibility — graphify.auto_update:** This question is shown only when the user's chosen `graphify.enabled` value is on. If `graphify.enabled` is off, omit the `graphify.auto_update` question and preserve the existing `graphify.auto_update` value in config (do not overwrite). Implementation: ask Graphify first; only ask Graph auto-update when Graphify is enabled. +**Conditional options — Worktrees (#2486):** whether a runtime can honor `workflow.use_worktrees: true` depends on its **declared `dispatch.isolation` capability, not its name** (#2584): `harness-worktree` (the host isolates each executor) and `orchestrator-worktree` (GSD drives the worktrees) both run parallel; `none` fails closed on an explicit `true`. Branch on the same negotiation the execution gate resolves, via the sentinel-free `inspect-dispatch-isolation` verb, which fail-closes unknown/undeclared/`undocumented` to `none`. The rule is one-directional: **never persist a value the isolation gate would fail closed on.** **Do not add a runtime-name test here.** Branch on `$INSPECTED_ISOLATION`: + +- **If `INSPECTED_ISOLATION` ≠ `none`** (`harness-worktree` or `orchestrator-worktree`): present the Worktrees question exactly as written below — unchanged behavior. The `none` branch is the entirety of the #2486 change. +- **If `INSPECTED_ISOLATION` = `none`:** replace the question's options with the two below — do NOT offer an enabling option, and NEVER write `workflow.use_worktrees: true` from this workflow when the runtime declares no isolation primitive, regardless of the user's answer: + +``` +{ + question: "Use git worktrees for parallel agent isolation?", + header: "Worktrees", + multiSelect: false, + options: [ + { label: "No (Recommended)", description: "Write use_worktrees: false. {FINDING}, so parallel plans run sequentially and {CONSEQUENCE}." }, + { label: "Leave unchanged", description: "Do not write the key. {ABSENCE}; an existing explicit value is kept intact (e.g. for a worktree-capable install sharing this config)." } + ] +} +``` + +**The three placeholders** carry the only difference between a capability GSD resolved and one it could not, so no text in this branch ever asserts a verdict that was not reached. Substitute them everywhere they appear — the two `description`s above, the pre-selection rationale, and the notice — from whichever column applies: + +| | `INSPECTED_RESOLVED=true` | `INSPECTED_RESOLVED=false` | +|---|---|---| +| `{FINDING}` | this runtime has no usable executor-isolation primitive | GSD could not resolve this runtime's executor-isolation capability | +| `{CONSEQUENCE}` | execution fails closed on an explicit true | GSD cannot tell whether execution will accept an explicit true | +| `{ABSENCE}` | absent, it resolves to false wherever this emit stamped that default | absent, it is the safe state until the capability resolves | + +Failing closed is right in both columns — never write `workflow.use_worktrees: true` from this branch either way. Only the explanation changes. + + Persistence: "No (Recommended)" → write `workflow.use_worktrees: false`; "Leave unchanged" → do not write `workflow.use_worktrees` at all (preserve the existing value or absence). + + Pre-selection: the generic "current values pre-selected" rule does not apply when `INSPECTED_ISOLATION` is `none` — an explicit `true` has no matching option by design. Pre-select "No (Recommended)" in every case, including an absent key (`$USE_WORKTREES_CURRENT` empty, the no-`--default` read's absent signal): absence resolves to `false` only where this emit stamped that default and to `true` otherwise, so leaving it absent can preserve the very state this branch prevents, while an explicit `false` is correct under either emit (#2486 review). The same applies when it is an explicit non-false value — the broken-inheritance case the notice below covers. "Leave unchanged" remains available for the deliberate shared-config case, but is never the default here. The default must be the repair the "(Recommended)" label points to, so accepting it never keeps a value this branch could not justify. In TEXT_MODE, mark that option as the default in the numbered list. + + Additionally, if `$USE_WORKTREES_CURRENT` is non-empty and not `false` (the config carries an explicit `true` — e.g. inherited from a worktree-capable install sharing the repo; empty means the key is absent, which needs no notice), prepend this notice before the question: + +``` +Note: the project config currently sets workflow.use_worktrees: true. +{FINDING}, so {CONSEQUENCE}. Choose "No" to repair it for this runtime, or +"Leave unchanged" to keep it for a worktree-capable install that shares this +config. +``` + ``` // Model profile is selected via a two-question split because AskUserQuestion enforces a // hard 4-option cap and there are 5 valid profiles (quality, balanced, budget, adaptive, @@ -411,7 +475,7 @@ Merge new settings into existing config.json: "research_before_questions": true/false, "discuss_mode": "discuss" | "assumptions", "skip_discuss": true/false, - "use_worktrees": true/false + "use_worktrees": true/false // never written as true when the runtime's dispatch.isolation is none; omitted entirely when the user chose "Leave unchanged" (#2486) }, "plan_review": { "source_grounding": true/false diff --git a/tests/emitted-drift-acks/2486-settings-worktrees-runtime.json b/tests/emitted-drift-acks/2486-settings-worktrees-runtime.json new file mode 100644 index 000000000..1a402a524 --- /dev/null +++ b/tests/emitted-drift-acks/2486-settings-worktrees-runtime.json @@ -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." + } +} diff --git a/tests/emitted-drift-acks/2573-state-head-freshness.json b/tests/emitted-drift-acks/2573-state-head-freshness.json index ecb372b44..f87f090a9 100644 --- a/tests/emitted-drift-acks/2573-state-head-freshness.json +++ b/tests/emitted-drift-acks/2573-state-head-freshness.json @@ -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 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 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." } } diff --git a/tests/gsd-agent-isolation-guard.test.cjs b/tests/gsd-agent-isolation-guard.test.cjs index 5eceb335b..01af113a3 100644 --- a/tests/gsd-agent-isolation-guard.test.cjs +++ b/tests/gsd-agent-isolation-guard.test.cjs @@ -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'); + }); +}); diff --git a/tests/host-integration.test.cjs b/tests/host-integration.test.cjs index 31778e9a4..c302eb09a 100644 --- a/tests/host-integration.test.cjs +++ b/tests/host-integration.test.cjs @@ -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; diff --git a/tests/runtime-converters.test.cjs b/tests/runtime-converters.test.cjs index 79d2c7986..a7d7fae69 100644 --- a/tests/runtime-converters.test.cjs +++ b/tests/runtime-converters.test.cjs @@ -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) // ────────────────────────────────────────────────────────────────────────