From 66228a89cf9aefd0eb3ad173b68e5d9ec932b1f9 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 19 Aug 2026 20:50:12 -0400 Subject: [PATCH] fix(#3637): carry the full executor contract in the orchestrator-worktree spawn (#3694) * test(#3637): pin the executor contract in the orchestrator-worktree spawn prompt * fix(#3637): carry the full executor contract in the orchestrator-worktree spawn prompt * fix(#3637): role-definition embed, embed-performance gates, drop stale ack * chore(#3637): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/clever-pumas-march.md | 5 + .../steps/executor-isolation-dispatch.md | 113 +++++++++++++++-- ...xecutor-isolation-prompt-contract.test.cjs | 119 ++++++++++++++++++ 3 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 .changeset/clever-pumas-march.md create mode 100644 tests/executor-isolation-prompt-contract.test.cjs diff --git a/.changeset/clever-pumas-march.md b/.changeset/clever-pumas-march.md new file mode 100644 index 000000000..874a7df4f --- /dev/null +++ b/.changeset/clever-pumas-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3694 +--- +**Codex worktree-parallel executors now launch with the full executor contract** — the orchestrator-worktree process spawn handed its child a short objective-only prompt, so executors reconstructed their role by repository search and force-staged gitignored SUMMARY.md files (`git add -f`) to satisfy an unconditional commit criterion. The spawn prompt now carries the embedded executor workflow, required reading with the explicit plan path, the gsd-executor persona, and skip-aware success criteria, and halts before spawn when the contract embeds cannot be resolved. (#3637) 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 1ebe03b9b..2b3a2269b 100644 --- a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md +++ b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md @@ -130,14 +130,37 @@ 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. 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: +First build the executor prompt. It is the **same prompt text the harness path's `Agent()` call uses** — same executor identity, same execution context, same required-reading contract — with only the harness-only framing removed: drop the `` build-time embed note (this backend pins the base itself via `worktree create --base` and verifies it at merge) and the `` harness block (its SUMMARY-commit semantics ride inside the embedded `execute-plan.md` worktree mode). Keep ``, ``, ``, `${AGENT_SKILLS}`, and `` from the harness prompt — substituting the worktree-specific `` framing below. 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. + +#3637: the process-spawned child has NO host subagent machinery — nothing loads the `gsd-executor` agent definition or the execute-plan workflow unless THIS prompt carries them. A short objective-only prompt forces the child to reconstruct its role from the repository (skill discovery, inference) and leaves it unaware of the gitignored-planning skip semantics, which is how executors ended up force-staging gitignored `SUMMARY.md` files to satisfy an unconditional commit criterion. Build-time embeds below are therefore MANDATORY, and the resolution check after the assignment fails closed: if any embed source cannot be read, do NOT spawn a generic process and hope — halt the wave (the fail-closed check after `dispatch-isolation` handles the worktree teardown). + +Assign the composed prompt to a shell variable so it can be passed as one argument: ```bash # Compose the executor prompt for THIS plan. Single-quoted multi-line # assignment (NOT a heredoc): these blocks are indented inside the workflow, # and a heredoc terminator must sit at column 0 — `<<-` strips only tabs, not # the leading spaces, so a heredoc here would never terminate. Single quotes -# also stop the shell expanding anything in the prompt body. +# also stop the shell expanding anything in the prompt body — the prompt MUST +# contain no single-quote character. +# +# ORCHESTRATOR BUILD-TIME EMBEDS (do these BEFORE the spawn, in order): +# 1. Inline each file listed in verbatim, in order. +# An unreadable source file is a halt condition (#3637 fail-closed), +# never a skip — a child without these texts is not a gsd-executor. +# 2. Substitute this plan's {plan_number}, {phase_number}, {phase_name}, +# {phase_dir}, and {plan_file} placeholders (same values the harness +# path substitutes into its Agent() prompt). +# 3. Inline the gsd-executor ROLE DEFINITION: read `agents/gsd-executor.md` +# (resolved against the install root the same way the harness runtime +# resolves subagent types) and inline it verbatim at the provenance +# marker below. The agent-skills query alone is NOT sufficient — its +# block is the skills include list whenever the project configures +# agent_skills for gsd-executor, and only an UNCONFIGURED non-Claude +# project gets the full agent file via the #2454 fallback. The child has +# no host subagent machinery, so the role definition must ride the prompt +# (#3637 acceptance: resolved agent instructions as launch-level +# instructions + provenance of which role definition was used). EXECUTOR_PROMPT=' Execute plan {plan_number} of phase {phase_number}-{phase_name}. Commit each task atomically. Create SUMMARY.md. @@ -145,19 +168,95 @@ Do NOT update STATE.md or ROADMAP.md — the orchestrator owns those writes afte -You are running as an executor in a git worktree GSD created for you. Your -working directory IS that worktree. Do not cd elsewhere, and do not run any -git command that targets the main checkout. Use normal git commits WITH hooks. -Do NOT use --no-verify. +You are the gsd-executor agent running in a git worktree GSD created for you. +Your working directory IS that worktree. Do not cd elsewhere, and do not run +any git command that targets the main checkout. Use normal git commits WITH +hooks. Do NOT use --no-verify. + +ORCHESTRATOR build-time embed (NOT a child-process runtime step): the files +below were inlined verbatim into this prompt before you were spawned. If any +section below is missing or truncated, stop and report it — do not improvise +the executor workflow from repository search. +- execute-plan.md (the executor workflow you run, including its worktree-mode + SUMMARY commit semantics and the gitignored-planning skip contract) +- summary.md template +- checkpoints.md +- tdd.md +- worktree-path-safety.md +- agents/gsd-executor.md (the ROLE DEFINITION you are executing — its steps + 0/0a/0b per-commit HEAD/cwd-drift/path-guard discipline applies to every + commit you make in this worktree, and its final_commit contract is the + authority for the skip semantics in ) + +(Inline the actual contents of each file above at compose time — this block +is the provenance record of what was embedded and where it came from.) + REQUIRED ORDER: Write SUMMARY.md, commit, then any narration. + +Read these files at execution start using the Read tool. +First resolve repo root so every path is anchored: +PROJECT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +- ${PROJECT_ROOT}/{phase_dir}/{plan_file} (Plan — THE plan you are executing) +- ${PROJECT_ROOT}/.planning/PROJECT.md (Project context — core value, requirements, evolution rules) +- ${PROJECT_ROOT}/.planning/STATE.md (State) +- ${PROJECT_ROOT}/.planning/config.json (Config, if exists) +- ${PROJECT_ROOT}/CLAUDE.md (Project instructions, if exists — follow project-specific guidelines and coding conventions) +- ${PROJECT_ROOT}/.claude/skills/ or ${PROJECT_ROOT}/.agents/skills/ (Project skills, if either exists — list skills, read SKILL.md for each, follow relevant rules during implementation) +- ${PROJECT_ROOT}/{phase_dir}/*-CONTEXT.md (User decisions from discuss-phase — honors locked choices; skip silently when none exist) +- ${PROJECT_ROOT}/{phase_dir}/*-RESEARCH.md (Technical research — pitfalls and patterns to follow; skip silently when none exist) +- ${PROJECT_ROOT}/{prior_wave_summaries} (SUMMARY.md files from earlier waves in this phase — what was already built; PARALLEL WAVES especially must not duplicate or clobber sibling work; substitute the empty string when this is wave 1) + + + +If CLAUDE.md or project instructions reference MCP tools, prefer those tools +over Grep/Glob for code navigation when available. Check tool availability +first — fall back to Grep/Glob if not accessible. + + +${AGENT_SKILLS} + - [ ] All tasks executed - [ ] Each task committed individually -- [ ] SUMMARY.md created AND committed in the plan directory +- [ ] SUMMARY.md created in the plan directory and committed — OR an + intentional skip recorded (skipped_gitignored when .planning is + gitignored, skipped_commit_docs_false when commit_docs is disabled). + Never force-stage gitignored planning artifacts: git add -f on + .planning paths is forbidden. An intentional skip is a success path. +- [ ] No modifications to shared orchestrator artifacts (the orchestrator handles all post-wave shared-file writes) ' [ -n "$EXECUTOR_PROMPT" ] || { echo "FATAL: executor prompt is empty for plan {plan_number}." >&2; exit 1; } +# #3637 fail-closed verification. Two classes of check: +# (a) template completeness — the prompt must carry the required-reading +# contract and the skip semantics at all (catches wholesale truncation +# back to the objective-only prompt); +# (b) embed PERFORMANCE — the compose-time placeholders must actually be +# gone. A raw, un-embedded template still contains its own +# instructions-about-instructions (the provenance parenthetical and the +# un-substituted persona marker); those strings surviving to spawn time +# mean the embeds were skipped and the child would hold instructions +# about files it never received — the milder recurrence of #3637. +# These run BEFORE worktree creation, so a gate exit leaves nothing to tear +# down; the post-dispatch-isolation fail-closed check governs the worktree +# itself once one exists. +printf '%s' "$EXECUTOR_PROMPT" | grep -q '' || { + echo "FATAL: executor prompt for plan {plan_number} is missing the required-reading contract — the template is truncated. Halting rather than spawning a generic process." >&2 + exit 1 +} +printf '%s' "$EXECUTOR_PROMPT" | grep -q 'skipped_gitignored' || { + echo "FATAL: executor prompt for plan {plan_number} is missing the gitignored-planning skip semantics — executors without them force-stage planning artifacts (#3637). Halting." >&2 + exit 1 +} +printf '%s' "$EXECUTOR_PROMPT" | grep -q 'Inline the actual contents' && { + echo "FATAL: executor prompt for plan {plan_number} still contains its compose-time placeholder — the build-time embeds were not performed. Halting rather than dispatching instructions-about-instructions (#3637)." >&2 + exit 1 +} +printf '%s' "$EXECUTOR_PROMPT" | grep -q '\${AGENT_SKILLS}' && { + echo "FATAL: executor prompt for plan {plan_number} still contains the un-substituted \${AGENT_SKILLS} marker — the role definition was not spliced in (#3637). Halting." >&2 + exit 1 +} ``` The prompt body must contain no single-quote character, since the assignment above is single-quoted; keep apostrophes out of it when editing. diff --git a/tests/executor-isolation-prompt-contract.test.cjs b/tests/executor-isolation-prompt-contract.test.cjs new file mode 100644 index 000000000..ec8ffcd87 --- /dev/null +++ b/tests/executor-isolation-prompt-contract.test.cjs @@ -0,0 +1,119 @@ +'use strict'; + +/** + * #3637 — the orchestrator-worktree process spawn must carry the gsd-executor + * identity and execution contract, not a short objective-only prompt. + * + * The process-spawned child has NO host subagent machinery: nothing loads the + * gsd-executor agent definition or the execute-plan workflow unless the + * EXECUTOR_PROMPT carries them. A short prompt forced both reporters' executors + * to reconstruct their role from repository search AND left them unaware of the + * gitignored-planning skip semantics — so they force-staged gitignored + * SUMMARY.md files (`git add -f`) to satisfy an unconditional + * "SUMMARY.md created AND committed" criterion. + * + * These are source-text-is-the-product assertions: the workflow markdown IS + * what the orchestrator runtime loads. The contract checked here is the + * documented dispatch text, per the issue's acceptance list. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const FRAGMENT = path.join( + __dirname, '..', 'gsd-core', 'workflows', 'execute-phase', 'steps', 'executor-isolation-dispatch.md', +); +// allow-test-rule: source-text-is-the-product the dispatch fragment is runtime-loaded workflow text (#3637) +const content = fs.readFileSync(FRAGMENT, 'utf8'); + +/** The EXECUTOR_PROMPT single-quoted assignment body. */ +function promptBody() { + const start = content.indexOf("EXECUTOR_PROMPT='"); + const end = content.indexOf("'", start); + assert.ok(start !== -1 && end !== -1, 'EXECUTOR_PROMPT assignment must exist'); + return content.slice(start, end); +} + +test('#3637: the executor prompt carries the required-reading contract with the explicit plan path', () => { + const body = promptBody(); + assert.match(body, //, 'the prompt must carry the required-reading contract'); + assert.match(body, /\{phase_dir\}\/\{plan_file\}/, 'the prompt must name the explicit plan path'); + assert.match(body, /PROJECT\.md/, 'project context is required reading'); +}); + +test('#3637: the executor prompt carries the execution-context build-time embeds', () => { + const body = promptBody(); + assert.match(body, //, 'the prompt must carry an execution_context block'); + assert.match(body, /execute-plan\.md/, 'execute-plan.md must be embedded (the executor workflow)'); + assert.match(body, /worktree-path-safety\.md/, 'worktree path safety must be embedded'); +}); + +test('#3637: the success criteria make an intentional gitignored skip a success path and forbid force-staging', () => { + const body = promptBody(); + assert.match(body, /skipped_gitignored/, 'skipped_gitignored must be named as an intentional success path'); + assert.match(body, /git add -f/, 'force-staging must be explicitly forbidden'); + assert.doesNotMatch( + body, + /SUMMARY\.md created AND committed(?![^\n]*OR)/, + 'the unconditional "created AND committed" criterion that drove git add -f must not appear bare', + ); +}); + +test('#3637: the compose step fails closed when the embeds are missing', () => { + assert.match( + content, + /grep -q ''[\s\S]{0,400}exit 1/, + 'a prompt missing the required-reading contract must halt before spawn', + ); + assert.match( + content, + /grep -q 'skipped_gitignored'[\s\S]{0,400}exit 1/, + 'a prompt missing the skip semantics must halt before spawn', + ); +}); + +test('#3637: embed PERFORMANCE is gated — an un-embedded template halts before spawn', () => { + // The two template-completeness greps above cannot detect skipped embeds + // (the placeholders live in the template itself). The performance gates + // catch the raw-template dispatch: the provenance parenthetical must be + // GONE and the persona marker must be substituted (#3637 review finding). + assert.match( + content, + /grep -q 'Inline the actual contents'[\s\S]{0,300}exit 1/, + 'a prompt still carrying its compose-time placeholder means the embeds were skipped — halt', + ); + assert.match( + content, + /grep -q '\\\$\{AGENT_SKILLS\}'[\s\S]{0,300}exit 1/, + 'an un-substituted persona marker means the role definition was not spliced — halt', + ); +}); + +test('#3637: the gsd-executor ROLE DEFINITION is a mandatory embed with provenance', () => { + // The agent-skills query alone is conditional (its block is a skills list + // whenever the project configures agent_skills; only unconfigured + // non-Claude projects get the full agent file via the #2454 fallback). + // The child has no host subagent machinery — the role definition itself + // must ride the prompt (#3637 acceptance bullets 1 and 5). + const body = promptBody(); + assert.match(body, /agents\/gsd-executor\.md/, 'the role definition file must be named as an embedded source'); + assert.match(body, /steps\s*0\/0a\/0b/, 'the per-commit HEAD/cwd-drift/path-guard discipline must be pointed at'); +}); + +test('#3637: prior-wave summaries are required reading (parallel waves must not clobber siblings)', () => { + const body = promptBody(); + assert.match(body, /prior_wave_summaries/, 'prior-wave SUMMARY files must be required reading on this parallel-wave backend'); +}); + +test('#3637: the persona rides the prompt (${AGENT_SKILLS}), same as the harness path', () => { + assert.match(promptBody(), /\$\{AGENT_SKILLS\}/, 'the gsd-executor persona variable must be spliced into the prompt'); +}); + +test('#3637: the prompt body contains no single-quote character (single-quoted assignment)', () => { + const body = promptBody(); + const startQuote = body.indexOf("'"); + const rest = body.slice(startQuote + 1); + assert.ok(!rest.includes("'"), 'the prompt body must be free of apostrophes or the shell assignment breaks'); +});