* fix(#3370): state checkpoint gate semantics in executor dispatch prompts * fix(#3370): set changeset pr to 3478 * fix(#3370): keep gate rule in routing fragment under phase-6 ceiling --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/bold-wasps-hop.md
Normal file
5
.changeset/bold-wasps-hop.md
Normal file
@@ -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.
|
||||
@@ -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`):
|
||||
|
||||
|
||||
@@ -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 `<worktree_branch_check>` build-time embed note and the `<parallel_execution>` harness block, keep `<objective>`, the execution context, and `<success_criteria>` 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 `<worktree_branch_check>` build-time embed note and the `<parallel_execution>` harness block, keep `<objective>`, the execution context, and `<success_criteria>` 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
|
||||
|
||||
@@ -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
|
||||
`<checkpoint_protocol>` (`agents/gsd-executor.md`), and `checkpoints.md` (the
|
||||
full gate table) is embedded in the dispatch `<execution_context>` 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.
|
||||
|
||||
@@ -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 <role> --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_<ROLE>}` (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 `<worktree_branch_check>` 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 `<worktree_branch_check>` 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).
|
||||
|
||||
|
||||
@@ -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."
|
||||
}
|
||||
}
|
||||
@@ -1,6 +0,0 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"execute-phase.md": "#3324: the <execution_context> 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 <worktree_branch_check> 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."
|
||||
}
|
||||
}
|
||||
@@ -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."
|
||||
}
|
||||
}
|
||||
@@ -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 <checkpoint_protocol> 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 [
|
||||
|
||||
Reference in New Issue
Block a user