From b55848bd7d247620870e6d858057f4514bf5332f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 10 May 2026 17:38:36 -0400 Subject: [PATCH] fix(codex): block unsupported execute worktrees --- .../fix-3360-codex-execute-worktrees.md | 5 ++ bin/install.js | 4 ++ get-shit-done/workflows/execute-phase.md | 29 ++++---- ...360-codex-execute-phase-worktrees.test.cjs | 68 +++++++++++++++++++ 4 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 .changeset/fix-3360-codex-execute-worktrees.md create mode 100644 tests/bug-3360-codex-execute-phase-worktrees.test.cjs diff --git a/.changeset/fix-3360-codex-execute-worktrees.md b/.changeset/fix-3360-codex-execute-worktrees.md new file mode 100644 index 000000000..4ab6042c1 --- /dev/null +++ b/.changeset/fix-3360-codex-execute-worktrees.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3365 +--- +**Codex `execute-phase` now fails closed when `workflow.use_worktrees=true`** — because Codex `spawn_agent` has no direct mapping for Claude Code's `isolation="worktree"`, the workflow now stops before executor dispatch instead of letting workspace-write agents edit the main checkout while the workflow assumes worktree isolation. (#3360) diff --git a/bin/install.js b/bin/install.js index a8d25524c..6095e9b15 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2334,6 +2334,10 @@ Direct mapping: at install time so \`model_overrides\` from \`.planning/config.json\` and \`~/.gsd/defaults.json\` are honored automatically by Codex's agent router. - \`fork_context: false\` by default — GSD agents load their own context via \`\` blocks +- \`Task(isolation="worktree")\` / \`Agent(isolation="worktree")\` → no direct Codex mapping. + Codex \`spawn_agent\` does not create or bind a git worktree automatically. + Workflows that require this isolation must fail closed or use an explicit + manual worktree protocol before spawning (#3360). Spawn restriction: - Codex restricts \`spawn_agent\` to cases where the user has explicitly diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index cb86b4e08..5e47fd7ac 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -77,13 +77,20 @@ Parse JSON for: `executor_model`, `verifier_model`, `commit_docs`, `parallelizat **If `response_language` is set:** Include `response_language: {value}` in all spawned subagent prompts so any user-facing output stays in the configured language. -Read worktree config: +Read runtime/worktree config and fail closed before any executor dispatch: ```bash +RUNTIME=$(gsd-sdk query config-get runtime --default claude 2>/dev/null || echo "claude") USE_WORKTREES=$(gsd-sdk query config-get workflow.use_worktrees 2>/dev/null || echo "true") EXECUTOR_STALL_INTERVAL_MINUTES=$(gsd-sdk query config-get executor.stall_detect_interval_minutes 2>/dev/null || echo "5") EXECUTOR_STALL_THRESHOLD_MINUTES=$(gsd-sdk query config-get executor.stall_threshold_minutes 2>/dev/null || echo "10") + +if [ "$RUNTIME" = "codex" ] && [ "$USE_WORKTREES" != "false" ]; then + echo "FATAL: Codex execute-phase worktree isolation is unsupported. Set workflow.use_worktrees=false or use a runtime with Agent isolation=\"worktree\" support." >&2 + exit 1 +fi ``` +Codex maps subagents to `spawn_agent`, which has no direct Codex mapping for Claude Code's `isolation="worktree"` parameter. Failing closed prevents main-checkout edits while the workflow believes agents are isolated. If the project uses git submodules, worktree isolation is unsafe **only when a plan touches a submodule path** — the executor commit protocol cannot correctly handle submodule commits inside isolated worktrees. The previous behavior unconditionally disabled worktree isolation whenever `.gitmodules` existed, which penalised every plan in a submodule project even when the plan was nowhere near a submodule. Compute submodule paths once and intersect them per-plan with the plan's declared `files_modified` frontmatter. @@ -499,9 +506,7 @@ increases monotonically across waves. `{status}` is `complete` (success), **Emit a plan-start heartbeat (literal line, no tool call) immediately before each `Agent()` dispatch (#2410):** - ``` - [checkpoint] phase {PHASE_NUMBER} wave {N}/{M} plan {plan_id} starting ({P}/{Q} plans done) - ``` + `[checkpoint] phase {PHASE_NUMBER} wave {N}/{M} plan {plan_id} starting ({P}/{Q} plans done)` Pass paths only — executors read files themselves with their fresh context window. For 200k models, this keeps orchestrator context lean (~10-15%). @@ -517,19 +522,13 @@ increases monotonically across waves. `{status}` is `complete` (success), ``` **Sequential dispatch for parallel execution (waves with 2+ agents):** - When spawning multiple agents in a wave, dispatch each `Agent()` call **one at a time - with `run_in_background: true`** — do NOT send all Agent calls in a single message. - `git worktree add` acquires an exclusive lock on `.git/config.lock`, so simultaneous - calls race for this lock and fail. Sequential dispatch ensures each worktree finishes - creation before the next begins (the round-trip latency of each tool call provides - natural spacing), while all agents still **run in parallel** once created. + Dispatch each `Agent()` call **one at a time with `run_in_background: true`**. Do NOT + send all Agent calls in a single message: simultaneous `git worktree add` calls race + on `.git/config.lock`. Agents still run in parallel once their worktrees are created. ```text - # CORRECT: dispatch one Agent() per message, each with run_in_background: true - # → worktrees created sequentially, agents execute in parallel - # - # WRONG: multiple Agent() calls in a single message - # → simultaneous git worktree add → .git/config.lock contention → failures + # CORRECT: one Agent() per message with run_in_background: true + # WRONG: multiple Agent() calls in one message -> .git/config.lock contention ``` ```text diff --git a/tests/bug-3360-codex-execute-phase-worktrees.test.cjs b/tests/bug-3360-codex-execute-phase-worktrees.test.cjs new file mode 100644 index 000000000..c38892167 --- /dev/null +++ b/tests/bug-3360-codex-execute-phase-worktrees.test.cjs @@ -0,0 +1,68 @@ +/** + * Regression test for bug #3360. + * + * Codex does not have a direct equivalent of Claude Code's + * `Agent(... isolation="worktree")`. The execute-phase workflow must fail + * closed for Codex + workflow.use_worktrees=true instead of spawning + * workspace-write executors in the main checkout. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const EXECUTE_PHASE = path.join(ROOT, 'get-shit-done', 'workflows', 'execute-phase.md'); +const { getCodexSkillAdapterHeader } = require('../bin/install.js'); + +function parseWorkflowSteps(content) { + return [...content.matchAll(/]*>([\s\S]*?)<\/step>/g)] + .map((match) => { + const body = match[2]; + return { + name: match[1], + readsRuntimeConfig: body.includes('RUNTIME=$(gsd-sdk query config-get runtime --default claude'), + codexWorktreeGuard: body.includes('Codex execute-phase worktree isolation is unsupported'), + worktreeDispatchGuidance: body.includes('isolation="worktree"'), + }; + }); +} + +function executePhaseWorktreeContract(content) { + const steps = parseWorkflowSteps(content); + const initializeIndex = steps.findIndex((step) => step.name === 'initialize'); + const firstWorktreeDispatchIndex = steps.findIndex((step) => step.worktreeDispatchGuidance); + assert.notEqual(initializeIndex, -1, 'workflow must have an initialize step'); + assert.notEqual(firstWorktreeDispatchIndex, -1, 'workflow must still document worktree dispatch guidance'); + + const initialize = steps[initializeIndex]; + return { + initializeReadsRuntimeConfig: initialize.readsRuntimeConfig, + initializeHasCodexWorktreeGuard: initialize.codexWorktreeGuard, + guardStepPrecedesWorktreeDispatch: initializeIndex <= firstWorktreeDispatchIndex, + }; +} + +describe('#3360 — Codex execute-phase fails closed for unsupported worktree isolation', () => { + test('execute-phase reads runtime before worktree dispatch and blocks Codex worktree mode', () => { + const workflow = fs.readFileSync(EXECUTE_PHASE, 'utf8'); + const contract = executePhaseWorktreeContract(workflow); + + assert.deepEqual(contract, { + initializeReadsRuntimeConfig: true, + initializeHasCodexWorktreeGuard: true, + guardStepPrecedesWorktreeDispatch: true, + }); + }); + + test('Codex adapter documents that worktree isolation has no direct spawn_agent mapping', () => { + const header = getCodexSkillAdapterHeader('gsd-execute-phase'); + assert.match(header, /isolation="worktree"/); + assert.match(header, /no direct Codex mapping/i); + }); +});