diff --git a/.changeset/bold-wasps-hop.md b/.changeset/bold-wasps-hop.md new file mode 100644 index 000000000..6cbb58396 --- /dev/null +++ b/.changeset/bold-wasps-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3478 +--- +Executor dispatch prompts now state checkpoint gate semantics: gate="blocking" (the default) is auto-approvable in auto-mode, only gate="blocking-human" always surfaces to a human. The phase-level and single-plan-level orchestrators no longer leave room to compose dispatch text that refuses auto-approval, which stalled autonomous runs at ordinary blocking checkpoints. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index d82a5864c..4630fb6ef 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -655,7 +655,7 @@ increases monotonically across waves. `{status}` is `complete` (success), Pass paths only — executors read files themselves. - **Executor routing (#1689).** Per plan, run `gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md` to set `EXECUTOR_TYPE` for `subagent_type="{EXECUTOR_TYPE}"` below. + **Executor routing (#1689/#3370).** Per plan, run `gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md` to set `EXECUTOR_TYPE` for `subagent_type="{EXECUTOR_TYPE}"` below. **Worktree mode** (`USE_WORKTREES` and `USE_WORKTREES_FOR_PLAN` not `false`): 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 5c714d505..1ebe03b9b 100644 --- a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md +++ b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md @@ -130,7 +130,7 @@ Run the loop below once per runnable plan in the wave, **one plan at a time** (` **Before running the bash block, substitute the plan's identifiers into it** exactly as you do for the `Agent()` prompt on the harness path: replace `{plan_number}` and `{phase_number}` with this plan's values. They are template placeholders, not shell variables. `$ORCH_ROOT` and `$EXPECTED_BASE` are real shell variables, already assigned earlier in this step; `$WAVE_WORKTREE_MANIFEST` was initialized above. -First build the executor prompt. It is the **same prompt text the harness path's `Agent()` call uses**, with the harness-only framing removed — drop the `` build-time embed note and the `` harness block, keep ``, the execution context, and `` verbatim. Assign it to a shell variable so it can be passed as one argument: +First build the executor prompt. It is the **same prompt text the harness path's `Agent()` call uses**, with the harness-only framing removed — drop the `` build-time embed note and the `` harness block, keep ``, the execution context, and `` verbatim. The checkpoint gate rule (#3370, in `per-plan-executor-routing.md`) applies here too: add no prompt text refusing or overriding auto-approval for the default `gate="blocking"` — only `blocking-human` always surfaces. Assign it to a shell variable so it can be passed as one argument: ```bash # Compose the executor prompt for THIS plan. Single-quoted multi-line diff --git a/gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md b/gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md index 9fcbb7c9f..6253463a2 100644 --- a/gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md +++ b/gsd-core/workflows/execute-phase/steps/per-plan-executor-routing.md @@ -52,3 +52,22 @@ Routing applies to the `Agent()`-based dispatch (harness-worktree and sequential modes). The `orchestrator-worktree` isolation backend spawns executors via a separate process path that has no `subagent_type` and is not routed in this release. + +## Checkpoint gate rule (#3370) + +Loaded with the routing resolution so the orchestrator reads it immediately +before composing each dispatch prompt, in every isolation mode. + +On `checkpoint:human-verify` / `checkpoint:decision` tasks, `gate="blocking"` +(the default) is auto-approvable in auto-mode — that is the executor's own +`` (`agents/gsd-executor.md`), and `checkpoints.md` (the +full gate table) is embedded in the dispatch `` verbatim. +Only `gate="blocking-human"` always surfaces to a human, regardless of +auto-mode. + +When composing the `Agent()` prompt, do NOT add text refusing or overriding +auto-approval for a `blocking` gate. Orchestrator-composed instructions that contradict the +executor's protocol win the executor's attention, stall autonomous runs at the +checkpoint, and defeat `_auto_chain_active`/auto-advance for the common case. +Executor-side gate semantics are already complete; compose nothing about gates +beyond what the template already embeds. diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index d92b56fd3..1dbb075aa 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -140,7 +140,7 @@ Otherwise: Apply checkpoint-based routing below. > **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 `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 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, honor checkpoint gate semantics (#3370) — gate="blocking" (the default) is auto-approvable in auto-mode per the executor's own checkpoint protocol, gate="blocking-human" always surfaces to a human; add no instruction overriding that protocol — 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. **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). diff --git a/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json b/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json deleted file mode 100644 index 342e327dc..000000000 --- a/tests/emitted-drift-acks/2652-quick-diagnose-dispatch-isolation.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "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/3324-at-includes-literal-text.json b/tests/emitted-drift-acks/3324-at-includes-literal-text.json deleted file mode 100644 index 2f4a626d4..000000000 --- a/tests/emitted-drift-acks/3324-at-includes-literal-text.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": 1, - "paths": { - "execute-phase.md": "#3324: the block of the gsd-executor Agent() dispatch prompt listed companion files as raw @~/ include lines. Claude Code expands @path only in natively-loaded markdown bodies, never inside a dynamically constructed prompt=\"...\" string, so every execute-plan.md-only step (segment_execution, previous_phase_check, verification_failure_gate, update_codebase_map) silently never reached dispatched executors. The block now carries the ORCHESTRATOR build-time embed instruction already established by the adjacent block (b88d6d6ef / #589) — the orchestrator reads each listed file and inlines its contents verbatim before calling Agent() — and the paths are backticked list entries with no @ sigil so no literal include line can leak into the prompt. Both dispatch paths share this block (sequential reuses the worktree-mode structure), and a repo-wide prompt-region guard in tests/workflow-size-budget.test.cjs prevents reintroduction anywhere under gsd-core/workflows/**. The 432-byte growth (92954 -> 93386, still under the XL 96 KiB ceiling) is the embed instruction line itself plus the backticked path list replacing the five bare include lines; every byte of it exists to prevent the silent step-loss failure mode from returning." - } -} diff --git a/tests/emitted-drift-acks/3370-execute-phase-gate-conflation.json b/tests/emitted-drift-acks/3370-execute-phase-gate-conflation.json new file mode 100644 index 000000000..b1036062c --- /dev/null +++ b/tests/emitted-drift-acks/3370-execute-phase-gate-conflation.json @@ -0,0 +1,7 @@ +{ + "version": 1, + "paths": { + "execute-phase.md": "#3370: the step-3 executor-routing line now also cites #3370 — the checkpoint gate rule itself lives in the execute-phase/steps/per-plan-executor-routing.md fragment (loaded per plan in every isolation mode immediately before the dispatch prompt is composed), the same keep-the-host-lean pattern #1689/#3417 used, because the host sits under the frozen ADR-857 Phase 6 ceiling (≤93400). Net growth is 6 bytes (93386 -> 93392): the routing citation only; the rule text, which forbids the orchestrator from composing dispatch text that refuses or overrides auto-approval for the default gate=\"blocking\" (only blocking-human always surfaces), is in the fragment. Supersedes the spent #3324 fragment (merged into next), which also named execute-phase.md and would otherwise double-ack the same path.", + "execute-plan.md": "#3370: the Pattern A dispatch prompt spec gained the gate-semantics clause (gate=\"blocking\" (the default) is auto-approvable in auto-mode per the executor's own checkpoint protocol, gate=\"blocking-human\" always surfaces to a human; add no instruction overriding that protocol), closing the identically-shaped dispatch-time gap on the single-plan path named in the issue. Growth ~248 bytes (38913 -> 39161, still under the DEFAULT 40 KiB ceiling). Supersedes the spent #2652 fragment (merged into next), which also named execute-plan.md and would otherwise double-ack the same path." + } +} diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index 7c42d1611..9c11c5baf 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -674,6 +674,106 @@ describe('#3324: no @-include lines inside Agent() prompt strings', () => { ); }); + // #3370 — the executor dispatch prompts must carry checkpoint gate semantics so the + // orchestrator cannot compose anti-auto-approval prompt text that conflates + // gate="blocking" (the default, auto-approvable) with gate="blocking-human" + // (always surfaces). The dispatch prompt text IS the product here — the templates + // below are what gets composed into the Agent() call — so region asserts on the + // template text are the behavioral seam, same precedent as the #3324 guards above. + const ANTI_AUTO_APPROVAL = /never auto-approve|do not auto-approve|must not auto-approve|under any circumstance, including/; + + function dispatchRegion(file, fromAnchor, toAnchor) { + const content = fs.readFileSync(path.join(WORKFLOWS_DIR, file), 'utf-8'); + const from = content.indexOf(fromAnchor); + assert.ok(from !== -1, `${file}: anchor "${fromAnchor}" not found`); + const to = content.indexOf(toAnchor, from); + assert.ok(to !== -1, `${file}: anchor "${toAnchor}" not found after "${fromAnchor}"`); + return content.slice(from, to); + } + + test('execute-phase step-3 routes checkpoint gate semantics through the per-plan routing fragment (#3370)', () => { + // The host file sits under the frozen ADR-857 Phase 6 ceiling (≤93400 bytes), so the + // gate rule lives in the per-plan-executor-routing fragment — the same + // keep-the-host-lean pattern #1689/#3417 used — which step 3 loads for EVERY plan + // in every isolation mode (harness-worktree, orchestrator-worktree, sequential) + // immediately before the dispatch prompt is composed. + const step = dispatchRegion( + 'execute-phase.md', + '**Spawn executor agents:**', + '**Wait for all agents in wave to complete.**', + ); + assert.match( + step, + /Executor routing \([^)]*#3370/, + 'step 3\'s executor-routing line must cite #3370 so the gate rule is loaded with it', + ); + + const fragment = fs.readFileSync( + path.join(WORKFLOWS_DIR, 'execute-phase', 'steps', 'per-plan-executor-routing.md'), + 'utf-8', + ); + // AC 1 + AC 3, phase-level: blocking is the auto-approvable default, blocking-human + // is the only always-surface gate, and the orchestrator is forbidden from injecting + // dispatch text that refuses auto-approval. + assert.match(fragment, /#3370/, 'the routing fragment must carry the gate rule'); + assert.match(fragment, /gate="blocking"/, 'the gate rule must name gate="blocking"'); + assert.match(fragment, /auto-approv/i, 'the gate rule must state blocking is auto-approvable in auto-mode'); + assert.match(fragment, /blocking-human/, 'the gate rule must name gate="blocking-human" as the always-surface carve-out'); + assert.match( + fragment, + /do NOT add text refusing or overriding\s+auto-approval/, + 'the gate rule must forbid composing dispatch text that refuses or overrides auto-approval', + ); + // Negative guard: the fix must not itself introduce the anti-auto-approval phrasing. + assert.doesNotMatch( + fragment, + ANTI_AUTO_APPROVAL, + 'the gate rule must not contain anti-auto-approval instructions (#3370)', + ); + }); + + test('execute-phase.md executor Agent() prompt contains no anti-auto-approval instruction in any block (#3370)', () => { + const step = dispatchRegion( + 'execute-phase.md', + '**Spawn executor agents:**', + '**Wait for all agents in wave to complete.**', + ); + // The gate rule lives in the step-3 instructions (previous test), which every + // isolation mode executes; the prompt template itself never carried gate text and + // must stay free of anti-auto-approval phrasing — the executor's semantics come + // from its own plus the build-time-embedded checkpoints.md + // (#3324), which this guards against the template contradicting. + assert.doesNotMatch( + step, + ANTI_AUTO_APPROVAL, + 'the step-3 dispatch region (instructions + Agent() prompt template) must not ' + + 'contain anti-auto-approval instructions (#3370)', + ); + }); + + test('execute-plan.md Pattern A dispatch carries the same gate semantics (#3370)', () => { + const patternA = dispatchRegion( + 'execute-plan.md', + '**Pattern A:** init_agent_tracking', + '**Pattern B:** Execute segment-by-segment', + ); + // AC 4: the single-plan-level dispatch path is covered, not just execute-phase. + assert.match(patternA, /#3370/, 'Pattern A must cite the gate-semantics rule'); + assert.match(patternA, /gate="blocking"/, 'Pattern A must name gate="blocking"'); + assert.match(patternA, /blocking-human/, 'Pattern A must name gate="blocking-human"'); + assert.match(patternA, /auto-approv/i, 'Pattern A must state blocking is auto-approvable in auto-mode'); + assert.match( + patternA, + /no instruction (?:that )?overrid/i, + 'Pattern A must forbid adding instructions that override the executor checkpoint protocol', + ); + assert.doesNotMatch( + patternA, + ANTI_AUTO_APPROVAL, + 'Pattern A must not contain anti-auto-approval instructions (#3370)', + ); + }); + test('execute-plan.md still defines the steps only it carries into the dispatch', () => { const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'execute-plan.md'), 'utf-8'); for (const marker of [