diff --git a/.changeset/graceful-seals-dance.md b/.changeset/graceful-seals-dance.md new file mode 100644 index 000000000..328bf50ce --- /dev/null +++ b/.changeset/graceful-seals-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2728 +--- +**`/gsd-quick` and the UAT-diagnosis step no longer abort with a FATAL on a non-Claude runtime that can actually isolate** — both dispatch sites resolved worktree isolation from a hardcoded `RUNTIME != "claude"` test, so every non-Claude host was refused regardless of what it could actually do. They now read the negotiated `dispatch.isolation` capability (#2584), and installs for runtimes that declare worktree support no longer stamp `workflow.use_worktrees` to `false`, which had pre-empted that negotiation. A runtime is judged by what it declares rather than by its name. A host that declares no isolation primitive at all still fails closed when worktrees are explicitly enabled — that FATAL is the fail-closed contract, not the bug — and a host whose isolation model the single-agent sites cannot express degrades to sequential, one agent at a time, on the main working tree. diff --git a/CONTEXT.md b/CONTEXT.md index d3ac030df..1458300d3 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -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; declared and negotiated but not yet consumed by any scheduler — Phase 1 of #2584). `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`; declared and testable but UNCONSUMED — no scheduler spawns anything with it yet, Phase 3 wires it). 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). `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 c987c286f..95774c72f 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. **Non-Claude note:** git worktree isolation uses Claude Code's `isolation="worktree"` agent primitive, which no other runtime honors. On any non-Claude install (Codex, Cursor, Antigravity, Qwen, etc.) a runtime-neutral `.planning/config.json` resolves the runtime to that install's own id and defaults this key to `false`; forcing `use_worktrees: true` on a non-Claude install fails closed before any executor dispatch (#1515, #1521). | +| `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.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 | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 99d74c6b0..4c61c8c2c 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -225,6 +225,7 @@ "debugger-semantic-recall.md", "debugger-techniques.md", "decimal-phase-calculation.md", + "dispatch-isolation-gate.md", "doc-conflict-engine.md", "domain-probes.md", "edge-probe.md", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 8c725f9ff..4cd9beba3 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -354,6 +354,7 @@ Full roster at `gsd-core/references/*.md`. References are shared knowledge docum | `universal-anti-patterns.md` | Universal anti-patterns to detect and avoid. | | `worktree-branch-check.md` | Canonical spawn-time worktree HEAD/base guard (worktree_branch_check): verify-only and fail-closed — per-agent-branch assertion, protected-ref refusal (#2924), and an exact-base assertion that halts with `exit 42` on mismatch so the orchestrator (worktree lifecycle owner) performs recovery (#48). Embedded into worktree sub-agent prompts at dispatch. | | `runtime-aware-dispatch.md` | Runtime-aware subagent dispatch protocol (#2508 Phase 4 Option A): before any `Agent(subagent_type="gsd-*")` call, resolve the type via `gsd_run query resolve-dispatch-type --requested --raw`. On named-dispatch runtimes (Claude/OpenCode/…) the name is returned unchanged; on built-in-only runtimes (kimi-code) it maps to `coder`/`explore`/`plan` by role-suffix. The persona rides `${AGENT_SKILLS_}` (Phase 3) regardless. Documents why a PreToolUse-remap hook (the epic's original Option B) is infeasible — Kimi Code's hook API supports only allow/deny, not tool_input rewriting. | +| `dispatch-isolation-gate.md` | Canonical gate deciding whether a dispatch site may run an agent isolated (#2584/#2652): resolves `ISOLATION` from the negotiated `dispatch.isolation` capability — never from a runtime id — fails closed to `none`, resolves the host's declared `harnessFlag` instead of hardcoding Claude Code's `isolation="worktree"` literal, and degrades single-agent sites to sequential on `orchestrator-worktree` hosts. Read by `quick.md`, `diagnose-issues.md`, and `execute-plan.md`. | | `worktree-path-safety.md` | Worktree guard suite: HEAD assertion, cwd-drift sentinel (step 0a, #3097), and absolute-path guard (step 0b, #3099) — loaded into executor spawn prompts via ``. | | `untrusted-input-boundary.md` | Shared prompt-injection boundary (#1577) `@`-included by the 10 research/doc-ingest agents (`gsd-project-researcher`, `gsd-phase-researcher`, `gsd-ui-researcher`, `gsd-assumptions-analyzer`, `gsd-advisor-researcher`, `gsd-doc-classifier`, `gsd-doc-synthesizer`, `gsd-research-synthesizer`, `gsd-ai-researcher`, `gsd-domain-researcher`): treat fetched/read text as data-not-instructions, self-scan before use (PromptArmor 2507.15219), task-anchor (2504.20472), and fence quoted text with a fresh random delimiter per wrap (PPA 2506.05739). Prompt-level defense-in-depth (2503.00061); the hook scanner is a separate pattern pre-filter. | | `artifact-types.md` | Planning artifact type definitions. | diff --git a/gsd-core/references/dispatch-isolation-gate.md b/gsd-core/references/dispatch-isolation-gate.md new file mode 100644 index 000000000..6b69ffc21 --- /dev/null +++ b/gsd-core/references/dispatch-isolation-gate.md @@ -0,0 +1,138 @@ +# Dispatch isolation gate (ADR-1239 / #2584) + +The single source of truth for **how a dispatch site decides whether it may run an agent +isolated**. Every workflow that spawns an executor-like subagent reads this file and follows +it, rather than restating the rule inline (#2652). + +`ISOLATION` — not `RUNTIME` — is the decision variable. **Never branch a dispatch site on a +runtime id.** Isolation is a negotiated capability declared per host; a runtime-name test +silently diverges the moment a new host declares support, which is exactly how #2652 happened: +`quick.md` and `diagnose-issues.md` kept a `RUNTIME != "claude"` gate after #2584 migrated the +phase scheduler, so Codex — which declares `orchestrator-worktree` — was refused isolation it +had in fact negotiated. + +## Resolve ISOLATION + +Run this in the dispatch site's config-gate step, right after `RUNTIME` / `USE_WORKTREES` are +read. It requires `gsd_run` to be defined (the standard shim preamble). + +```bash +# Isolation is a NEGOTIATED CAPABILITY, not a runtime id (#2584). Fail-closed to none. +# #3045: this call PERSISTS the resolution to the run-scoped sentinel the isolation +# guard hooks read, as an unconditional side effect of resolving it. +# Keep the resolver's own failure DISTINGUISHABLE from a genuine `none`. Both +# fail closed — that policy is right — but only one of them may claim the host +# declared no primitive. A shim-resolution failure, a non-zero exit or empty +# stdout is NOT a capability verdict, and reporting it as one tells a Claude +# Code user their runtime "declares no executor-isolation primitive", which is +# false (#2652 review). +_ISOLATION_RAW=$(gsd_run query dispatch-isolation --raw 2>/dev/null) +_ISOLATION_RC=$? +if [ $_ISOLATION_RC -ne 0 ] || [ -z "$_ISOLATION_RAW" ]; then + ISOLATION=none + ISOLATION_RESOLVED=false # fail closed, but we did NOT learn a verdict +else + ISOLATION="$_ISOLATION_RAW" + ISOLATION_RESOLVED=true +fi +case "$ISOLATION" in + harness-worktree|orchestrator-worktree|none) ;; + *) ISOLATION=none; ISOLATION_RESOLVED=false ;; # out of vocabulary is not a verdict either +esac + +# Project-level opt-out wins on every host; a host with no primitive fails closed. +[ "$USE_WORKTREES" = "false" ] && ISOLATION=none +if [ "$ISOLATION" = "none" ] && [ "$USE_WORKTREES" != "false" ]; then + if [ "$ISOLATION_RESOLVED" = "true" ]; then + echo "FATAL: runtime '$RUNTIME' declares no executor-isolation primitive (dispatch.isolation=none) — agents would run unisolated against the main checkout. Set workflow.use_worktrees=false." >&2 + else + echo "FATAL: could not resolve this runtime's executor-isolation capability — 'gsd_run query dispatch-isolation' failed or returned nothing, so GSD cannot tell whether isolation is available. Refusing to dispatch rather than guess (a guard that cannot verify must not answer 'safe'). Re-run once the gsd-tools shim resolves, or set workflow.use_worktrees=false to run sequentially on purpose." >&2 + fi + exit 1 +fi + +# Re-record: the opt-out above is decided in shell, where the resolver cannot see +# it, so the sentinel still asserts the naturally-resolved mode. See "Re-record +# after every degrade" below — this is the first of the mandatory calls. +gsd_run query dispatch-isolation --raw --force-isolation "$ISOLATION" >/dev/null 2>&1 || true +``` + +| `ISOLATION` | Meaning | What a dispatch site does | +|---|---|---| +| `harness-worktree` | The host's own harness creates and binds a worktree per agent. | Pass the host's declared `harnessFlag` on the dispatch. GSD runs no git. | +| `orchestrator-worktree` | No harness primitive, but a headless exec accepting a working directory. | GSD creates the worktree and process-spawns the agent into it. GSD performs every git operation. | +| `none` | No isolation primitive. | Run inline, sequentially. | + +Fail-closed is the invariant: an undeclared, unknown, or unresolvable declaration degrades to +`none`, never to an unsafe parallel path. + +## Resolve the harness flag (harness-worktree only) + +The flag is descriptor data — never hardcode `isolation="worktree"`, which is Claude Code's +literal and wrong on any other harness-worktree host (Cursor declares the same capability). + +```bash +HARNESS_FLAG="" +if [ "$ISOLATION" = "harness-worktree" ]; then + HARNESS_FLAG=$(gsd_run query dispatch-isolation --json 2>/dev/null \ + | node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{try{const j=JSON.parse(s);process.stdout.write(j&&j.harnessFlag?j.harnessFlag:"")}catch{process.stdout.write("")}})') + [ -n "$HARNESS_FLAG" ] || { echo "FATAL: runtime declares dispatch.isolation=harness-worktree but no harnessIsolationFlag — refusing to dispatch an agent that would believe it is isolated." >&2; exit 1; } +fi +``` + +Substitute `$HARNESS_FLAG` for the `{harnessFlag}` placeholder in the dispatch call. On Claude +Code it resolves to literally `isolation="worktree"`. + +## Single-agent dispatch sites + +`quick.md` and `diagnose-issues.md` spawn through the host's own subagent tool, which can only +express the `harness-worktree` model. On an `orchestrator-worktree` host they must degrade to +sequential rather than pass a harness flag the host will ignore — dispatching unisolated while +reporting isolation is the one outcome worse than running sequentially: + +```bash +if [ "$ISOLATION" = "orchestrator-worktree" ]; then + echo "⚠ Runtime '$RUNTIME' declares dispatch.isolation=orchestrator-worktree, which requires GSD-driven process spawning. This dispatch site uses the host subagent tool, so it is running sequentially on the main working tree instead. Parallel wave execution (/gsd:execute-phase) is unaffected." >&2 + ISOLATION=none + USE_WORKTREES=false + gsd_run query dispatch-isolation --raw --force-isolation none >/dev/null 2>&1 || true +fi +``` + +## Re-record after every degrade + +**Any block that changes `$ISOLATION` after the resolve above MUST re-record it before +dispatch.** This is not optional bookkeeping — it is what keeps the dispatch legal. + +`query dispatch-isolation` writes the mode it resolved into a run-scoped sentinel +(`.gsd/dispatch-isolation-sentinel.json`) as an unconditional side effect, and the shipped +`PreToolUse` isolation guards read that sentinel at the instant of the dispatch call +(`hooks/gsd-agent-isolation-guard.js`, `hooks/gsd-cursor-subagent-start.js`, shared reader +`hooks/lib/isolation-sentinel.js`, #3045). Every degrade in this file is decided **in shell**, +where the resolver cannot see it. Degrade without re-recording and the sentinel still asserts +`harness-worktree` while the dispatch correctly omits the harness flag — the guard reads that +as a dropped isolation flag and **denies the dispatch with exit 2**. The task does not run +unisolated; it does not run at all. + +```bash +gsd_run query dispatch-isolation --raw --force-isolation "$ISOLATION" >/dev/null 2>&1 || true +``` + +`--force-isolation` pushes the final, shell-computed value through the same single write path +(`none` also clears the stored `harnessFlag`, since none applies to sequential dispatch). It is +idempotent and last-write-wins, so a site that degrades more than once simply calls it again — +record immediately before dispatch so the sentinel is always fresh. Best-effort by design: a +write failure must never fail the dispatch, since the guards' sentinel-absent fallback is safe, +just less precise. + +Wave sites re-record per plan rather than per phase — see +`gsd-core/workflows/execute-phase/steps/per-plan-worktree-gate.md`. + +Wave fan-out sites (`execute-phase`) implement both models — see +`gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md`. + +## Base divergence + +Any site that ends up with `ISOLATION != none` must also run the `worktree.base-check` +auto-degrade before dispatch (#683, #1369, #1941), or the agent's `worktree_branch_check` guard +halts on a stale fork base. `quick.md` and `execute-phase` do this today. diff --git a/gsd-core/references/execute-phase-between-wave-reset.md b/gsd-core/references/execute-phase-between-wave-reset.md index a9fef5a81..bef48ceb0 100644 --- a/gsd-core/references/execute-phase-between-wave-reset.md +++ b/gsd-core/references/execute-phase-between-wave-reset.md @@ -7,7 +7,9 @@ 7c. **Between-wave manifest reset and worktree base refresh (waves 2+ only — #1369):** - **REQUIRED before each wave transition when `USE_WORKTREES != "false"` and `RUNTIME = "claude"`.** + **REQUIRED before each wave transition when `USE_WORKTREES != "false"` and + `ISOLATION = "harness-worktree"`** (#2652 — keyed on the negotiated isolation model, not the + runtime name). Wave N's `WAVE_WORKTREE_MANIFEST` was consumed by `worktree.cleanup-wave` in step 5.5. It must be unset so wave N+1's step 3 creates a fresh manifest for the new wave's worktrees. Without this, @@ -26,7 +28,7 @@ # advanced. Re-assert worktree.baseRef:"head" (idempotent — no-op if already set) so the # Claude Code harness re-reads the live HEAD on the next Agent(isolation="worktree") call # rather than using a cached session-start commit as the fork base. - if [ "$RUNTIME" = "claude" ] && [ "$USE_WORKTREES" != "false" ]; then + if [ "$ISOLATION" = "harness-worktree" ] && [ "$USE_WORKTREES" != "false" ]; then gsd_run query worktree.set-baseref 2>/dev/null || true # Safety re-check: evaluate degradation AFTER the wave N commits. If HEAD has diverged @@ -37,7 +39,9 @@ _DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) [ -n "$_DEGRADE_MSG" ] && printf '%s\n' "$_DEGRADE_MSG" >&2 printf 'Degrading to sequential mode for remaining waves: HEAD advanced past worktree fork base after wave %s merge (#1369).\n' "${N}" >&2 + # Both must move together (#2652): dispatch keys on ISOLATION. USE_WORKTREES=false + ISOLATION=none fi fi ``` diff --git a/gsd-core/references/execute-phase-wave-guard.md b/gsd-core/references/execute-phase-wave-guard.md index c28aa496a..9a0ad9067 100644 --- a/gsd-core/references/execute-phase-wave-guard.md +++ b/gsd-core/references/execute-phase-wave-guard.md @@ -7,16 +7,20 @@ immediately with a base-mismatch fatal. **Run this check at the start of every wave when `USE_WORKTREES != "false"` and - `RUNTIME = "claude"`**, including Wave 1 (where it mirrors the initialize-step check): + `ISOLATION = "harness-worktree"`** (#2652 — the harness caches the fork base, so this is a + property of the isolation model, not of the runtime name; Cursor declares it too), + including Wave 1 (where it mirrors the initialize-step check): ```bash - if [ "$RUNTIME" = "claude" ] && [ "${USE_WORKTREES:-true}" != "false" ]; then + if [ "$ISOLATION" = "harness-worktree" ] && [ "${USE_WORKTREES:-true}" != "false" ]; then _WAVE_DEGRADE=$(gsd_run query worktree.base-check --pick shouldDegrade 2>/dev/null || true) if [ "$_WAVE_DEGRADE" = "true" ]; then _WAVE_DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) [ -n "$_WAVE_DEGRADE_MSG" ] && printf '%s\n' "$_WAVE_DEGRADE_MSG" >&2 echo "⚠ [#1369] Worktree fork base diverged from orchestrator HEAD (wave merges advanced HEAD past origin/HEAD). Auto-degrading to sequential mode for this wave to avoid base-mismatch halts." >&2 + # Both must move together (#2652): dispatch keys on ISOLATION. USE_WORKTREES=false + ISOLATION=none fi fi ``` diff --git a/gsd-core/workflows/diagnose-issues.md b/gsd-core/workflows/diagnose-issues.md index da494e5b9..df82a702c 100644 --- a/gsd-core/workflows/diagnose-issues.md +++ b/gsd-core/workflows/diagnose-issues.md @@ -61,12 +61,14 @@ gaps = [ _GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi; if [ -n "${CLAUDE_ENV_FILE:-}" ] && [ -n "${GSD_TOOLS:-}" ]; then printf "export PATH='%s':\"\$PATH\"\n" "${GSD_TOOLS%/*}" >> "$CLAUDE_ENV_FILE" 2>/dev/null || true; fi USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true") RUNTIME=$(gsd_run query config-get runtime --default claude --raw 2>/dev/null || echo "claude") -if [ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]; then - echo "FATAL: git worktree isolation (isolation=\"worktree\") is unsupported on runtime '$RUNTIME' — it would run executor agents unisolated against the main checkout. Set workflow.use_worktrees=false." >&2 - exit 1 -fi ``` +**Resolve isolation now (#2584/#2652).** Read @gsd-core/references/dispatch-isolation-gate.md +and run its `Resolve ISOLATION`, `Single-agent dispatch sites`, and `Resolve the harness flag` +blocks in order; they set `ISOLATION`/`HARNESS_FLAG` via `query dispatch-isolation`. +`ISOLATION` — not `RUNTIME` — gates the worktree decision at spawn; the `spawn_agents` step +below consumes both variables. + **Report diagnosis plan to user:** ``` @@ -108,18 +110,32 @@ the fork base cannot be reliably resolved. The verify-only `/dev/null || true) if [ "$_DIAG_SHOULD_DEGRADE" = "true" ]; then _DIAG_DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) [ -n "$_DIAG_DEGRADE_MSG" ] && printf '%s\n' "$_DIAG_DEGRADE_MSG" >&2 echo "⚠ [#2649] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for diagnosis to avoid a base-mismatch halt." >&2 + ISOLATION=none USE_WORKTREES=false fi fi + +# Re-record after the base-check degrade, immediately before the spawn below, so the +# #3045 sentinel matches the dispatch the guard is about to see (#3045). +gsd_run query dispatch-isolation --raw --force-isolation "$ISOLATION" >/dev/null 2>&1 || true ``` -**Spawn debug agents in parallel:** +**Spawn debug agents — parallel only when each one is isolated:** + +**`ISOLATION` decides the fan-out, not just the flag (#2652).** When +`ISOLATION = "harness-worktree"`, spawn all agents in a single message: each gets its own +worktree, so concurrent edits cannot collide. When `ISOLATION = "none"` — including after the +`orchestrator-worktree` fallback and after the #2649 base-check degrade — the agents would all +run against the **primary checkout**, so spawn them **one at a time**, waiting for each to +return before spawning the next. Fanning out unisolated debuggers is the outcome the +`orchestrator-worktree` degrade exists to avoid; degrading the flag while keeping the +parallelism would announce sequential mode and then do the opposite. For each gap, fill the debug-subagent-prompt template and spawn: @@ -127,6 +143,16 @@ Print: `◆ Spawning diagnostics agent... (each runs in a subagent — no output Before spawning, materialize the guard into WORKTREE_GUARD: read `gsd-core/references/worktree-branch-check.md`, substitute `{EXPECTED_BASE}` with `$EXPECTED_BASE`, and use the resulting `` block (the runnable guard) as WORKTREE_GUARD below. +**Only when `ISOLATION = "harness-worktree"`.** When `ISOLATION = "none"` the agent runs on +the main working tree, where the guard's HEAD assertion cannot hold — set `WORKTREE_GUARD` to +the empty string instead, or every diagnostic agent halts on a base mismatch it was never +meant to check (#2652). + +**Substitute `{harnessFlag}` in the `Agent()` call below** with `$HARNESS_FLAG` followed by a +comma when `ISOLATION = "harness-worktree"`, and with the empty string otherwise — the same +build-time substitution `execute-phase.md` performs. `{harnessFlag}` is a template +placeholder, not a shell variable. + > **Runtime-aware dispatch (#2508 Phase 4).** GSD workflows dispatch specialized subagents by role. Before dispatching on a built-in-only runtime (kimi-code — three built-ins only), resolve the role to a built-in via `gsd_run query resolve-dispatch-type --requested --raw`. On named-dispatch runtimes (Claude/OpenCode/…) the role is returned unchanged; on kimi-code it maps to `coder`/`explore`/`plan` by role-suffix. The persona rides `${AGENT_SKILLS_}` (Phase 3) regardless. See @gsd-core/references/runtime-aware-dispatch.md. @@ -135,14 +161,14 @@ Before spawning, materialize the guard into WORKTREE_GUARD: read `gsd-core/refer Agent( prompt=filled_debug_subagent_prompt + "\n\n" + WORKTREE_GUARD + "\n\n\n- {phase_dir}/{phase_num}-UAT.md\n- {state_path}\n\n${AGENT_SKILLS_DEBUGGER}", subagent_type="gsd-debugger", - ${USE_WORKTREES !== "false" ? 'isolation="worktree",' : ''} + {harnessFlag} description="Debug: {truth_short}" ) ``` > **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above to spawn debug agent(s), stop working on this task immediately. Do not read more files, edit code, or run tests related to these gaps while the subagent(s) are active. Wait for all subagents to return before proceeding. This prevents duplicate work, conflicting edits, and wasted context. -**All agents spawn in single message** (parallel execution). +**All agents spawn in a single message (parallel execution) ONLY when `ISOLATION = "harness-worktree"`.** When `ISOLATION = "none"`, spawn one agent per message and wait for each to return — see the fan-out rule above (#2652). Template placeholders: - `{truth}`: The expected behavior that failed diff --git a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md index 51b1656db..5c714d505 100644 --- a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md +++ b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md @@ -7,6 +7,11 @@ resolution and its fail-closed guard. ## Resolve ISOLATION +The resolution rule is shared with every other dispatch site — see +@gsd-core/references/dispatch-isolation-gate.md, the canonical statement of the +`ISOLATION`-not-`RUNTIME` contract (#2652). This fragment keeps the wave-specific +extras (`worktree.reap-orphans`, the `worktree.base-check` auto-degrade) inline below. + Run this in the config-gate step, right after `RUNTIME`/`USE_WORKTREES` are read. ```bash @@ -20,16 +25,33 @@ _GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-pars # threads the phase identifier into that same atomic write (mode + harnessFlag # + phase together — see hooks/lib/isolation-sentinel.js for how the guards # consume it). -ISOLATION=$(gsd_run query dispatch-isolation --raw --phase "${PHASE_NUMBER:-}" 2>/dev/null || echo "none") +# Keep the resolver's own failure DISTINGUISHABLE from a genuine `none`, exactly +# as references/dispatch-isolation-gate.md does — this site declares that gate +# canonical, so it must not carry the older collapsing shape. Both outcomes fail +# closed, which is right, but only one of them may claim the host declared no +# primitive (#2652 review). +_ISOLATION_RAW=$(gsd_run query dispatch-isolation --raw --phase "${PHASE_NUMBER:-}" 2>/dev/null) +_ISOLATION_RC=$? +if [ $_ISOLATION_RC -ne 0 ] || [ -z "$_ISOLATION_RAW" ]; then + ISOLATION=none + ISOLATION_RESOLVED=false # fail closed, but we did NOT learn a verdict +else + ISOLATION="$_ISOLATION_RAW" + ISOLATION_RESOLVED=true +fi case "$ISOLATION" in harness-worktree|orchestrator-worktree|none) ;; - *) ISOLATION=none ;; + *) ISOLATION=none; ISOLATION_RESOLVED=false ;; # out of vocabulary is not a verdict either esac # Project-level opt-out wins on every host; a host with no primitive fails closed. [ "$USE_WORKTREES" = "false" ] && ISOLATION=none if [ "$ISOLATION" = "none" ] && [ "$USE_WORKTREES" != "false" ]; then - echo "FATAL: runtime '$RUNTIME' declares no executor-isolation primitive (dispatch.isolation=none) — executors would run unisolated against the main checkout. Set workflow.use_worktrees=false." >&2 + if [ "$ISOLATION_RESOLVED" = "true" ]; then + echo "FATAL: runtime '$RUNTIME' declares no executor-isolation primitive (dispatch.isolation=none) — executors would run unisolated against the main checkout. Set workflow.use_worktrees=false." >&2 + else + echo "FATAL: could not resolve this runtime's executor-isolation capability — 'gsd_run query dispatch-isolation' failed or returned nothing, so GSD cannot tell whether isolation is available. Refusing to dispatch rather than guess (a guard that cannot verify must not answer 'safe'). Re-run once the gsd-tools shim resolves, or set workflow.use_worktrees=false to run sequentially on purpose." >&2 + fi exit 1 fi diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index c4ea92b72..d92b56fd3 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -92,6 +92,8 @@ PLAN_START_EPOCH=$(date +%s) # Count tasks — match ]' .planning/phases/XX-name/{phase}-{plan}-PLAN.md 2>/dev/null || echo "0") INLINE_THRESHOLD=$(gsd_run query config-get workflow.inline_plan_threshold 2>/dev/null || echo "2") +USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true") +RUNTIME=$(gsd_run query config-get runtime --default claude --raw 2>/dev/null || echo "claude") grep -n "type=\"checkpoint" .planning/phases/XX-name/{phase}-{plan}-PLAN.md ``` @@ -109,13 +111,38 @@ Otherwise: Apply checkpoint-based routing below. | Verify-only | B (segmented) | Segments between checkpoints. After none/human-verify → SUBAGENT. After decision/human-action → MAIN | | Decision | C (main) | Execute entirely in main context | +**Resolve isolation now — AFTER the pattern is chosen, and only for a pattern that dispatches +(#2584/#2652).** + +- **Pattern C: skip this entirely.** It executes inline in the main context and spawns no + agent, so there is nothing to isolate. Running the gate here would abort an + `isolation`-`none` host with a FATAL for a run that was never going to dispatch anything. +- **Pattern A:** read @gsd-core/references/dispatch-isolation-gate.md and run its + `Resolve ISOLATION`, `Single-agent dispatch sites`, and `Resolve the harness flag` blocks in + order; they set `ISOLATION`/`HARNESS_FLAG` via `query dispatch-isolation`. `ISOLATION` — not + `RUNTIME` — gates the worktree decision. Substitute `{harnessFlag}` in Pattern A's `Agent()` + with `$HARNESS_FLAG`+comma when `ISOLATION = "harness-worktree"`, else empty. + `{harnessFlag}` is a template placeholder, not a shell variable. +- **Pattern B: segments are NOT isolated, and must record that before dispatching.** Each + segment subagent continues on the working tree the previous segment left behind, so putting + them in per-agent worktrees would break the sequence. Record `none` before the first segment + dispatch, or the sentinel still asserts the phase-level `harness-worktree` and the #3045 + `PreToolUse` guard denies every segment dispatch with `exit 2`: + + ```bash + ISOLATION=none + gsd_run query dispatch-isolation --raw --force-isolation none >/dev/null 2>&1 || true + ``` + + Segment dispatches therefore carry no `{harnessFlag}`. + > **Runtime-aware dispatch (#2508 Phase 4).** GSD workflows dispatch specialized subagents by role. Before dispatching on a built-in-only runtime (kimi-code — three built-ins only), resolve the role to a built-in via `gsd_run query resolve-dispatch-type --requested --raw`. On named-dispatch runtimes (Claude/OpenCode/…) the role is returned unchanged; on kimi-code it maps to `coder`/`explore`/`plan` by role-suffix. The persona rides `${AGENT_SKILLS_}` (Phase 3) regardless. See @gsd-core/references/runtime-aware-dispatch.md. -**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → **before spawning, run the #2649 pre-dispatch worktree base-check** (mirrors execute-phase #683/#1369 and quick #1941): if `workflow.use_worktrees` is not `false`, run `gsd_run query worktree.base-check --pick shouldDegrade`; if it returns `true`, print its `--pick message` to stderr, emit the `⚠ [#2649] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for this plan to avoid a base-mismatch halt.` warning, and treat worktrees as disabled for this dispatch (spawn WITHOUT `isolation="worktree"`). Claude Code's `isolation="worktree"` forks from `origin/HEAD`, not live local HEAD; without this gate a plan whose commit advanced local HEAD past a stale `origin/HEAD` hits the verify-only guard's `exit 42` mid-execution with no auto-degrade. → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`) **and the #2649 base-check did not degrade**. **When using `isolation="worktree"`, embed the `` block from `gsd-core/references/worktree-branch-check.md` into the prompt, substituting `{EXPECTED_BASE}` with the captured base SHA.** That guard is **verify-only and fail-closed** (#48) and stays active as a backstop whether or not the base-check degraded: it asserts a per-agent `agent-*` / `worktree-agent-*` branch and the exact base, forbids `git update-ref` self-recovery (#2924), and on any mismatch prints `FATAL:` and `exit 42` so the orchestrator can recover — the sub-agent never rewrites a worktree it did not create. This supersedes the former self-recovery (#2015), whose destructive base rewrite could fail silently under a deny rule; the base-drift it addressed affects all platforms, and base correction is now the orchestrator's responsibility. +**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → **before spawning, run the #2649 pre-dispatch worktree base-check** (mirrors execute-phase #683/#1369 and quick #1941): if `ISOLATION = "harness-worktree"`, run `gsd_run query worktree.base-check --pick shouldDegrade`; if it returns `true`, print its `--pick message` to stderr, emit the `⚠ [#2649] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for this plan to avoid a base-mismatch halt.` warning, and treat `ISOLATION` as `"none"` for this dispatch (spawn without `{harnessFlag}`), **then re-record the degrade before spawning** — run `gsd_run query dispatch-isolation --raw --force-isolation none >/dev/null 2>&1 || true`. That re-record is mandatory, not bookkeeping: the resolve step already persisted `harness-worktree` to the run-scoped sentinel, the degrade above happens where the resolver cannot see it, and the shipped `PreToolUse` isolation guard (#3045) reads that sentinel at the instant of the `Agent()` call — a stale `harness-worktree` against a dispatch that correctly omits `{harnessFlag}` is denied with `exit 2`, so the plan does not run at all. See `Re-record after every degrade` in `gsd-core/references/dispatch-isolation-gate.md`. Claude Code's `isolation="worktree"` forks from `origin/HEAD`, not live local HEAD; without this gate a plan whose commit advanced local HEAD past a stale `origin/HEAD` hits the verify-only guard's `exit 42` mid-execution with no auto-degrade. → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `{harnessFlag}` only when `ISOLATION = "harness-worktree"` and the #2649 base-check did not degrade** — never hardcode `isolation="worktree"`, which is Claude Code's own literal and wrong on any other harness-worktree host. **When dispatching with `{harnessFlag}`, embed the `` block from `gsd-core/references/worktree-branch-check.md` into the prompt, substituting `{EXPECTED_BASE}` with the captured base SHA.** That guard is **verify-only and fail-closed** (#48) and stays active as a backstop whether or not the base-check degraded: it asserts a per-agent `agent-*` / `worktree-agent-*` branch and the exact base, forbids `git update-ref` self-recovery (#2924), and on any mismatch prints `FATAL:` and `exit 42` so the orchestrator can recover — the sub-agent never rewrites a worktree it did not create. This supersedes the former self-recovery (#2015), whose destructive base rewrite could fail silently under a deny rule; the base-drift it addressed affects all platforms, and base correction is now the orchestrator's responsibility. -**Pattern B:** Execute segment-by-segment. Autonomous segments: spawn subagent for assigned tasks only (no SUMMARY/commit). Checkpoints: main context. After all segments: aggregate, create SUMMARY, commit. See segment_execution. +**Pattern B:** Execute segment-by-segment. Autonomous segments: spawn subagent for assigned tasks only (no SUMMARY/commit). Checkpoints: main context. After all segments: aggregate, create SUMMARY, commit. See segment_execution. **Segments run unisolated on the main working tree by design** — each continues where the previous one stopped — so dispatch them WITHOUT `{harnessFlag}`, and only after the `ISOLATION=none` re-record above has run (#2652/#3045). **Pattern C:** Execute in main using standard flow (step name="execute"). diff --git a/gsd-core/workflows/quick.md b/gsd-core/workflows/quick.md index 65af0d3cf..db52251cd 100644 --- a/gsd-core/workflows/quick.md +++ b/gsd-core/workflows/quick.md @@ -154,12 +154,16 @@ PROJECT_PATH="$(dirname "${quick_dir}")/PROJECT.md" ```bash USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true") RUNTIME=$(gsd_run query config-get runtime --default claude --raw 2>/dev/null || echo "claude") -if [ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]; then - echo "FATAL: git worktree isolation (isolation=\"worktree\") is unsupported on runtime '$RUNTIME' — it would run executor agents unisolated against the main checkout. Set workflow.use_worktrees=false." >&2 - exit 1 -fi ``` +**Resolve isolation now (#2584/#2652).** Read @gsd-core/references/dispatch-isolation-gate.md +and run its `Resolve ISOLATION`, `Single-agent dispatch sites`, and `Resolve the harness flag` +blocks in order; they set `ISOLATION`/`HARNESS_FLAG` via `query dispatch-isolation`. +`ISOLATION` — not `RUNTIME` — gates every worktree decision below. Substitute `{harnessFlag}` +in Step 6's `Agent()` with `$HARNESS_FLAG`+comma when `ISOLATION = "harness-worktree"`, else +empty. `{harnessFlag}` +is a template placeholder, not a shell variable. + If `USE_WORKTREES` is not `"false"`, run a startup orphan sweep before spawning any executors. This reaps locked worktrees whose lock-owner process is dead, whose branch is merged into the default branch, and whose lock file mtime is older than 5 minutes. Running it at startup prevents accumulation of orphaned worktrees from prior sessions that exited without cleanup (#3707). ```bash @@ -376,21 +380,37 @@ halts with a base-mismatch fatal — potentially many commits behind, not just o immediately before capturing `EXPECTED_BASE` so it reflects the most current local state. ```bash -if [ "$RUNTIME" = "claude" ] && [ "${USE_WORKTREES:-true}" != "false" ]; then +if [ "$ISOLATION" = "harness-worktree" ] && [ "${USE_WORKTREES:-true}" != "false" ]; then _QUICK_SHOULD_DEGRADE=$(gsd_run query worktree.base-check --pick shouldDegrade 2>/dev/null || true) if [ "$_QUICK_SHOULD_DEGRADE" = "true" ]; then _QUICK_DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) [ -n "$_QUICK_DEGRADE_MSG" ] && printf '%s\n' "$_QUICK_DEGRADE_MSG" >&2 echo "⚠ [#1941] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for this quick task to avoid a base-mismatch halt." >&2 USE_WORKTREES=false + ISOLATION=none fi fi + +# Re-resolve (and, as a side effect, re-persist) now that the base-check +# auto-degrade above may have changed $ISOLATION since the Step 2 gate's +# `dispatch-isolation` call (#3045). That first call recorded the NATURALLY +# resolved mode into the run-scoped sentinel the isolation guard hooks read +# (hooks/gsd-agent-isolation-guard.js, hooks/gsd-cursor-subagent-start.js via +# hooks/lib/isolation-sentinel.js). The degrade above is decided HERE, in +# shell — the resolver cannot see it — so without this the sentinel still +# asserts `harness-worktree` while the dispatch below correctly omits the +# harness flag, and the guard denies the dispatch with exit 2. `--force-isolation` +# pushes the FINAL, shell-computed value through that SAME single write path +# (`none` also clears the stored harnessFlag, since none applies to sequential +# dispatch). Best-effort: a write failure here must never fail the task — the +# guards' own sentinel-absent fallback is safe, just less precise. +gsd_run query dispatch-isolation --raw --force-isolation "$ISOLATION" >/dev/null 2>&1 || true ``` Capture current HEAD before spawning (used for worktree branch check): ```bash EXPECTED_BASE=$(git rev-parse HEAD) -if [ "${USE_WORKTREES:-true}" != "false" ]; then +if [ "$ISOLATION" = "harness-worktree" ]; then # keyed on ISOLATION like every other dispatch-coupled branch (#2652) # BSD/macOS mktemp only randomizes XXXXXX when it is the final path component, so make a # suffixless temp then append the extension — portable across BSD + GNU (#1520). QUICK_WORKTREE_MANIFEST=$(mktemp "${TMPDIR:-/tmp}/gsd-quick-worktree-XXXXXX") && mv "$QUICK_WORKTREE_MANIFEST" "${QUICK_WORKTREE_MANIFEST}.json" && QUICK_WORKTREE_MANIFEST="${QUICK_WORKTREE_MANIFEST}.json" || exit 1 @@ -406,7 +426,7 @@ Agent( prompt=" Execute quick task ${quick_id}. -${USE_WORKTREES !== "false" ? ` +${ISOLATION === "harness-worktree" ? ` ORCHESTRATOR build-time embed (NOT a sub-agent runtime step): before this dispatch, read \`gsd-core/references/worktree-branch-check.md\`, substitute \`{EXPECTED_BASE}\` with the base SHA captured above (${EXPECTED_BASE}), substitute \`{EXPECTED_BASE_ALTERNATE}\` with \`${QUICK_PLAN_PARENT}\` when it differs from \`${EXPECTED_BASE}\` (otherwise empty), and replace this note with that fragment's \`\` block so the dispatched prompt carries the runnable guard verbatim — do not pass this instruction through in its place. @@ -480,17 +500,17 @@ SUMMARY.md and stop — the user must rerun with worktrees disabled. ", subagent_type="gsd-executor", model="{executor_model}", - ${USE_WORKTREES !== "false" ? 'isolation="worktree",' : ''} + {harnessFlag} description="Execute: ${DESCRIPTION}" ) ``` > **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. -If the executor ran with `isolation="worktree"`, append its returned `{agent_id, worktree_path, branch, expected_base, allowed_bases}` metadata to `QUICK_WORKTREE_MANIFEST` before cleanup. Set `expected_base` to `${EXPECTED_BASE}` and `allowed_bases` to `["${EXPECTED_BASE}", "${QUICK_PLAN_PARENT}"]` with duplicates removed. If any required field is unavailable, stop and ask for recovery; do not discover global worktrees. +If the executor ran isolated (`ISOLATION = "harness-worktree"` at dispatch), append its returned `{agent_id, worktree_path, branch, expected_base, allowed_bases}` metadata to `QUICK_WORKTREE_MANIFEST` before cleanup. Set `expected_base` to `${EXPECTED_BASE}` and `allowed_bases` to `["${EXPECTED_BASE}", "${QUICK_PLAN_PARENT}"]` with duplicates removed. If any required field is unavailable, stop and ask for recovery; do not discover global worktrees. After executor returns: -1. **Worktree cleanup:** If the executor ran with `isolation="worktree"`, merge the worktree branch back and clean up: +1. **Worktree cleanup:** If the executor ran isolated (`ISOLATION = "harness-worktree"` at dispatch), merge the worktree branch back and clean up: ```bash QUICK_WORKTREE_MANIFEST=${QUICK_WORKTREE_MANIFEST:-$WAVE_WORKTREE_MANIFEST} [ -n "${QUICK_WORKTREE_MANIFEST:-}" ] && [ -f "$QUICK_WORKTREE_MANIFEST" ] || { @@ -504,7 +524,7 @@ After executor returns: # Fail closed: SDK refusal (safety guard #3174/#3384) must surface — do not swallow exit 1. gsd_run query worktree.cleanup-wave --manifest "$QUICK_WORKTREE_MANIFEST" || exit 1 ``` - If `workflow.use_worktrees` is `false`, skip this step. + If `ISOLATION` was not `"harness-worktree"` at dispatch (including a #1941 base-check degrade — that is *this* file's degrade; #2649 is the `diagnose-issues.md` / `execute-plan.md` one), skip this step. > **ISOLATED-RUN RECOVERY — FAIL SAFE (#1292):** When an isolated (worktree) run is *rejected* — the user declines to merge it, the orchestrator surfaces recovery guidance for a blocked/halted plan, or the run over-reached the requested scope — the worktree-isolation contract MUST hold through recovery. Do **NOT** propose continuing on `main`/the primary checkout as the default or recommended recovery path. Default to a **safe halt** and offer: (a) re-attempt in a **fresh, narrowly-scoped worktree**, or (b) inspect or discard the rejected worktree without merging. Any path that edits the primary checkout requires an **explicit, clearly-labeled confirmation** from the user first — editing `main` directly is never the proposed or default option for a run the user configured to be isolated. diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index 210564d64..b6dc879a7 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -24,6 +24,7 @@ const { readGsdCommandNames, transformContentToHyphen } = commandRoster; import runtimeNamePolicy = require('./runtime-name-policy.cjs'); const { getDirName } = runtimeNamePolicy; import capabilityRegistry = require('./capability-registry.cjs'); +import hostIntegration = require('./host-integration.cjs'); import { posixNormalize } from './shell-command-projection.cjs'; // #2870: install-scope.cts is a leaf-tier sibling (imports only // runtime-homes.cjs + node builtins, never this module) — no cycle. See the @@ -2535,20 +2536,76 @@ const NON_CLAUDE_RUNTIMES: string[] = Object.keys(capabilityRegistry.runtimes) .filter((id) => id !== 'claude') .sort(); +/** + * #2652: The isolation a runtime can actually negotiate at dispatch time, + * resolved from the registry exactly as `gsd_run query dispatch-isolation` + * resolves it at runtime (`routeDispatchIsolation`, gsd-core/bin/gsd-tools.cjs): + * the declared value must be in the closed vocabulary, a `harness-worktree` + * host must also declare the flag the scheduler passes, and an + * `orchestrator-worktree` host must carry a descriptor that resolves. Anything + * else — unknown runtime, `undocumented`, out-of-vocabulary, a throw — is + * `none` (ADR-1239, "Fail-closed"). + * + * Install time cannot know the worktree path a future dispatch will target, so + * the descriptor is probed with a placeholder; `resolveOrchestratorExec` fails + * only on descriptor shape, never on a well-formed target's value. + * + * @private — exported as `_negotiatedDispatchIsolation` for tests. + */ +function _negotiatedDispatchIsolation(runtime: string): string { + try { + const runtimeEntry = capabilityRegistry?.runtimes?.[runtime] ?? null; + const declared = runtimeEntry?.runtime?.hostIntegration?.dispatch?.isolation ?? null; + + if (declared === 'harness-worktree') { + const declaredFlag = runtimeEntry?.runtime?.harnessIsolationFlag ?? null; + return typeof declaredFlag === 'string' && declaredFlag.length > 0 + ? 'harness-worktree' + : 'none'; + } + + if (declared === 'orchestrator-worktree') { + return hostIntegration.resolveOrchestratorExec( + runtimeEntry?.runtime?.orchestratorExec, + '/gsd-orchestrator-worktree-probe', + ).ok + ? 'orchestrator-worktree' + : 'none'; + } + + return 'none'; + } catch { + return 'none'; + } +} + /** * #1521: Every non-Claude runtime resolves its own runtime identity from a - * runtime-neutral config, and defaults workflow.use_worktrees to false — - * GSD's worktree isolation uses Claude Code's isolation="worktree" spawn - * parameter, which no other runtime honors. Stamped into the emitted - * workflow runtime-resolution blocks. (Generalizes the Codex-only #1515 fix.) + * runtime-neutral config. Stamped into the emitted workflow runtime-resolution + * blocks. (Generalizes the Codex-only #1515 fix.) + * + * #1521 also stamped `workflow.use_worktrees` to default false for every + * non-Claude runtime, because GSD's worktree isolation was Claude Code's + * `isolation="worktree"` spawn parameter and no other runtime honored it. + * #2584 removed that premise: isolation is now a negotiated capability + * (`dispatch.isolation`), and Cursor declares `harness-worktree` while Codex, + * OpenCode, Kimi and Kimi Code declare `orchestrator-worktree`. Stamping the + * false default for those hosts resolved `USE_WORKTREES=false` before + * `dispatch.isolation` was ever consulted, so a runtime that declares worktree + * support still got `ISOLATION=none` — judged by its name after all, which is + * the defect #2652 exists to remove. The stamp is therefore scoped to the + * runtimes whose negotiated isolation really is `none`, where the default it + * writes is the outcome the resolver would reach anyway. * * @private — exported as `_stampNonClaudeRuntimeDefaults` for tests. */ function _stampNonClaudeRuntimeDefaults(content: string, runtime: string): string { - content = content.replace( - /config-get workflow\.use_worktrees --raw 2>\/dev\/null \|\| echo "true"/g, - 'config-get workflow.use_worktrees --default false --raw 2>/dev/null || echo "false"', - ); + if (_negotiatedDispatchIsolation(runtime) === 'none') { + content = content.replace( + /config-get workflow\.use_worktrees --raw 2>\/dev\/null \|\| echo "true"/g, + 'config-get workflow.use_worktrees --default false --raw 2>/dev/null || echo "false"', + ); + } content = content.replace( /config-get runtime --default claude --raw 2>\/dev\/null \|\| echo "claude"/g, `config-get runtime --default ${runtime} --raw 2>/dev/null || echo "${runtime}"`, @@ -3092,6 +3149,8 @@ export = { _computePathPrefix: computePathPrefix, _applyRuntimeRewrites, _stampNonClaudeRuntimeDefaults, + // #2652: registry-resolved dispatch isolation, mirroring routeDispatchIsolation + _negotiatedDispatchIsolation, // #1521: canonical non-Claude runtime list for test files and tooling NON_CLAUDE_RUNTIMES, }; diff --git a/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json b/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json deleted file mode 100644 index 1f4de8d3e..000000000 --- a/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": 1, - "paths": { - "diagnose-issues.md": "#2649: spawn_agents step gained a pre-dispatch worktree.base-check gate (mirrors execute-phase #683/#1369 and quick #1941). Claude Code's isolation=\"worktree\" forks from origin/HEAD, not live local HEAD; without the gate the documented GSD steady state (commit every step locally, push only on request) hit the verify-only worktree_branch_check guard's exit-42 halt mid-investigation. Growth is the base-check bash block (gsd_run query worktree.base-check --pick shouldDegrade → USE_WORKTREES=false + stderr warning) + the #2649 rationale comment. The verify-only guard stays as a backstop." - } -} diff --git a/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json b/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json new file mode 100644 index 000000000..342e327dc --- /dev/null +++ b/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "paths": { + "quick.md": "#2652 — same migration. The resolution block lives in gsd-core/references/dispatch-isolation-gate.md and this file @-references it, so this file's own bytes stay under its prompt-stuffing size limit (SIZE_ONLY_WORKFLOWS on next). To be accurate about what that buys: @-references are EAGERLY INLINED, so the extraction does NOT reduce loaded context — it slightly increases it (reference body plus the pointer). The reason to extract is single-sourcing the gate across the four sites that reference it (quick.md, diagnose-issues.md, execute-plan.md, execute-phase/steps/executor-isolation-dispatch.md), not context economy; an earlier revision of this entry claimed otherwise and was wrong. Growth is the reference pointer, the ISOLATION=none degrade pairing, the post-dispatch bookkeeping conditions re-keyed from the Claude-rendered literal onto ISOLATION (round-4 review Blocker), and the #3045 re-record after the #1941 base-check degrade (round-5 review Blocker B1).", + "diagnose-issues.md": "#2652 — migrates the dispatch site off the pre-#2584 RUNTIME != \"claude\" gate onto the negotiated dispatch.isolation seam. Growth is the pointer at gsd-core/references/dispatch-isolation-gate.md, the ISOLATION-conditional WORKTREE_GUARD/{harnessFlag} substitution instructions this file had no equivalent of, and the #3045 re-record after the #2649 base-check degrade (round-5 review Blocker B1). The resolution / orchestrator-worktree degrade / harness-flag shell is NOT inline here: an earlier revision of this branch inlined a reordered copy of the reference, which is the divergence a single source of truth exists to prevent (round-6 review Major 3). This file now reads the reference the same way quick.md and execute-plan.md do, so its own delta shrank accordingly.", + "execute-plan.md": "#2652 — the fifth dispatch site, converted after review: Pattern A hardcoded isolation=\"worktree\" (Claude Code's literal) gated only on workflow.use_worktrees, with no capability negotiation. Growth is the USE_WORKTREES/RUNTIME config reads and the pointer at gsd-core/references/dispatch-isolation-gate.md (the resolution block itself living in that reference rather than inline here), the #2649 base-check merged into Pattern A's paragraph by that rebase, and the #3045 re-record that degrade now performs before spawning (round-5 review Blocker B1).", + "gsd-core/workflows/execute-phase.md": "#2652 review round 3 — not a content change to this workflow, which is byte-identical in the source tree. The delta is emit-time: _stampNonClaudeRuntimeDefaults no longer rewrites the workflow.use_worktrees read to --default false for a runtime whose negotiated dispatch.isolation is not none, so the emitted line keeps the unstamped true default. It moves for exactly the five hosts that declare worktree support (cursor harness-worktree; codex/opencode/kimi/kimi-code orchestrator-worktree) and is unchanged for every isolation-none runtime. Fixes the review Blocker: the install-time stamp resolved USE_WORKTREES=false before dispatch-isolation was ever consulted, so the gate this PR migrates dispatch onto was still deciding isolation by runtime name one layer down." + } +} diff --git a/tests/emitted-drift-acks/2658-trae-instruction-file-path.json b/tests/emitted-drift-acks/2658-trae-instruction-file-path.json index 6e8bfd2d0..05c4c5106 100644 --- a/tests/emitted-drift-acks/2658-trae-instruction-file-path.json +++ b/tests/emitted-drift-acks/2658-trae-instruction-file-path.json @@ -11,7 +11,6 @@ "gsd-core/templates/README.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", "gsd-core/templates/claude-md.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", "gsd-core/templates/codebase/structure.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", - "gsd-core/workflows/execute-phase.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", "gsd-core/workflows/execute-plan.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", "gsd-core/workflows/help/modes/full.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", "gsd-core/workflows/milestone-summary.md": "#2658: same CLAUDE.md replacement-target change as checkpoints.md above (bare directory '.trae/rules/' -> concrete file '.trae/rules/rules.md').", diff --git a/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json b/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json index 62ea585aa..d1c2fb591 100644 --- a/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json +++ b/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json @@ -1,9 +1,6 @@ { "version": 1, "paths": { - "execute-plan.md": { - "reason": "Issue #3218 (epic #3180 phase 8): replaced a raw `ls *-PLAN.md | wc -l` re-derivation with a `gsd_run query find-phase` call plus jq extraction of the count field, routing the plan count through scanPhasePlans (ADR-3180 §7.5) instead of a glob. A few lines longer than the shell one-liner it replaces." - }, "plan-phase.md": { "reason": "Issue #3218 (epic #3180 phase 8): grew the most of the four files because it migrated TWO sites off `ls *-PLAN.md | wc -l` (the step 9a and step 11a filesystem-fallback DISK_PLANS checks) to `gsd_run query find-phase` + jq, and because both sites now take the PHYSICAL plan set (`plan_count_all`, status:superseded included) for their 'did the planner/checker write files to disk?' question — the old glob undercounted on the nested plans/ layout and missed loosely-named files." }, diff --git a/tests/execute-phase-wave.test.cjs b/tests/execute-phase-wave.test.cjs index a2ae73a86..3ec636cff 100644 --- a/tests/execute-phase-wave.test.cjs +++ b/tests/execute-phase-wave.test.cjs @@ -702,11 +702,24 @@ describe('execute-phase: inter-wave worktree base re-check (#1369)', () => { assert.ok(refIdx < step1Idx, 'wave-guard @-reference must appear before step 1'); }); - test('step 0.5 guards on RUNTIME=claude (worktree isolation is Claude Code-specific)', () => { + // #2652: previously required `RUNTIME = "claude"`, encoding the pre-#2584 premise + // that worktree isolation is Claude-specific. #2584 replaced that with the + // negotiated dispatch.isolation capability — Cursor declares harness-worktree too, + // and the harness fork-base caching this guard exists for is a property of the + // isolation model, not of the runtime name. + test('step 0.5 guards on the negotiated capability, not a runtime id', () => { const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); assert.ok( - content.includes('RUNTIME') && (content.includes('"claude"') || content.includes("'claude'")), - 'step 0.5 must guard on RUNTIME=claude' + content.includes('ISOLATION') && content.includes('harness-worktree'), + 'step 0.5 must guard on ISOLATION = harness-worktree' + ); + assert.ok( + !/\[\s*"\$RUNTIME"\s*=/.test(content), + 'step 0.5 must NOT branch on a RUNTIME literal (#2584/#2652)' + ); + assert.ok( + content.includes('ISOLATION=none'), + 'degrade must clear ISOLATION as well as USE_WORKTREES — dispatch reads ISOLATION (#2652)' ); }); @@ -780,11 +793,21 @@ describe('execute-phase: between-wave manifest reset (#1369, #3384)', () => { assert.ok(refPtr < idx8, 'between-wave @-reference must appear before step 8'); }); - test('step 7c guards on RUNTIME=claude for worktree-specific operations', () => { + // #2652: see the step 0.5 note above — migrated from the runtime-name premise to + // the negotiated dispatch.isolation capability. + test('step 7c guards on the negotiated capability, not a runtime id', () => { const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); assert.ok( - content.includes('RUNTIME') && (content.includes('"claude"') || content.includes("'claude'")), - 'step 7c must guard on RUNTIME=claude' + content.includes('ISOLATION') && content.includes('harness-worktree'), + 'step 7c must guard on ISOLATION = harness-worktree' + ); + assert.ok( + !/\[\s*"\$RUNTIME"\s*=/.test(content), + 'step 7c must NOT branch on a RUNTIME literal (#2584/#2652)' + ); + assert.ok( + content.includes('ISOLATION=none'), + 'degrade must clear ISOLATION as well as USE_WORKTREES — dispatch reads ISOLATION (#2652)' ); }); }); diff --git a/tests/fix-1941-quick-worktree-stale-base.test.cjs b/tests/fix-1941-quick-worktree-stale-base.test.cjs index 0b59589dd..614711b1b 100644 --- a/tests/fix-1941-quick-worktree-stale-base.test.cjs +++ b/tests/fix-1941-quick-worktree-stale-base.test.cjs @@ -46,23 +46,38 @@ describe('quick: pre-dispatch worktree base re-check (#1941)', () => { assert.ok(content.includes('#1941'), 'quick.md must reference #1941'); }); - test('degrade check sets USE_WORKTREES=false when shouldDegrade is true', () => { + test('degrade check clears BOTH USE_WORKTREES and ISOLATION when shouldDegrade is true', () => { const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); const baseCheckIdx = content.indexOf('worktree.base-check'); - const block = content.slice(baseCheckIdx, baseCheckIdx + 600); + const block = content.slice(baseCheckIdx, baseCheckIdx + 900); + assert.ok(block.includes('shouldDegrade'), 'degrade check must branch on shouldDegrade'); + // Both must move together (#2652). Dispatch keys on ISOLATION while the prompt + // guard and worktree manifest key on USE_WORKTREES; clearing only one dispatches + // an isolated executor with no base guard and no manifest, then blocks in cleanup + // looking for a manifest that was never initialized. + assert.ok(block.includes('USE_WORKTREES=false'), 'degrade must set USE_WORKTREES=false'); assert.ok( - block.includes('shouldDegrade') && block.includes('USE_WORKTREES=false'), - 'degrade check must override USE_WORKTREES=false when shouldDegrade is true' + block.includes('ISOLATION=none'), + 'degrade must ALSO set ISOLATION=none — dispatch reads ISOLATION, so clearing only ' + + 'USE_WORKTREES still passes the harness isolation flag (#2652)' ); }); - test('degrade check guards on RUNTIME=claude (worktree isolation is Claude Code-specific)', () => { + // #2652: this assertion previously required `RUNTIME = "claude"`, encoding the + // pre-#2584 premise that worktree isolation is Claude-specific. #2584 replaced + // that with the negotiated dispatch.isolation capability, so the guard now keys + // on the capability — Cursor also declares harness-worktree. + test('degrade check guards on the negotiated capability, not a runtime id', () => { const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); const baseCheckIdx = content.indexOf('worktree.base-check'); - const block = content.slice(Math.max(0, baseCheckIdx - 200), baseCheckIdx + 200); + const block = content.slice(Math.max(0, baseCheckIdx - 300), baseCheckIdx + 200); assert.ok( - block.includes('RUNTIME') && (block.includes('"claude"') || block.includes("'claude'")), - 'degrade check must guard on RUNTIME=claude' + block.includes('ISOLATION') && block.includes('harness-worktree'), + 'degrade check must guard on ISOLATION = harness-worktree' + ); + assert.ok( + !/\[\s*"\$RUNTIME"\s*=/.test(block), + 'degrade check must NOT branch on a RUNTIME literal (#2584/#2652)' ); }); diff --git a/tests/fixtures/install-tree/antigravity.json b/tests/fixtures/install-tree/antigravity.json index e029172d3..66964e1d8 100644 --- a/tests/fixtures/install-tree/antigravity.json +++ b/tests/fixtures/install-tree/antigravity.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/augment.json b/tests/fixtures/install-tree/augment.json index d646d27c8..88faa6cbc 100644 --- a/tests/fixtures/install-tree/augment.json +++ b/tests/fixtures/install-tree/augment.json @@ -142,6 +142,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/claude-local.json b/tests/fixtures/install-tree/claude-local.json index ea845fc3b..1edfce4d8 100644 --- a/tests/fixtures/install-tree/claude-local.json +++ b/tests/fixtures/install-tree/claude-local.json @@ -141,6 +141,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/claude.json b/tests/fixtures/install-tree/claude.json index a754641ff..b64a3012d 100644 --- a/tests/fixtures/install-tree/claude.json +++ b/tests/fixtures/install-tree/claude.json @@ -70,6 +70,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/cline.json b/tests/fixtures/install-tree/cline.json index e469b877b..4ccf9af3e 100644 --- a/tests/fixtures/install-tree/cline.json +++ b/tests/fixtures/install-tree/cline.json @@ -74,6 +74,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/codebuddy.json b/tests/fixtures/install-tree/codebuddy.json index 4d8a51ddd..616aeaca5 100644 --- a/tests/fixtures/install-tree/codebuddy.json +++ b/tests/fixtures/install-tree/codebuddy.json @@ -142,6 +142,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/codex.json b/tests/fixtures/install-tree/codex.json index c5bd41e73..d62d45316 100644 --- a/tests/fixtures/install-tree/codex.json +++ b/tests/fixtures/install-tree/codex.json @@ -177,6 +177,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/copilot.json b/tests/fixtures/install-tree/copilot.json index ebdf2fe99..93dcda8be 100644 --- a/tests/fixtures/install-tree/copilot.json +++ b/tests/fixtures/install-tree/copilot.json @@ -72,6 +72,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/cursor.json b/tests/fixtures/install-tree/cursor.json index 88319b726..29c979a08 100644 --- a/tests/fixtures/install-tree/cursor.json +++ b/tests/fixtures/install-tree/cursor.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/hermes.json b/tests/fixtures/install-tree/hermes.json index f5988a958..55738333e 100644 --- a/tests/fixtures/install-tree/hermes.json +++ b/tests/fixtures/install-tree/hermes.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/kilo.json b/tests/fixtures/install-tree/kilo.json index 19d08fc7b..ac782c5f9 100644 --- a/tests/fixtures/install-tree/kilo.json +++ b/tests/fixtures/install-tree/kilo.json @@ -142,6 +142,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/kimi-code.json b/tests/fixtures/install-tree/kimi-code.json index d1eda65f2..d6daead57 100644 --- a/tests/fixtures/install-tree/kimi-code.json +++ b/tests/fixtures/install-tree/kimi-code.json @@ -103,6 +103,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/kimi.json b/tests/fixtures/install-tree/kimi.json index 3d55c88e9..575b49e01 100644 --- a/tests/fixtures/install-tree/kimi.json +++ b/tests/fixtures/install-tree/kimi.json @@ -139,6 +139,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/opencode.json b/tests/fixtures/install-tree/opencode.json index b1da53bf5..b78f2e2a1 100644 --- a/tests/fixtures/install-tree/opencode.json +++ b/tests/fixtures/install-tree/opencode.json @@ -142,6 +142,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/pi.json b/tests/fixtures/install-tree/pi.json index 15b976fbe..defc30f5c 100644 --- a/tests/fixtures/install-tree/pi.json +++ b/tests/fixtures/install-tree/pi.json @@ -39,6 +39,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/qwen.json b/tests/fixtures/install-tree/qwen.json index ccba25186..aaa750cc6 100644 --- a/tests/fixtures/install-tree/qwen.json +++ b/tests/fixtures/install-tree/qwen.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/trae.json b/tests/fixtures/install-tree/trae.json index fe49afaa9..b3a739d28 100644 --- a/tests/fixtures/install-tree/trae.json +++ b/tests/fixtures/install-tree/trae.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/windsurf.json b/tests/fixtures/install-tree/windsurf.json index 58567748c..9666f1e9d 100644 --- a/tests/fixtures/install-tree/windsurf.json +++ b/tests/fixtures/install-tree/windsurf.json @@ -71,6 +71,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/fixtures/install-tree/zcode.json b/tests/fixtures/install-tree/zcode.json index 64325b2c4..8c936cece 100644 --- a/tests/fixtures/install-tree/zcode.json +++ b/tests/fixtures/install-tree/zcode.json @@ -142,6 +142,7 @@ "gsd-core/references/debugger-semantic-recall.md", "gsd-core/references/debugger-techniques.md", "gsd-core/references/decimal-phase-calculation.md", + "gsd-core/references/dispatch-isolation-gate.md", "gsd-core/references/doc-conflict-engine.md", "gsd-core/references/domain-probes.md", "gsd-core/references/edge-probe-fixtures/01-round-half-even/expected-coverage.json", diff --git a/tests/host-integration.test.cjs b/tests/host-integration.test.cjs index d3ff09599..d4cd594de 100644 --- a/tests/host-integration.test.cjs +++ b/tests/host-integration.test.cjs @@ -35,6 +35,7 @@ const { _HOST_INTEGRATION_VOCAB, validateCapability, } = require('../gsd-core/bin/lib/capability-validator.cjs'); +const { cleanup, readFileNormalized } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); @@ -1952,3 +1953,552 @@ describe('#2627 dispatch-isolation CLI route', () => { assert.equal(pi.harnessFlag, null); }); }); + +// --------------------------------------------------------------------------- +// #2652 — dispatch-site parity: isolation is decided by the negotiated +// dispatch.isolation capability, never by a runtime id. +// +// #2584 migrated the phase scheduler off `RUNTIME != "claude"` but left quick.md +// and diagnose-issues.md behind, so Codex — which declares orchestrator-worktree +// — was refused isolation it had negotiated. That is this repo's +// DEFECT.GENERATIVE-FIX-DIVERGENCE shape: parallel surfaces reading one contract, +// one migrated and the others silently stale. This guard fails when any dispatch +// site reintroduces a runtime-name test around its isolation decision. +// --------------------------------------------------------------------------- +// Scan workflows AND the reference fragments they inline: scheduler branches that +// mutate USE_WORKTREES live in gsd-core/references/ too (execute-phase-wave-guard, +// execute-phase-between-wave-reset), and a workflows-only scan misses them. +// Shared by both #2652 and #2728 suites below — a second, hand-listed copy is how +// the degrade scan silently narrowed to three files (round-7 review, Major 6). +const SCAN_ROOTS = [ + path.join(REPO_ROOT, 'gsd-core', 'workflows'), + path.join(REPO_ROOT, 'gsd-core', 'references'), +]; + +function collectMarkdown(dir) { + const out = []; + if (!fs.existsSync(dir)) return out; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...collectMarkdown(full)); + else if (entry.name.endsWith('.md')) out.push(full); + } + return out; +} + +describe('#2652 dispatch-site parity — isolation gates on capability, not runtime id', () => { + const ISOLATION_TOKEN = /USE_WORKTREES|ISOLATION|isolation="worktree"|harnessFlag/; + + // Any shape that reads RUNTIME as a branch condition. Line-based matching missed + // multiline `&&`, [[ ]], case, `test`, and JS-template forms, so match the RUNTIME + // test itself and then look for an isolation token within the following window. + // + // Operand ORDER is not fixed either: `[ "claude" != "$RUNTIME" ]` is the same gate + // written backwards, and hand-written left-only patterns let it through. Each + // comparison shape is therefore generated in both orders from a single template, so + // a new shape cannot be added in one order and forgotten in the other. + const RT = '"?\\$\\{?RUNTIME\\}?"?'; // $RUNTIME / "$RUNTIME" / "${RUNTIME}" + const JS_RT = 'RUNTIME'; // bare identifier inside a ${…} template + const QLIT = '["\'][a-z-]+["\']'; // "claude" + const LIT = '["\']?[a-z-]+["\']?'; // claude / "claude" ([[ ]] permits bare) + + /** One comparison shape → two regexes, operands in either order. */ + const bothOrders = (tpl, a, b) => [ + new RegExp(tpl.replace('%L', a).replace('%R', b)), + new RegExp(tpl.replace('%L', b).replace('%R', a)), + ]; + + const RUNTIME_TESTS = [ + ...bothOrders('\\[\\s*%L\\s*(?:!=|==?)\\s*%R\\s*\\]', RT, QLIT), // [ "$RUNTIME" = "x" ] + ...bothOrders('\\[\\[\\s*%L\\s*(?:!=|==?)\\s*%R\\s*\\]\\]', RT, LIT), // [[ "$RUNTIME" == x ]] + ...bothOrders('\\btest\\s+%L\\s*(?:!=|=)\\s*%R', RT, QLIT), // test "$RUNTIME" = "x" + ...bothOrders('\\$\\{\\s*%L\\s*(?:===|!==|==|!=)\\s*%R', JS_RT, QLIT), // ${RUNTIME === "x" ? ...} + /\bcase\s+"?\$\{?RUNTIME\}?"?\s+in\b/, // case "$RUNTIME" in (no reversed form) + ]; + + const WINDOW = 400; // chars after the RUNTIME test to look for an isolation decision + + function isolationGateOffenders(text, label) { + const hits = []; + for (const re of RUNTIME_TESTS) { + const global = new RegExp(re.source, 'g'); + let m; + while ((m = global.exec(text)) !== null) { + const window = text.slice(m.index, m.index + WINDOW); + if (ISOLATION_TOKEN.test(window)) { + const line = text.slice(0, m.index).split('\n').length; + hits.push(`${label}:${line}: ${m[0].trim()}`); + } + } + } + return hits; + } + + const dispatchSites = SCAN_ROOTS.flatMap(collectMarkdown).filter(f => + ISOLATION_TOKEN.test(fs.readFileSync(f, 'utf-8')), + ); + + test('the scan covers the known dispatch sites (guards against a vacuous pass)', () => { + const rel = dispatchSites.map(f => path.relative(REPO_ROOT, f).replace(/\\/g, '/')); + // Assert identities, not just a count: a count survives the scan silently + // drifting off the files that actually matter. + for (const required of [ + 'gsd-core/workflows/quick.md', + 'gsd-core/workflows/diagnose-issues.md', + 'gsd-core/workflows/execute-plan.md', + 'gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md', + ]) { + assert.ok(rel.includes(required), `dispatch-site scan must cover ${required}; found ${rel.length} files`); + } + }); + + // #2652 review: executor-isolation-dispatch.md declared + // references/dispatch-isolation-gate.md canonical while keeping the OLDER + // collapsing resolver inline — `|| echo "none"`, no ISOLATION_RESOLVED — so a + // transient shim failure aborted /gsd:execute-phase telling a Claude or + // Cursor user their runtime "declares no executor-isolation primitive". Still + // fail-closed, so not an unsafe-dispatch hole, but the correction this PR is + // about was unwired at one of the five sites. Nothing caught it: the emitted + // coverage in install.test.cjs checks the REFERENCE, not each site's own copy. + test('every site that inlines the resolver uses the non-collapsing shape', () => { + // A site "inlines the resolver" only if it ASSIGNS from it. The other + // dispatch sites @-reference the gate and merely re-record a degrade + // (`--force-isolation …` with no assignment), which carries no verdict of + // its own — matching those too would flag files that have nothing to fix. + const INLINE_RESOLVE = /\w+=\$\(gsd_run query dispatch-isolation --raw/; + // Dedupe: SCAN_ROOTS already yields the gate reference, so appending it + // again made `length >= 2` satisfiable by the gate alone — the executor + // site could drop out of the predicate entirely and this would still pass. + const candidates = [...new Set( + [...dispatchSites, path.join(REPO_ROOT, 'gsd-core', 'references', 'dispatch-isolation-gate.md')] + .filter(f => fs.existsSync(f)) + .map(f => path.resolve(f)), + )]; + const inliners = candidates + .map(f => ({ rel: path.relative(REPO_ROOT, f).replace(/\\/g, '/'), text: fs.readFileSync(f, 'utf-8') })) + .filter(({ text }) => INLINE_RESOLVE.test(text)); + + // Pin identities, not a count. A count cannot tell "the executor site was + // fixed" from "the executor site stopped matching the predicate". + assert.deepEqual( + inliners.map(i => i.rel).sort(), + [ + 'gsd-core/references/dispatch-isolation-gate.md', + 'gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md', + ], + 'the set of files inlining the resolver changed — a new inliner needs the same treatment, and a missing one means the predicate stopped seeing it', + ); + + for (const { rel, text } of inliners) { + // Any assignment target, not just ISOLATION — `_ISOLATION_RAW=$(… || echo + // "none")` restores the identical defect while leaving ISOLATION_RESOLVED + // in the file, so a name-specific pattern passes on a broken block. + assert.doesNotMatch( + text, + /\w+=\$\(gsd_run query dispatch-isolation --raw[^\n]*\|\|[^\n]*echo/, + `${rel}: collapses a resolver failure straight to none — that reports "declares no primitive" for a host that simply could not be queried`, + ); + assert.match( + text, + /ISOLATION_RESOLVED=false/, + `${rel}: inlines the resolver but never records that no verdict was learned`, + ); + assert.match( + text, + /could not resolve this runtime's executor-isolation capability/, + `${rel}: has no distinct message for the unresolved case, so both outcomes read as a capability verdict`, + ); + } + }); + + test('the detector flags every known runtime-gate shape (discrimination proof)', () => { + // Each of these slipped past the original same-line, single-bracket detector. + const mutations = { + 'single bracket, same line': + 'if [ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]; then', + 'multiline &&': + 'if [ "$RUNTIME" != "claude" ] && \\\n [ "$USE_WORKTREES" != "false" ]; then', + 'double bracket': + 'if [[ "$RUNTIME" == claude ]]; then\n USE_WORKTREES=false\nfi', + 'nested, later assignment': + 'if [ "$RUNTIME" = "codex" ]; then\n echo hi\n ISOLATION=none\nfi', + 'case statement': + 'case "$RUNTIME" in\n claude) USE_WORKTREES=true ;;\nesac', + 'js template': + '${RUNTIME === "claude" ? \'isolation="worktree",\' : \'\'}', + 'test builtin': + 'if test "$RUNTIME" = "claude"; then\n ISOLATION=harness-worktree\nfi', + // Reversed operands — the same gate written backwards. Every one of these + // evaded the original left-only patterns (#2728 review, Minor). + 'reversed single bracket': + 'if [ "claude" != "$RUNTIME" ] && [ "$USE_WORKTREES" != "false" ]; then', + 'reversed double bracket': + 'if [[ claude == "$RUNTIME" ]]; then\n USE_WORKTREES=false\nfi', + 'reversed test builtin': + 'if test "claude" = "$RUNTIME"; then\n ISOLATION=harness-worktree\nfi', + 'reversed js template': + '${"claude" === RUNTIME ? \'isolation="worktree",\' : \'\'}', + }; + for (const [name, snippet] of Object.entries(mutations)) { + assert.equal( + isolationGateOffenders(snippet, 'mutation').length >= 1, + true, + `detector must flag the "${name}" reintroduction — otherwise the guard below proves nothing`, + ); + } + }); + + test('the detector flags generated shell-comparison permutations (property)', () => { + // The mutation table above is 11 hand-picked cases; this generates the cross + // product of the axes an author actually varies — bracket form, operator, + // operand order, quoting, spacing, runtime id. A permutation the hand-written + // patterns miss shows up here rather than in production (#2728 review, Nit). + fc.assert( + fc.property( + fc.constantFrom('[', '[[', 'test'), + fc.constantFrom('=', '==', '!='), + fc.boolean(), // reversed operands? + fc.constantFrom('"$RUNTIME"', '$RUNTIME', '"${RUNTIME}"'), + fc.constantFrom('claude', 'codex', 'kimi-code'), + fc.boolean(), // quote the literal? + fc.constantFrom('', ' '), // extra padding + (form, op, reversed, rtTok, id, quoted, pad) => { + // `test` has no `==` form and never takes brackets; bare literals are + // only legal inside [[ ]]. + if (form === 'test' && op === '==') return true; + const lit = quoted || form !== '[[' ? `"${id}"` : id; + const [l, r] = reversed ? [lit, rtTok] : [rtTok, lit]; + const cond = `${l}${pad} ${op} ${pad}${r}`; + const snippet = form === 'test' + ? `if test ${cond}; then\n ISOLATION=none\nfi` + : `if ${form} ${cond} ${form === '[[' ? ']]' : ']'}; then\n ISOLATION=none\nfi`; + return isolationGateOffenders(snippet, 'prop').length >= 1; + }, + ), + { numRuns: 300 }, + ); + }); + + test('a RUNTIME read with no isolation decision nearby is NOT flagged (no false positive)', () => { + const benign = 'RUNTIME=$(gsd_run query config-get runtime --raw)\necho "runtime is $RUNTIME"'; + assert.deepEqual(isolationGateOffenders(benign, 'benign'), []); + }); + + test('no dispatch site gates isolation on a runtime id', () => { + const offenders = []; + for (const file of dispatchSites) { + offenders.push( + ...isolationGateOffenders( + fs.readFileSync(file, 'utf-8'), + path.relative(REPO_ROOT, file).replace(/\\/g, '/'), + ), + ); + } + assert.deepEqual( + offenders, + [], + 'isolation must be resolved from `gsd_run query dispatch-isolation` (see ' + + 'gsd-core/references/dispatch-isolation-gate.md), never from a RUNTIME comparison:\n' + + offenders.join('\n'), + ); + }); + + // #2728 review Blocker — the ISOLATION_TOKEN regex above treats + // `isolation="worktree"` as a legitimate isolation marker, so the runtime-gate + // detector cannot catch a *conditional* keyed on that literal. But the literal + // is Claude Code's own rendering of {harnessFlag}; Cursor's rendering is + // `--worktree`, so any post-dispatch step gated on the literal is a silent + // no-op for a correctly-isolated Cursor run (quick.md's manifest append and + // worktree merge-back were exactly this — isolated work never merged, never + // cleaned up). Post-dispatch bookkeeping must key on the negotiated ISOLATION + // value instead (dispatch-isolation-gate.md's "never hardcode" rule). + const LITERAL_CONDITION = /\bIf\b[^.\n]*`isolation="worktree"`/g; + + function literalConditionOffenders(text, label) { + const hits = []; + let m; + const re = new RegExp(LITERAL_CONDITION.source, 'g'); + while ((m = re.exec(text)) !== null) { + const line = text.slice(0, m.index).split('\n').length; + hits.push(`${label}:${line}: ${m[0].trim()}`); + } + return hits; + } + + test('the literal-condition detector flags the pre-fix quick.md shapes (discrimination proof)', () => { + const preFix = { + 'manifest append': + 'If the executor ran with `isolation="worktree"`, append its returned metadata to `QUICK_WORKTREE_MANIFEST` before cleanup.', + 'worktree cleanup': + '1. **Worktree cleanup:** If the executor ran with `isolation="worktree"`, merge the worktree branch back and clean up:', + }; + for (const [name, snippet] of Object.entries(preFix)) { + assert.equal( + literalConditionOffenders(snippet, 'mutation').length, + 1, + `detector must flag the pre-fix "${name}" conditional — otherwise the guard below proves nothing`, + ); + } + // Explanatory prose that merely *names* the literal (no conditional) stays legal. + assert.deepEqual( + literalConditionOffenders( + 'Claude Code\'s `isolation="worktree"` forks new worktrees from `origin/HEAD`.', + 'benign', + ), + [], + ); + }); + + test('no dispatch site conditions post-dispatch behavior on the Claude-rendered literal', () => { + const offenders = []; + for (const file of dispatchSites) { + offenders.push( + ...literalConditionOffenders( + fs.readFileSync(file, 'utf-8'), + path.relative(REPO_ROOT, file).replace(/\\/g, '/'), + ), + ); + } + assert.deepEqual( + offenders, + [], + 'post-dispatch steps must key on `ISOLATION = "harness-worktree"`, never on ' + + 'Claude Code\'s rendered `isolation="worktree"` literal (a Cursor dispatch renders ' + + '`--worktree` and would silently skip these steps):\n' + offenders.join('\n'), + ); + }); + + test('quick.md post-dispatch bookkeeping keys on the negotiated ISOLATION value', () => { + const quick = fs.readFileSync( + path.join(REPO_ROOT, 'gsd-core', 'workflows', 'quick.md'), 'utf-8', + ); + assert.match( + quick, + /If the executor ran isolated \(`ISOLATION = "harness-worktree"` at dispatch\), append its returned/, + 'the QUICK_WORKTREE_MANIFEST append must be gated on ISOLATION', + ); + assert.match( + quick, + /\*\*Worktree cleanup:\*\* If the executor ran isolated \(`ISOLATION = "harness-worktree"` at dispatch\)/, + 'the worktree merge-back/cleanup must be gated on ISOLATION', + ); + assert.match( + quick, + /If `ISOLATION` was not `"harness-worktree"` at dispatch[^\n]*skip this step/, + 'the cleanup skip clause must mirror the same ISOLATION gate (USE_WORKTREES stays true on an isolated Cursor run)', + ); + }); +}); + +// --------------------------------------------------------------------------- +// #2728 review BLOCKER (B1/B2/B3) — a degrade must re-RECORD, not just reassign. +// +// Every isolation degrade in a dispatch site is decided in SHELL, where +// `routeDispatchIsolation` cannot see it. That resolver persists whatever it +// resolved to the run-scoped sentinel as an unconditional side effect (#3045 +// CORE REDESIGN, hooks/lib/isolation-sentinel.js), so a degrade that only +// reassigns `$ISOLATION` leaves the sentinel asserting `harness-worktree` +// while the dispatch correctly omits the harness flag. The shipped PreToolUse +// guard reads the sentinel at the instant of the `Agent()` call and denies the +// mismatch with exit 2 — the work does not run unisolated, it does not run. +// +// WHY THIS ASSERTS THE RECORDED VALUE, NOT `$ISOLATION`: asserting the local +// variable is precisely what let this class through. `$ISOLATION` was already +// correct at all three sites — `none` — and the defect was entirely in what +// reached the sentinel. So these tests execute each workflow's own degrade +// block under a `gsd_run` stub that captures every call, and assert on the +// value the workflow PUSHED THROUGH THE WRITE PATH. +// --------------------------------------------------------------------------- +describe('#2728 B1 — isolation degrades re-record through the single write path', () => { + const { runHook } = require('./helpers/process-seam.cjs'); + // Class-norm timeout, not a local literal (CONTRIBUTING: class-norm + // timeouts live in tests/helpers/timeouts.cjs). This harness is a short + // shell probe against a gsd_run stub — exactly the PROBE class. + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + const os = require('node:os'); + + const WORKFLOWS = path.join(REPO_ROOT, 'gsd-core', 'workflows'); + + /** + * Pull the fenced ```bash block containing `marker` out of a workflow. + * + * DEFECT.WINDOWS-CRLF-TEST-PORTABILITY: the captured body is handed to + * `bash` below, so it must be CRLF-free. A `\r?\n` fence regex is NOT + * sufficient — it only protects the delimiter match, leaving embedded `\r` + * on every line of the body, which bash treats as part of the token + * (helpers.cjs documents exactly this trap). Normalize at the READ + * boundary so everything downstream is LF-only by construction. + */ + function bashBlockContaining(file, marker) { + const text = readFileNormalized(file); + for (const m of text.matchAll(/```bash\r?\n([\s\S]*?)```/g)) { + if (m[1].includes(marker)) return m[1]; + } + assert.fail(`no \`\`\`bash block containing ${JSON.stringify(marker)} in ${file}`); + } + + /** + * Run a degrade block with the base-check forced to fire, under a `gsd_run` + * stub that logs its argv. Returns every `dispatch-isolation` call the block + * made, in order — i.e. the writes that would have hit the sentinel. + */ + function recordedWrites(block) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-degrade-')); + const log = path.join(dir, 'calls.log'); + // The stub answers the base-check `true` so the degrade path is TAKEN; + // every other query returns empty. It logs the full argv of each call. + const harness = [ + 'set -u', + `gsd_run() { printf '%s\\n' "$*" >> ${JSON.stringify(log)};`, + ' case "$*" in', + ' *"worktree.base-check"*"shouldDegrade"*) printf true ;;', + ' *"worktree.base-check"*"message"*) printf "base diverged" ;;', + ' *) printf "" ;;', + ' esac; }', + 'ISOLATION=harness-worktree', + 'USE_WORKTREES=true', + 'RUNTIME=claude', + 'PHASE_NUMBER=7', + block, + // Prove the local variable was ALSO correct, so a failure below can only + // mean the recording is missing — not that the degrade itself misfired. + 'printf "FINAL_LOCAL=%s\\n" "$ISOLATION"', + ].join('\n'); + + // Through the process seam, never a hand-rolled spawnSync (CONTRIBUTING + // "Spawning a subprocess: use the process seam"). `runHook` documents + // `interpreter: 'bash'` for running a shell script, so the harness is + // written to a file rather than passed as `-c`. Bounded by construction + // (DEFECT.UNBOUNDED-SUBPROCESS): pure shell against a `gsd_run` stub — no + // git, no network, no real CLI — so it completes in milliseconds. + const scriptPath = path.join(dir, 'degrade-harness.sh'); + fs.writeFileSync(scriptPath, harness); + const res = runHook(scriptPath, [], { interpreter: 'bash', timeoutMs: PROBE_TIMEOUT_MS }); + if (res.outcome !== 'exited') { + cleanup(dir); + assert.fail(`degrade block did not complete: outcome=${res.outcome} ${res.stderr || ''}`); + } + assert.equal(res.exitCode, 0, `degrade block exited ${res.exitCode}: ${res.stderr}`); + assert.match(res.stdout, /FINAL_LOCAL=none/, 'the degrade must set $ISOLATION=none locally'); + + const calls = fs.existsSync(log) + ? fs.readFileSync(log, 'utf-8').split(/\r?\n/).filter(Boolean) + : []; + cleanup(dir); + return calls.filter(c => c.includes('dispatch-isolation')); + } + + /** The isolation mode the last write pushed, or null if nothing was written. */ + function recordedIsolation(block) { + const writes = recordedWrites(block); + if (writes.length === 0) return null; + const last = writes[writes.length - 1]; + const m = last.match(/--force-isolation\s+(\S+)/); + return m ? m[1] : null; + } + + const DEGRADE_SITES = [ + { + label: 'quick.md #1941 base-check degrade', + file: path.join(WORKFLOWS, 'quick.md'), + marker: '_QUICK_SHOULD_DEGRADE', + }, + { + label: 'diagnose-issues.md #2649 base-check degrade', + file: path.join(WORKFLOWS, 'diagnose-issues.md'), + marker: '_DIAG_SHOULD_DEGRADE', + }, + ]; + + for (const site of DEGRADE_SITES) { + test(`${site.label} records none, not just the local variable`, () => { + const block = bashBlockContaining(site.file, site.marker); + assert.equal( + recordedIsolation(block), + 'none', + `${site.label}: $ISOLATION degraded to none but the block never pushed that ` + + 'through `query dispatch-isolation --force-isolation`. The sentinel still ' + + 'asserts harness-worktree, so the #3045 PreToolUse guard denies the dispatch ' + + 'with exit 2. Re-record immediately after the degrade.', + ); + }); + } + + test('the harness detects a degrade that only reassigns (fail-first proof)', () => { + // Strip the re-record from the shipped block. If the assertion above can + // still pass against this, it is not testing what it claims to test. + const block = bashBlockContaining( + path.join(WORKFLOWS, 'quick.md'), '_QUICK_SHOULD_DEGRADE', + ); + const preFix = block + .split('\n') + .filter(l => !l.includes('--force-isolation')) + .join('\n'); + + assert.notEqual(preFix, block, 'the shipped block must contain a --force-isolation re-record'); + assert.equal( + recordedIsolation(preFix), + null, + 'the pre-fix shape must record NOTHING — otherwise these tests prove nothing. ' + + 'Note $ISOLATION is `none` in BOTH shapes: that is exactly why asserting the ' + + 'local variable would have passed on the defect.', + ); + }); + + test('every dispatch-site degrade block re-records before the block ends', () => { + // Coverage guard: a NEW degrade site added later cannot silently skip the + // re-record. Scans the shipped shell rather than a hand-listed set. + const offenders = []; + // DERIVED, not hand-listed. The previous revision named three files, so a + // degrade added anywhere else passed a check whose name claims "every" + // (#2728 round-7 review, Major 6). Scan every workflow and reference. + const scan = SCAN_ROOTS.flatMap(collectMarkdown); + assert.ok( + scan.length > 10, + `the degrade scan collected only ${scan.length} files — the walk is broken, not the tree`, + ); + + // Wave sites re-record PER PLAN, not inline: `execute-phase-wave-guard.md` and + // `execute-phase-between-wave-reset.md` degrade `USE_WORKTREES=false` together + // with `ISOLATION=none`, and `per-plan-worktree-gate.md` seeds + // `USE_WORKTREES_FOR_PLAN="$USE_WORKTREES"` and pushes `--force-isolation none` + // before each plan's dispatch. That is a real re-record, just delegated — so + // these two are exempt. The exemption is ASSERTED below rather than assumed, + // so deleting the delegate fails this test instead of silently widening a hole. + const DELEGATED_TO_PER_PLAN_GATE = new Set([ + 'gsd-core/references/execute-phase-wave-guard.md', + 'gsd-core/references/execute-phase-between-wave-reset.md', + ]); + const perPlanGate = readFileNormalized( + path.join(WORKFLOWS, 'execute-phase', 'steps', 'per-plan-worktree-gate.md'), + ); + assert.ok( + perPlanGate.includes('USE_WORKTREES_FOR_PLAN="$USE_WORKTREES"') && + perPlanGate.includes('--force-isolation none'), + 'per-plan-worktree-gate.md no longer inherits USE_WORKTREES and re-records `none`, so the ' + + 'wave degrade sites above are no longer covered by delegation — either restore the ' + + 'delegate or make those sites re-record inline', + ); + + for (const file of scan) { + const rel = path.relative(REPO_ROOT, file).replace(/\\/g, '/'); + if (DELEGATED_TO_PER_PLAN_GATE.has(rel)) continue; + const text = readFileNormalized(file); + for (const m of text.matchAll(/```bash\r?\n([\s\S]*?)```/g)) { + const block = m[1]; + if (!/^\s*ISOLATION=none\s*$/m.test(block)) continue; + if (!block.includes('--force-isolation')) { + const line = text.slice(0, m.index).split(/\r?\n/).length; + offenders.push(`${rel}:${line}`); + } + } + } + assert.deepEqual( + offenders, + [], + 'these shell blocks degrade $ISOLATION to none without re-recording it through ' + + '`query dispatch-isolation --force-isolation` — the #3045 guard will deny the ' + + 'resulting dispatch:\n' + offenders.join('\n'), + ); + }); +}); diff --git a/tests/install.test.cjs b/tests/install.test.cjs index f6f2b891d..e0407ed30 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -6924,7 +6924,15 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, installSpawnEnv } = require('./helpers.cjs'); +// #2652 round-7: new subprocesses go through the process seam, never a +// hand-rolled spawnSync (CONTRIBUTING). The pre-existing `installAndRead` +// spawn below is byte-identical to the base and left alone as out of scope. +const { runNode: seamRunNode, runHook: seamRunHook } = require('./helpers/process-seam.cjs'); +// Class-norm timeouts, not local literals (CONTRIBUTING: they live in +// tests/helpers/timeouts.cjs). The install is the INSTALL class; the emitted +// gate is a short CLI probe against a temp fixture, i.e. the PROBE class. +const { INSTALL_TIMEOUT_MS, PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const INSTALL = path.join(__dirname, '..', 'bin', 'install.js'); @@ -6958,15 +6966,31 @@ function installAndRead(runtime) { // RED tests: these MUST FAIL before the copyWithPathReplacement wiring is added // --------------------------------------------------------------------------- -test('real install: codex-emitted execute-phase.md resolves runtime=codex and defaults worktrees off (#1521)', () => { +test('real install: codex-emitted execute-phase.md resolves runtime=codex and leaves worktrees to the isolation negotiation (#1521, #2652)', () => { + // End-to-end counterpart of the unit coverage in + // tests/runtime-converters.test.cjs. #1521 asserted `--default false` here + // because worktree isolation *was* Claude Code's isolation="worktree" spawn + // parameter and no other host honored it. #2584 replaced that premise with + // the negotiated `dispatch.isolation` capability, and codex declares + // `orchestrator-worktree` — GSD creates the worktree and spawns + // `codex exec --cd `. Keeping the stamp would resolve + // USE_WORKTREES=false before `gsd_run query dispatch-isolation` was ever + // consulted, re-deciding isolation by runtime name (#2652's Blocker). + // + // The runtime-identity stamp is unaffected and still asserted below — only + // the use_worktrees half moved. const c = installAndRead('codex'); assert.ok( c.includes('config-get runtime --default codex --raw'), 'codex runtime default not stamped in real install', ); assert.ok( - c.includes('config-get workflow.use_worktrees --default false --raw'), - 'codex use_worktrees not defaulted false in real install', + !c.includes('config-get workflow.use_worktrees --default false --raw'), + 'codex install stamped use_worktrees=false, which pre-empts the dispatch.isolation negotiation for a host that declares orchestrator-worktree (#2652)', + ); + assert.ok( + c.includes('config-get workflow.use_worktrees --raw 2>/dev/null || echo "true"'), + 'codex install lost the unstamped use_worktrees read — the isolation gate has nothing left to negotiate', ); assert.ok( !c.includes('config-get runtime --default claude --raw'), @@ -6974,6 +6998,23 @@ test('real install: codex-emitted execute-phase.md resolves runtime=codex and de ); }); +test('real install: an isolation-none runtime still gets use_worktrees stamped false (#1521 preserved, #2652 scoped)', () => { + // The other arm: #2652 narrowed the stamp, it did not remove it. windsurf + // declares `dispatch.isolation: none`, so the false default it writes is the + // outcome the resolver reaches anyway, and #1521's protection stays intact + // for every host that genuinely cannot isolate. Without this arm the change + // above could silently become "never stamp" and nothing would notice. + const c = installAndRead('windsurf'); + assert.ok( + c.includes('config-get workflow.use_worktrees --default false --raw'), + 'windsurf declares isolation=none and must still receive the #1521 false stamp', + ); + assert.ok( + c.includes('config-get runtime --default windsurf --raw'), + 'windsurf runtime default not stamped in real install', + ); +}); + test('real install: cursor-emitted execute-phase.md resolves runtime=cursor (#1521)', () => { const c = installAndRead('cursor'); assert.ok( @@ -6986,6 +7027,203 @@ test('real install: cursor-emitted execute-phase.md resolves runtime=cursor (#15 ); }); +// --------------------------------------------------------------------------- +// #2652 review round-5/6 Major 2 — Cursor end-to-end, not by reasoning. +// +// Cursor is the runtime this PR newly enables: it declares +// `dispatch.isolation: harness-worktree` with `harnessIsolationFlag: "--worktree"`, +// and narrowing the #1521 `use_worktrees=false` stamp is what lets its +// negotiation be reached at all. The resolver agreeing with itself is not +// evidence that `--worktree` — rather than Claude Code's own +// `isolation="worktree"` literal, or nothing at all — is what ends up in the +// `{harnessFlag}` slot of the `Agent()` call. +// +// So: run a REAL cursor install, then execute the gate blocks THAT INSTALL +// EMITTED against the gsd-tools.cjs THAT INSTALL EMITTED, and read the two +// shell variables the dispatch sites substitute. Nothing here is a fixture. +// --------------------------------------------------------------------------- + +// 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'; + +test('real install: cursor negotiates --worktree through its own emitted gate and it lands in the emitted Agent() slot (#2652)', { skip: NO_BASH }, (t) => { + const { readFileNormalized } = require('./helpers.cjs'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-inst-cursor-gate-')); + t.after(() => cleanup(dir)); + // Through the process seam and the install isolation seam, never a + // hand-rolled spawnSync from raw process.env (CONTRIBUTING "Spawning a + // subprocess: use the process seam"): ambient GSD_HOME / runtime-location + // vars otherwise leak in and make capability discovery host-dependent. + const res = seamRunNode( + [INSTALL, '--cursor', '--global', '--config-dir', dir], + { env: installSpawnEnv({ HOME: dir, USERPROFILE: dir }), timeoutMs: INSTALL_TIMEOUT_MS }, + ); + assert.strictEqual(res.outcome, 'exited', `install --cursor did not complete: ${res.outcome}`); + assert.strictEqual(res.exitCode, 0, `install --cursor failed: ${res.stderr || res.stdout}`); + + const tools = path.join(dir, 'gsd-core', 'bin', 'gsd-tools.cjs'); + const gate = path.join(dir, 'gsd-core', 'references', 'dispatch-isolation-gate.md'); + assert.ok(fs.existsSync(tools), `cursor install emitted no gsd-tools.cjs at ${tools}`); + assert.ok(fs.existsSync(gate), `cursor install emitted no dispatch-isolation-gate.md at ${gate}`); + + // Precondition, stated as an assertion rather than assumed: the emitted + // quick.md must still carry the UNSTAMPED use_worktrees read. If #1521's + // `--default false` stamp came back for cursor, USE_WORKTREES resolves + // false and the gate below degrades to none before negotiating anything — + // the harness flag would then be empty for a reason unrelated to the flag. + const quick = readFileNormalized(path.join(dir, 'gsd-core', 'workflows', 'quick.md')); + assert.ok( + quick.includes('config-get workflow.use_worktrees --raw 2>/dev/null || echo "true"'), + 'cursor-emitted quick.md lost the unstamped use_worktrees read — the isolation negotiation is pre-empted at install time (#2652)', + ); + + // Pull the gate's OWN blocks — the ones a dispatch site is told to run — + // out of the emitted reference. readFileNormalized strips CRLF first + // (DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE). + // Anchor on the gate's own HEADINGS, not on an assignment literal inside a + // block. The workflows tell a dispatch site to run the `Resolve ISOLATION` + // and `Resolve the harness flag` blocks BY NAME, so the heading is the + // contract and the block body is free to change under it. Keying off the + // body instead is what broke here: when the resolver grew its + // `_ISOLATION_RAW`/`ISOLATION_RESOLVED` split — so a shim failure stops + // masquerading as a declared `none` — the old `ISOLATION=$(gsd_run query + // dispatch-isolation --raw` finder stopped matching, and a test whose + // subject is "does the emitted gate resolve cursor correctly" failed as + // "there is no such block". + const gateText = readFileNormalized(gate); + const blockUnder = (heading) => { + const at = gateText.indexOf(`## ${heading}`); + if (at === -1) return undefined; + return (gateText.slice(at).match(/```bash\r?\n([\s\S]*?)```/) || [])[1]; + }; + const resolveBlock = blockUnder('Resolve ISOLATION'); + const flagBlock = blockUnder('Resolve the harness flag'); + assert.ok(resolveBlock, 'emitted dispatch-isolation-gate.md has no `## Resolve ISOLATION` heading with a bash block under it'); + assert.ok(flagBlock, 'emitted dispatch-isolation-gate.md has no `## Resolve the harness flag` heading with a bash block under it'); + + // A disposable project dir: `query dispatch-isolation` writes the #3045 + // sentinel into its cwd as an unconditional side effect. + const proj = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cursor-gate-proj-')); + t.after(() => cleanup(proj)); + // Declare the runtime the way a real Cursor project does — through + // `.planning/config.json`, which is the tier `resolveRuntime` actually + // reads (GSD_RUNTIME > .planning/config.json > 'claude'). Injecting + // GSD_RUNTIME=cursor instead would prove the resolver works when handed + // the answer, not that a Cursor install reaches it on its own. (The + // install's ~/.gsd/defaults.json `runtime` is NOT a resolveRuntime tier.) + fs.mkdirSync(path.join(proj, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(proj, '.planning', 'config.json'), JSON.stringify({ runtime: 'cursor' })); + + const script = [ + 'set -u', + `gsd_run() { node ${JSON.stringify(tools)} "$@"; }`, + 'USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true")', + 'RUNTIME=$(gsd_run query config-get runtime --default cursor --raw 2>/dev/null || echo "cursor")', + resolveBlock, + flagBlock, + 'printf "ISOLATION=%s\\nHARNESS_FLAG=%s\\n" "$ISOLATION" "$HARNESS_FLAG"', + ].join('\n'); + + // Bounded (DEFECT.UNBOUNDED-SUBPROCESS): a handful of local gsd-tools + // invocations, no network and no git beyond repo introspection. + // GSD_RUNTIME outranks the config tier in resolveRuntime, so an ambient + // one in the developer's shell would satisfy this test without the + // install's own config ever being read. Blank it explicitly. + const hermeticEnv = { ...process.env, HOME: dir, USERPROFILE: dir }; + delete hermeticEnv.GSD_RUNTIME; + + // Seam again — `runHook` documents `interpreter: 'bash'` for a shell + // script, so the gate is written to a file rather than passed as `-c`. + const gateScript = path.join(proj, 'run-gate.sh'); + fs.writeFileSync(gateScript, script); + const run = seamRunHook(gateScript, [], { + interpreter: 'bash', + cwd: proj, + env: hermeticEnv, + timeoutMs: PROBE_TIMEOUT_MS, + }); + assert.strictEqual( + run.outcome, 'exited', + `emitted cursor gate did not complete: outcome=${run.outcome} ${run.stderr || ''}`, + ); + assert.strictEqual(run.exitCode, 0, `emitted cursor gate exited ${run.exitCode}: ${run.stderr}`); + + assert.match( + run.stdout, + /^ISOLATION=harness-worktree$/m, + `cursor declares dispatch.isolation=harness-worktree but its own emitted gate resolved otherwise:\n${run.stdout}\n${run.stderr}`, + ); + const flagLine = run.stdout.match(/^HARNESS_FLAG=(.*)$/m); + assert.ok(flagLine, `the gate printed no HARNESS_FLAG line:\n${run.stdout}\n${run.stderr}`); + const harnessFlag = flagLine[1]; + assert.strictEqual( + harnessFlag, + '--worktree', + `cursor resolved "${harnessFlag}" instead of its declared --worktree. Claude Code's own isolation="worktree" literal reaching a Cursor dispatch is the #2652 defect inverted.`, + ); + // The reviewer framed this as argv-injection-shaped: the value is spliced + // into an argument list. Pin that it stays a single bare token. + assert.ok( + /^[-\w=".]+$/.test(harnessFlag), + `the harness flag carries shell/argv-significant characters and is spliced into an argument slot: ${JSON.stringify(harnessFlag)}`, + ); + + // ── The slot itself ────────────────────────────────────────────────── + // Resolving the value is only half of it. Perform the substitution the + // emitted workflows document ("{harnessFlag}" → "$HARNESS_FLAG" plus a + // comma when ISOLATION = harness-worktree) against the Agent() call THIS + // INSTALL EMITTED, and assert on the rendered dispatch. Without this, + // deleting the placeholder from the emitted Agent() — the exact way the + // flag would stop reaching the dispatch — leaves the test green. + // Name the dispatch each site's isolated agent actually goes through, so + // "some Agent() call in the file has the placeholder" cannot stand in for + // "the EXECUTOR/DEBUGGER dispatch has it". Both files contain other + // Agent() calls (quick.md dispatches a reviewer too); the placeholder + // drifting onto one of those is a real defect that a `.find()` alone + // would report as a pass. + for (const { wf, agent } of [ + { wf: 'quick.md', agent: 'gsd-executor' }, + { wf: 'diagnose-issues.md', agent: 'gsd-debugger' }, + ]) { + const text = readFileNormalized(path.join(dir, 'gsd-core', 'workflows', wf)); + const calls = text.match(/Agent\(\n(?:[^\n]*\n)*?\)/g) || []; + const withSlot = calls.filter(b => b.includes('{harnessFlag}')); + assert.strictEqual( + withSlot.length, + 1, + `${wf}: expected exactly one emitted Agent() call carrying {harnessFlag}, found ${withSlot.length}. ` + + 'Zero means the negotiated flag has no slot to reach; more than one means the isolated ' + + `dispatch is ambiguous.\n${withSlot.join('\n---\n')}`, + ); + const call = withSlot[0]; + assert.ok( + call.includes(`subagent_type="${agent}"`), + `${wf}: the {harnessFlag} slot is not on the ${agent} dispatch — it drifted onto a different Agent() call, so the isolated agent would be spawned without it:\n${call}`, + ); + assert.strictEqual( + (call.match(/\{harnessFlag\}/g) || []).length, + 1, + `${wf}: expected exactly one {harnessFlag} slot in the dispatch call, got:\n${call}`, + ); + + const rendered = call.replace('{harnessFlag}', `${harnessFlag},`); + assert.match( + rendered, + /^\s*--worktree,$/m, + `${wf}: rendering the emitted Agent() call for cursor did not put --worktree in its argument list:\n${rendered}`, + ); + assert.ok( + !rendered.includes('{harnessFlag}'), + `${wf}: an unsubstituted {harnessFlag} survives into the dispatch:\n${rendered}`, + ); + assert.ok( + !rendered.includes('isolation="worktree"'), + `${wf}: the rendered cursor dispatch carries Claude Code's own harness literal — the flag is descriptor data, never hardcoded:\n${rendered}`, + ); + } +}); + test('real install: claude-emitted execute-phase.md keeps claude default + worktrees on (#1521)', () => { const c = installAndRead('claude'); assert.ok( diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index c785489f5..ee60a43b1 100644 --- a/tests/no-bare-gsd-tools-command-position.test.cjs +++ b/tests/no-bare-gsd-tools-command-position.test.cjs @@ -86,7 +86,7 @@ const PROSE_ALLOWLIST = [ { file: 'agents/gsd-phase-researcher.md', line: 33, reason: 'package-legitimacy provenance rule names the command as the source of an OK verdict; descriptive' }, { file: 'agents/gsd-roadmapper.md', line: 642, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, { file: 'agents/gsd-intel-updater.md', line: 40, reason: 'cross-platform note names the `gsd-tools intel ` CLI surface descriptively ("CLI invocations go through..."); not an agent instruction' }, - { file: 'gsd-core/workflows/execute-plan.md', line: 387, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, + { file: 'gsd-core/workflows/execute-plan.md', line: 414, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, { file: 'agents/gsd-research-synthesizer.md', line: 65, reason: 'a code comment inside a fenced block explaining what the commit step loads (`# Planning config loaded via gsd-tools query ...`); descriptive, not an invocation — and explicitly names gsd-tools.cjs as the alternative' }, ]; diff --git a/tests/runtime-converters.test.cjs b/tests/runtime-converters.test.cjs index 17d012130..79d2c7986 100644 --- a/tests/runtime-converters.test.cjs +++ b/tests/runtime-converters.test.cjs @@ -934,21 +934,30 @@ test('codex emit stamps its own runtime default into the runtime-resolution line ); }); -test('codex emit defaults workflow.use_worktrees to false', () => { +test('codex emit leaves workflow.use_worktrees at the true default — isolation is negotiated, not stamped (#1515 premise superseded by #2584/#2652)', () => { + // #1515 stamped `--default false` here because worktree isolation *was* + // Claude Code's isolation="worktree" spawn parameter, so a Codex install that + // resolved use_worktrees=true would have run unisolated while believing it was + // isolated. #2584 removed that premise: Codex declares + // `dispatch.isolation: orchestrator-worktree`, meaning GSD creates the + // worktree itself and spawns `codex exec --cd ` — supported, not + // unsafe. Keeping the stamp would resolve USE_WORKTREES=false before + // dispatch-isolation is consulted, re-deciding isolation by runtime name, + // which is the exact defect #2652 removes. The safety property #1515 protects + // is now held by the isolation gate's fail-closed resolution, not by a + // name-scoped install-time default. const line = 'USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true")\n'; const out = conversion._applyRuntimeRewrites(line, 'codex', '$HOME/.codex/', true, undefined); - assert.ok( - out.includes('config-get workflow.use_worktrees --default false --raw'), - `Expected 'config-get workflow.use_worktrees --default false --raw' in output; got:\n${out}`, + assert.strictEqual( + conversion._negotiatedDispatchIsolation('codex'), + 'orchestrator-worktree', + 'codex must declare orchestrator-worktree for this expectation to hold', ); - assert.ok( - out.includes('|| echo "false")'), - `Expected '|| echo "false")' in output; got:\n${out}`, - ); - assert.ok( - !out.includes('|| echo "true")'), - `Expected '|| echo "true")' to be fully rewritten; got:\n${out}`, + assert.strictEqual( + out, + line, + `Expected the use_worktrees read to survive codex emit untouched; got:\n${out}`, ); }); @@ -986,12 +995,17 @@ test('regression: every edited workflow gets codex-stamped (source↔engine pari for (const wf of WORKFLOWS) { const src = fs.readFileSync(path.join(__dirname, '..', 'gsd-core', 'workflows', wf), 'utf8'); const out = conversion._applyRuntimeRewrites(src, 'codex', '$HOME/.codex/', true, undefined); - // No un-stamped claude/true resolution line may survive codex emit on ANY surface. + // No un-stamped claude runtime line may survive codex emit on ANY surface. assert.ok(!out.includes(CLAUDE_RUNTIME), `${wf}: residual un-stamped runtime read — engine regex no longer matches source line (parity drift)`); - assert.ok(!out.includes(TRUE_WT), `${wf}: residual un-stamped use_worktrees read — parity drift`); // If the source HAS such a read, the codex form must be present. if (src.includes(CLAUDE_RUNTIME)) assert.ok(out.includes(CODEX_RUNTIME), `${wf}: runtime read not stamped to codex`); - if (src.includes(TRUE_WT)) assert.ok(out.includes(FALSE_WT), `${wf}: use_worktrees read not defaulted to false`); + // #2652: codex declares orchestrator-worktree, so the use_worktrees read is + // left for `gsd_run query dispatch-isolation` to decide at run time — the + // install-time `--default false` stamp would pre-empt that negotiation. + if (src.includes(TRUE_WT)) { + assert.ok(out.includes(TRUE_WT), `${wf}: use_worktrees read was stamped away for a runtime that negotiates worktree isolation (#2652)`); + assert.ok(!out.includes(FALSE_WT), `${wf}: codex gained the --default false use_worktrees stamp (#2652)`); + } } }); @@ -1036,11 +1050,16 @@ test('property: codex stamping is idempotent on resolution lines (#1515)', () => 'use strict'; /** * Regression tests for #1521: every non-Claude runtime stamps its own runtime - * identity + workflow.use_worktrees=false into emitted workflows. + * identity into emitted workflows, and stamps workflow.use_worktrees=false + * where its negotiated dispatch.isolation is `none`. * - * GSD's worktree isolation relies on Claude Code's isolation="worktree" spawn - * parameter, which no other runtime honors. #1519 (Codex-only fix) is - * generalized here to ALL non-Claude runtimes. + * #1519 (Codex-only fix) was generalized here to ALL non-Claude runtimes on the + * premise that GSD's worktree isolation was Claude Code's isolation="worktree" + * spawn parameter, which no other runtime honored. #2584 replaced that premise + * with the negotiated `dispatch.isolation` capability, and #2652 scoped the + * use_worktrees stamp to match: a host declaring harness-worktree or + * orchestrator-worktree keeps the `true` default, because stamping it false + * pre-empts the negotiation and re-decides isolation by runtime name. * * All tests assert on the SUT's RETURN VALUE (engine output), not raw file reads, * except the parity integration test which carries the allow-test-rule exemption. @@ -1068,13 +1087,14 @@ const FALSE_WT_LINE = 'config-get workflow.use_worktrees --default false --raw 2 // Parity across ALL non-Claude runtimes × all 5 workflows // --------------------------------------------------------------------------- -test('parity: every non-Claude runtime stamps its own runtime default and use_worktrees=false on all workflows (#1521)', () => { +test('parity: every non-Claude runtime stamps its own runtime default, and use_worktrees=false only where isolation negotiates to none (#1521, #2652)', () => { // allow-test-rule: pending-migration-to-typed-ir [#3090] // `out` is _applyRuntimeRewrites's engine-transformed shell text, not shipped // source — substring-matching it is the "Rendered file" row CONTRIBUTING // requires a typed-IR builder for. No such IR exists yet for the shell // rewrite output; production change out of scope here. Tracked under #3090. for (const rt of NON_CLAUDE) { + const isolation = conversion._negotiatedDispatchIsolation(rt); for (const wf of WORKFLOWS) { const src = fs.readFileSync( path.join(__dirname, '..', 'gsd-core', 'workflows', wf), @@ -1088,12 +1108,6 @@ test('parity: every non-Claude runtime stamps its own runtime default and use_wo `${rt}/${wf}: residual un-stamped claude runtime read — _stampNonClaudeRuntimeDefaults not applied`, ); - // No un-stamped use_worktrees=true line may survive - assert.ok( - !out.includes(TRUE_WT_LINE), - `${rt}/${wf}: residual un-stamped use_worktrees=true read — _stampNonClaudeRuntimeDefaults not applied`, - ); - // If the source had a runtime read, the output must have --default if (src.includes(CLAUDE_RUNTIME_LINE)) { assert.ok( @@ -1102,17 +1116,215 @@ test('parity: every non-Claude runtime stamps its own runtime default and use_wo ); } - // If the source had a use_worktrees read, the output must have --default false - if (src.includes(TRUE_WT_LINE)) { + if (!src.includes(TRUE_WT_LINE)) continue; + + // #2652: the use_worktrees=false stamp is scoped to the runtimes whose + // negotiated dispatch.isolation really is `none`. A host that declares + // harness-worktree (cursor) or orchestrator-worktree (codex, opencode, + // kimi, kimi-code) must keep the unstamped `true` default, or the stamp + // resolves USE_WORKTREES=false before dispatch-isolation is consulted and + // the runtime is judged by its name after all. + if (isolation === 'none') { + assert.ok( + !out.includes(TRUE_WT_LINE), + `${rt}/${wf}: residual un-stamped use_worktrees=true read — _stampNonClaudeRuntimeDefaults not applied`, + ); assert.ok( out.includes(FALSE_WT_LINE), `${rt}/${wf}: use_worktrees line not defaulted to false`, ); + } else { + assert.ok( + out.includes(TRUE_WT_LINE), + `${rt}/${wf}: declares dispatch.isolation=${isolation} but the use_worktrees read was stamped away — the install-time default pre-empts the negotiated capability (#2652)`, + ); + assert.ok( + !out.includes(FALSE_WT_LINE), + `${rt}/${wf}: declares dispatch.isolation=${isolation} but gained the --default false stamp`, + ); } } } }); +// --------------------------------------------------------------------------- +// #2652: the stamp's scope is the negotiated capability, not the runtime name +// --------------------------------------------------------------------------- + +test('regression #2652: _stampNonClaudeRuntimeDefaults leaves use_worktrees alone for every runtime that negotiates a worktree isolation', () => { + const line = + 'USE_WORKTREES=$(gsd_run query config-get workflow.use_worktrees --raw 2>/dev/null || echo "true")\n'; + + // The set is derived from the registry, not hand-listed, so a newly declared + // worktree host is covered the moment it lands — the same reason + // NON_CLAUDE_RUNTIMES is derived rather than literal (#1521). + const declaresWorktree = NON_CLAUDE.filter( + (rt) => conversion._negotiatedDispatchIsolation(rt) !== 'none', + ); + const declaresNone = NON_CLAUDE.filter( + (rt) => conversion._negotiatedDispatchIsolation(rt) === 'none', + ); + + // Both arms must be non-empty or the test proves nothing about either. + assert.ok( + declaresWorktree.length > 0, + 'no non-Claude runtime negotiates a worktree isolation — the regression this pins is unreachable', + ); + assert.ok( + declaresNone.length > 0, + 'every non-Claude runtime negotiates a worktree isolation — the false stamp is dead code', + ); + + for (const rt of declaresWorktree) { + assert.strictEqual( + conversion._stampNonClaudeRuntimeDefaults(line, rt), + line, + `${rt}: negotiates ${conversion._negotiatedDispatchIsolation(rt)} but its use_worktrees default was stamped false at install time, so dispatch-isolation-gate.md resolves ISOLATION=none regardless of what it declared (#2652)`, + ); + } + + for (const rt of declaresNone) { + assert.ok( + conversion._stampNonClaudeRuntimeDefaults(line, rt).includes(FALSE_WT_LINE), + `${rt}: negotiates none, so the false default must still be stamped (#1521)`, + ); + } +}); + +test('regression #2652: _negotiatedDispatchIsolation fails closed on an undeclared or unknown runtime', () => { + // Mirrors routeDispatchIsolation's fail-closed contract (ADR-1239): anything + // outside the closed vocabulary resolves to `none`, so the #1521 stamp — and + // the workflows' isolation gate — degrade to sequential rather than to an + // unisolated dispatch that believes it is isolated. + for (const unknown of ['not-a-runtime', '', 'CLAUDE', '__proto__']) { + assert.strictEqual( + conversion._negotiatedDispatchIsolation(unknown), + 'none', + `${JSON.stringify(unknown)}: expected fail-closed 'none'`, + ); + } + + // A registry-known host whose declared value is the `undocumented` sentinel + // (or absent) is out of vocabulary and must degrade the same way. + const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); + const undocumented = NON_CLAUDE.filter((rt) => { + const declared = registry?.runtimes?.[rt]?.runtime?.hostIntegration?.dispatch?.isolation; + return declared === 'undocumented' || declared == null; + }); + assert.ok(undocumented.length > 0, 'no runtime declares the undocumented sentinel — nothing to pin'); + for (const rt of undocumented) { + assert.strictEqual(conversion._negotiatedDispatchIsolation(rt), 'none', `${rt}: undocumented must degrade to none`); + } +}); + +// --------------------------------------------------------------------------- +// #2652 review round-5/6 Major 1 — PARITY with the runtime resolver. +// +// `_negotiatedDispatchIsolation` (install time, this module) and +// `routeDispatchIsolation` (dispatch time, gsd-core/bin/gsd-tools.cjs) are two +// implementations of ONE rule: what isolation may this host negotiate. They +// read the same inputs — the capability registry and `resolveOrchestratorExec` +// — but duplicate the DECISION on top of them, on two surfaces (the install +// engine and the CLI query hub) with no call edge between them. So neither +// symbol appears in the other's impact graph and no static analysis sees the +// duplication. Divergence would be silent, and its consequence is the #2652 +// defect returning by the back door: the installer stamping +// `use_worktrees=false` for a host the resolver would have granted a worktree +// (or the reverse — an unstamped host whose dispatch then refuses). +// +// The assertion is therefore behavioral on BOTH sides: the resolver leg drives +// the REAL CLI and reads its actual stdout. +// --------------------------------------------------------------------------- + +test('regression #2652: _negotiatedDispatchIsolation agrees with the routeDispatchIsolation CLI for every registered runtime', () => { + const { runGsdTools, createTempProject, cleanup: cleanupDir } = require('./helpers.cjs'); + const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); + const RUNTIME_IDS = Object.keys(registry.runtimes).sort(); + + // Install time cannot know the worktree a future dispatch will target, so + // `_negotiatedDispatchIsolation` probes the orchestrator descriptor with a + // placeholder. The CLI is asked BOTH ways, because the two calls are both real: + // + // --cwd-target the executor-spawn call, which resolves the descriptor. + // Same question install time asks; must match exactly. + // (no --cwd-target) the FIRST call every dispatch site makes — the + // `Resolve ISOLATION` block in dispatch-isolation-gate.md. + // It skips descriptor resolution, so it can only differ + // from install time for an orchestrator host whose + // descriptor does not resolve. No such host exists today + // and that is worth pinning: if one lands, the install + // stamps `use_worktrees=false` while the workflow gate + // still reports `orchestrator-worktree` — the #2652 split + // brain, in the one shape the target-bound leg cannot see. + const PROBE_TARGET = '/gsd-orchestrator-worktree-probe'; + + const dir = createTempProject('gsd-2652-parity-'); + try { + const disagreements = []; + const seen = new Set(); + for (const rt of RUNTIME_IDS) { + const res = runGsdTools( + ['query', 'dispatch-isolation', '--json', '--cwd-target', PROBE_TARGET], + dir, + { GSD_RUNTIME: rt, HOME: dir }, + ); + assert.equal(res.success, true, `${rt}: dispatch-isolation query failed: ${res.error}`); + const parsed = JSON.parse(res.output); + + // Guard the guard: if resolveRuntime normalized `rt` to some other id, the + // comparison below would be pinning the wrong host and silently pass. + assert.equal( + parsed.runtime, + rt, + `${rt}: the CLI resolved runtime "${parsed.runtime}" instead — parity would be measured against the wrong host`, + ); + + const gateRes = runGsdTools( + ['query', 'dispatch-isolation', '--raw'], + dir, + { GSD_RUNTIME: rt, HOME: dir }, + ); + assert.equal(gateRes.success, true, `${rt}: no-target dispatch-isolation query failed: ${gateRes.error}`); + const gateIsolation = gateRes.output.trim(); + + const installTime = conversion._negotiatedDispatchIsolation(rt); + seen.add(installTime); + if (installTime !== parsed.isolation) { + disagreements.push( + `${rt}: install-time _negotiatedDispatchIsolation → "${installTime}", ` + + `dispatch-time routeDispatchIsolation (--cwd-target) → "${parsed.isolation}"`, + ); + } + if (installTime !== gateIsolation) { + disagreements.push( + `${rt}: install-time _negotiatedDispatchIsolation → "${installTime}", ` + + `workflow-gate routeDispatchIsolation (no --cwd-target) → "${gateIsolation}"`, + ); + } + } + + assert.deepEqual( + disagreements, + [], + 'the install-time and dispatch-time isolation resolvers disagree. One of them was ' + + 'changed without the other (DEFECT.GENERATIVE-FIX-DIVERGENCE): the installer and the ' + + 'dispatch gate would then make opposite isolation decisions for the same host.\n' + + disagreements.join('\n'), + ); + + // A parity check over a set that only ever answers `none` proves nothing — + // both legs could be stubbed to a constant and still agree. + for (const mode of ['harness-worktree', 'orchestrator-worktree', 'none']) { + assert.ok( + seen.has(mode), + `no registered runtime resolves to "${mode}" — the parity above never exercised that branch`, + ); + } + } finally { + cleanupDir(dir); + } +}); + // --------------------------------------------------------------------------- // Claude unchanged — no stamping for the native runtime // --------------------------------------------------------------------------- @@ -1196,21 +1408,38 @@ test('execute-phase.md, quick.md, and diagnose-issues.md guards are generalized // #2584 Phase 3 (#2627): execute-phase.md graduated PAST the `!= "claude"` // guard — worktree isolation there is now keyed on the negotiated // `dispatch.isolation` capability, so no runtime name appears in its guard at - // all. quick.md and diagnose-issues.md still use the #1521 generalized form - // (they do not negotiate isolation), so they keep asserting it. - const GUARD_WORKFLOWS = ['quick.md', 'diagnose-issues.md']; + // all. + // + // #2652: quick.md and diagnose-issues.md have now graduated too. This block + // previously asserted they still carried the #1521 generalized form, with a + // comment noting "they do not negotiate isolation" — i.e. the test knowingly + // pinned the un-migrated state #2652 was filed about. They negotiate now, so + // they are held to the same capability-keyed contract as execute-phase.md. + const GUARD_WORKFLOWS = ['quick.md', 'diagnose-issues.md', 'execute-phase.md']; for (const wf of GUARD_WORKFLOWS) { const src = fs.readFileSync( path.join(__dirname, '..', 'gsd-core', 'workflows', wf), 'utf8', ); assert.ok( - src.includes('[ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]'), - `${wf}: expected generalized guard [ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]`, + !src.includes('[ "$RUNTIME" != "claude" ] && [ "$USE_WORKTREES" != "false" ]'), + `${wf}: worktree isolation must branch on dispatch.isolation, not on != "claude" (#2584/#2652)`, ); assert.ok( !src.includes('[ "$RUNTIME" = "codex" ] && [ "$USE_WORKTREES" != "false" ]'), - `${wf}: found Codex-specific guard — should have been generalized to != "claude"`, + `${wf}: found Codex-specific guard — isolation is a negotiated capability (#2584/#2652)`, + ); + } + + // The two migrated sites must actually negotiate, not merely drop the guard. + for (const wf of ['quick.md', 'diagnose-issues.md']) { + const src = fs.readFileSync( + path.join(__dirname, '..', 'gsd-core', 'workflows', wf), + 'utf8', + ); + assert.ok( + src.includes('query dispatch-isolation'), + `${wf}: must resolve ISOLATION via \`gsd_run query dispatch-isolation\` (#2652)`, ); }