From 6e242bd76a097e9788aa5376d02d1a41e213a16c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 14:00:06 -0400 Subject: [PATCH 1/5] fix: allow quick worktree parent plan base (#1347) --- .changeset/eager-ravens-dart.md | 5 ++ gsd-core/references/worktree-branch-check.md | 16 ++-- gsd-core/workflows/quick.md | 25 +++++- src/worktree-safety.cts | 11 ++- ...432-quick-plan-predispatch-commit.test.cjs | 37 +++++++++ tests/workflow-size-baseline.json | 2 +- tests/worktree-cleanup.test.cjs | 2 +- tests/worktree-safety.test.cjs | 81 +++++++++++++++++++ tests/worktree.test.cjs | 6 +- 9 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 .changeset/eager-ravens-dart.md diff --git a/.changeset/eager-ravens-dart.md b/.changeset/eager-ravens-dart.md new file mode 100644 index 000000000..50003fc19 --- /dev/null +++ b/.changeset/eager-ravens-dart.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1347 +--- +**Quick worktree execution now accepts parent-or-plan bases for pre-dispatch plan commits** — quick mode records the parent and plan commit around the pre-dispatch PLAN.md commit, lets the worktree guard accept either approved base, materializes the plan from git objects when a runtime forks from the parent, and teaches cleanup to validate the same allowed-base set. (#1265) diff --git a/gsd-core/references/worktree-branch-check.md b/gsd-core/references/worktree-branch-check.md index dbc4fda8f..e44f4e2ea 100644 --- a/gsd-core/references/worktree-branch-check.md +++ b/gsd-core/references/worktree-branch-check.md @@ -6,9 +6,13 @@ block — do not inline a copy elsewhere. History of coordinated edits: #2924, # **Contract for orchestrators:** before dispatch, capture `EXPECTED_BASE=$(git rev-parse HEAD)`, then embed the block below into the sub-agent prompt verbatim, substituting `{EXPECTED_BASE}` -with that captured SHA. The sub-agent only *verifies* and fails closed; the orchestrator -(the worktree lifecycle owner) performs any base recovery — the sub-agent never rewrites a -worktree it did not create (#48). +with that captured SHA. Orchestrators that intentionally create a docs-only pre-dispatch +plan commit may also substitute `{EXPECTED_BASE_ALTERNATE}` with that commit's immediate +parent so runtimes that fork from either side of the docs-only commit pass the same +fail-closed guard (#1265). Otherwise substitute `{EXPECTED_BASE_ALTERNATE}` with an empty +string. The sub-agent only *verifies* and fails closed; the orchestrator (the worktree +lifecycle owner) performs any base recovery — the sub-agent never rewrites a worktree it +did not create (#48). FIRST ACTION: HEAD assertion MUST run before anything else, and this block is @@ -30,8 +34,10 @@ if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing to commit (#2924)." >&2 exit 42 fi -if [ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ]; then - echo "FATAL: worktree base mismatch — HEAD is $(git rev-parse HEAD), expected {EXPECTED_BASE}. Orchestrator owns recovery; sub-agent refuses to rewrite the worktree (#48)." >&2 +ACTUAL_BASE=$(git rev-parse HEAD) +EXPECTED_BASE_ALTERNATE="{EXPECTED_BASE_ALTERNATE}" +if [ "$ACTUAL_BASE" != "{EXPECTED_BASE}" ] && { [ -z "$EXPECTED_BASE_ALTERNATE" ] || [ "$ACTUAL_BASE" != "$EXPECTED_BASE_ALTERNATE" ]; }; then + echo "FATAL: worktree base mismatch — HEAD is $ACTUAL_BASE, expected {EXPECTED_BASE}${EXPECTED_BASE_ALTERNATE:+ or $EXPECTED_BASE_ALTERNATE}. Orchestrator owns recovery; sub-agent refuses to rewrite the worktree (#48)." >&2 exit 42 fi ``` diff --git a/gsd-core/workflows/quick.md b/gsd-core/workflows/quick.md index ce6882dde..c63254641 100644 --- a/gsd-core/workflows/quick.md +++ b/gsd-core/workflows/quick.md @@ -632,7 +632,10 @@ When `USE_WORKTREES !== "false"`, commit PLAN.md to the current branch **before* Skip this step entirely if `USE_WORKTREES === "false"` (non-worktree mode: PLAN.md is committed in Step 8 as usual). ```bash +QUICK_PLAN_PARENT="" +QUICK_PLAN_COMMIT="" if [ "${USE_WORKTREES}" != "false" ]; then + QUICK_PLAN_PARENT=$(git rev-parse HEAD) COMMIT_DOCS=$(gsd_run query config-get commit_docs 2>/dev/null || echo "true") if [ "$COMMIT_DOCS" != "false" ]; then git add "${QUICK_DIR}/${quick_id}-PLAN.md" @@ -650,8 +653,12 @@ if [ "${USE_WORKTREES}" != "false" ]; then git commit -m "docs(${quick_id}): pre-dispatch plan for ${DESCRIPTION}" -- "${QUICK_DIR}/${quick_id}-PLAN.md" \ || { echo "ERROR: pre-dispatch PLAN.md commit failed — likely a pre-commit hook failure. Fix the hook output above (or set workflow.worktree_skip_hooks=true to bypass) and re-run." >&2; exit 1; } fi + QUICK_PLAN_COMMIT=$(git rev-parse HEAD) fi fi + if [ -z "$QUICK_PLAN_COMMIT" ]; then + QUICK_PLAN_COMMIT=$(git rev-parse HEAD) + fi fi ``` @@ -678,8 +685,22 @@ Execute quick task ${quick_id}. ${USE_WORKTREES !== "false" ? ` -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}), 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. +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. + +FIRST ACTION after the worktree branch check: ensure the quick PLAN.md exists at a worktree-rooted relative path before any Read/Edit/Write path can be primed. If \`${QUICK_DIR}/${quick_id}-PLAN.md\` is absent, materialize it from the shared git object store: + +\`\`\`bash +QUICK_PLAN_COMMIT="${QUICK_PLAN_COMMIT}" +QUICK_PLAN_PATH="${QUICK_DIR}/${quick_id}-PLAN.md" +if [ ! -f "$QUICK_PLAN_PATH" ]; then + mkdir -p "$(dirname "$QUICK_PLAN_PATH")" + git show "${QUICK_PLAN_COMMIT}:${QUICK_PLAN_PATH}" > "$QUICK_PLAN_PATH" || { + echo "FATAL: unable to materialize quick plan from ${QUICK_PLAN_COMMIT}:${QUICK_PLAN_PATH}; refusing to continue." >&2 + exit 42 + } +fi +\`\`\` ` : ''} @@ -743,7 +764,7 @@ SUMMARY.md and stop — the user must rerun with worktrees disabled. > **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}` metadata to `QUICK_WORKTREE_MANIFEST` before cleanup. If any field is unavailable, stop and ask for recovery; do not discover global worktrees. +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. After executor returns: 1. **Worktree cleanup:** If the executor ran with `isolation="worktree"`, merge the worktree branch back and clean up: diff --git a/src/worktree-safety.cts b/src/worktree-safety.cts index dd30af403..892166789 100644 --- a/src/worktree-safety.cts +++ b/src/worktree-safety.cts @@ -416,6 +416,7 @@ interface CleanupManifestEntry { worktree_path: string; branch: string; expected_base: string; + allowed_bases?: string[]; } function normalizeCleanupManifestEntry(entry: unknown): CleanupManifestEntry | null { @@ -428,11 +429,16 @@ function normalizeCleanupManifestEntry(entry: unknown): CleanupManifestEntry | n const expectedBase = typeof e.expected_base === 'string' ? e.expected_base : ''; if (!worktreePath || !branch || !expectedBase) return null; if (!/^worktree-agent-[A-Za-z0-9._/-]+$/.test(branch)) return null; + const rawAllowedBases = Array.isArray(e.allowed_bases) ? e.allowed_bases : []; + const allowedBases = Array.from(new Set( + [expectedBase, ...rawAllowedBases.filter((base): base is string => typeof base === 'string' && base.length > 0)] + )); return { agent_id: typeof e.agent_id === 'string' ? e.agent_id : null, worktree_path: worktreePath, branch, expected_base: expectedBase, + allowed_bases: allowedBases, }; } @@ -693,7 +699,10 @@ function executeWorktreeWaveCleanupPlan(plan: WaveCleanupPlan | null, deps: Work } const mergeBase = execGit(['merge-base', 'HEAD', entry.branch], { cwd: plan.repoRoot }); - if (!gitResultOk(mergeBase) || mergeBase.stdout.trim() !== entry.expected_base) { + const allowedBases = Array.isArray(entry.allowed_bases) && entry.allowed_bases.length > 0 + ? entry.allowed_bases + : [entry.expected_base]; + if (!gitResultOk(mergeBase) || !allowedBases.includes(mergeBase.stdout.trim())) { result.status = 'blocked'; result.reason = 'base_mismatch'; result.stderr = mergeBase?.stderr || ''; diff --git a/tests/bug-2432-quick-plan-predispatch-commit.test.cjs b/tests/bug-2432-quick-plan-predispatch-commit.test.cjs index 7648e96df..950b969c2 100644 --- a/tests/bug-2432-quick-plan-predispatch-commit.test.cjs +++ b/tests/bug-2432-quick-plan-predispatch-commit.test.cjs @@ -99,4 +99,41 @@ describe('quick.md pre-dispatch PLAN.md commit (#2432)', () => { 'executor files_to_read must NOT contain hardcoded absolute paths' ); }); + + test('#1265 records both parent base and plan commit for worktree dispatch', () => { + const step56Start = content.indexOf('Step 5.6'); + const step6Start = content.indexOf('Step 6:', step56Start); + const step56Block = content.slice(step56Start, step6Start); + const step6Block = content.slice(step6Start, content.indexOf('After executor returns:', step6Start)); + + assert.ok( + step56Block.includes('QUICK_PLAN_PARENT'), + 'quick worktree mode must record the pre-dispatch parent SHA before committing PLAN.md (#1265)' + ); + assert.ok( + step56Block.includes('QUICK_PLAN_COMMIT'), + 'quick worktree mode must record the PLAN.md commit SHA after the pre-dispatch commit (#1265)' + ); + assert.ok( + step6Block.includes('EXPECTED_BASE_ALTERNATE'), + 'executor guard embed must carry the alternate accepted base for parent-vs-plan worktree forks (#1265)' + ); + }); + + test('#1265 executor materializes PLAN.md from the plan commit when worktree starts at parent', () => { + const executorTask = content.indexOf('subagent_type="gsd-executor"'); + assert.ok(executorTask !== -1, 'executor Task() spawn must exist'); + const promptStart = content.lastIndexOf('prompt="', executorTask); + const promptEnd = content.indexOf('subagent_type="gsd-executor"', promptStart); + const executorPrompt = content.slice(promptStart, promptEnd); + + assert.ok( + executorPrompt.includes('git show') && executorPrompt.includes('QUICK_PLAN_COMMIT'), + 'executor prompt must materialize PLAN.md from git show : when absent (#1265)' + ); + assert.ok( + executorPrompt.indexOf('git show') < executorPrompt.indexOf(''), + 'PLAN.md materialization must be a first action before files_to_read can prime paths (#1265)' + ); + }); }); diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index 1709816a3..13d07faf6 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -57,7 +57,7 @@ "pr-branch.md": 9561, "profile-user.md": 20650, "progress.md": 29387, - "quick.md": 47251, + "quick.md": 48435, "reapply-patches.md": 20393, "remove-phase.md": 8469, "remove-workspace.md": 7507, diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs index 79c327f3f..5a87ef393 100644 --- a/tests/worktree-cleanup.test.cjs +++ b/tests/worktree-cleanup.test.cjs @@ -689,7 +689,7 @@ describe('bug #3384: worktree cleanup workflow contracts', () => { assert.match(content, /WAVE_WORKTREE_MANIFEST|QUICK_WORKTREE_MANIFEST/); assert.match(content, /worktree\.cleanup-wave/); assert.match(content, /mktemp "\$\{TMPDIR:-\/tmp\}\/gsd-quick-worktree-/); - assert.match(content, /append its returned `\{agent_id, worktree_path, branch, expected_base\}`/); + assert.match(content, /append its returned `\{agent_id, worktree_path, branch, expected_base, allowed_bases\}`/); // After #3797 architectural fix: quick.md delegates entirely to the SDK's cleanup-wave // command (which handles manifest parsing internally). The shell fallback with manual // QUICK_WORKTREE_MANIFEST node-e code is removed — the gsd_run call with || exit 1 is the diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index 172932b86..3eb7044a4 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -518,6 +518,7 @@ describe('planWorktreeWaveCleanup', () => { worktree_path: '/repo/.claude/worktrees/agent-a1', branch: 'worktree-agent-a1', expected_base: 'abc123', + allowed_bases: ['abc123', 'def456'], }, ], }); @@ -527,11 +528,13 @@ describe('planWorktreeWaveCleanup', () => { worktree_path: entry.worktree_path, branch: entry.branch, expected_base: entry.expected_base, + allowed_bases: entry.allowed_bases, })), [{ agent_id: 'a1', worktree_path: '/repo/.claude/worktrees/agent-a1', branch: 'worktree-agent-a1', expected_base: 'abc123', + allowed_bases: ['abc123', 'def456'], }]); assert.equal(plan.discovery, 'manifest'); }); @@ -562,6 +565,84 @@ describe('planWorktreeWaveCleanup', () => { // ─── executeWorktreeWaveCleanupPlan ─────────────────────────────────────────── describe('executeWorktreeWaveCleanupPlan', () => { + test('#1265 accepts a merge-base listed in allowed_bases even when expected_base is the plan commit', () => { + const plan = { + ok: true, + repoRoot: '/repo/main', + action: 'cleanup_wave', + discovery: 'manifest', + entries: [{ + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'plancommit', + allowed_bases: ['plancommit', 'parentcommit'], + }], + }; + const result = executeWorktreeWaveCleanupPlan(plan, { + execGit: (args) => { + const key = args.join(' '); + if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { + return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; + } + if (key === 'merge-base HEAD worktree-agent-a1') { + return { exitCode: 0, stdout: 'parentcommit', stderr: '' }; + } + if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key.startsWith('merge worktree-agent-a1')) { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === 'worktree remove /repo/.claude/worktrees/agent-a1 --force') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === 'branch -D worktree-agent-a1') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }); + + assert.equal(result.ok, true); + assert.equal(result.entries[0].status, 'merged_removed'); + }); + + test('#1265 still blocks a merge-base outside expected_base and allowed_bases', () => { + const plan = { + ok: true, + repoRoot: '/repo/main', + action: 'cleanup_wave', + discovery: 'manifest', + entries: [{ + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'plancommit', + allowed_bases: ['plancommit', 'parentcommit'], + }], + }; + const result = executeWorktreeWaveCleanupPlan(plan, { + execGit: (args) => { + const key = args.join(' '); + if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { + return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; + } + if (key === 'merge-base HEAD worktree-agent-a1') { + return { exitCode: 0, stdout: 'unrelatedbase', stderr: '' }; + } + throw new Error(`unexpected git call after rejected base: ${key}`); + }, + }); + + assert.equal(result.ok, false); + assert.equal(result.entries[0].status, 'blocked'); + assert.equal(result.entries[0].reason, 'base_mismatch'); + }); + test('does not delete a branch when worktree removal fails', () => { const calls = []; const plan = { diff --git a/tests/worktree.test.cjs b/tests/worktree.test.cjs index bcadb25b0..25f73fd83 100644 --- a/tests/worktree.test.cjs +++ b/tests/worktree.test.cjs @@ -176,8 +176,9 @@ describe('canonical worktree-branch-check fragment is the single source of truth assert.ok(block.includes('update-ref'), 'fragment block must reference update-ref prohibition'); }); - test('fragment block asserts exact base and fails closed with exit 42 (#48)', () => { - assert.ok(block.includes('git rev-parse HEAD') && block.includes('{EXPECTED_BASE}'), 'fragment must assert HEAD equals {EXPECTED_BASE} exactly (#48)'); + test('fragment block asserts an allowed base set and fails closed with exit 42 (#48, #1265)', () => { + assert.ok(block.includes('git rev-parse HEAD') && block.includes('{EXPECTED_BASE}'), 'fragment must assert HEAD against {EXPECTED_BASE} (#48)'); + assert.ok(block.includes('{EXPECTED_BASE_ALTERNATE}'), 'fragment must support one orchestrator-approved alternate base for quick parent/plan forks (#1265)'); assert.ok(/exit 42/.test(block), 'fragment must fail closed with exit 42 on mismatch (#48)'); }); }); @@ -848,4 +849,3 @@ done < <(${DISCOVERY_PIPELINE}) }); }); }); - From 9540fe43b974da8e8e83fed2810d0832b8dbf2f8 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 14:19:08 -0400 Subject: [PATCH 2/5] fix: have executor self-report worktree metadata (#1349) --- .changeset/1297-worktree-metadata.md | 6 ++++++ agents/gsd-executor.md | 22 ++++++++++++++++++++++ gsd-core/workflows/execute-phase.md | 3 ++- tests/agent-size-baseline.json | 2 +- tests/workflow-size-baseline.json | 2 +- tests/worktree-cleanup.test.cjs | 23 +++++++++++++++++++++++ 6 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 .changeset/1297-worktree-metadata.md diff --git a/.changeset/1297-worktree-metadata.md b/.changeset/1297-worktree-metadata.md new file mode 100644 index 000000000..c4cb0c5ae --- /dev/null +++ b/.changeset/1297-worktree-metadata.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1349 +--- + +**Parallel worktree execution now has executor-authored cleanup metadata** — executor agents capture their worktree path, branch, and expected base before task commits and return a parseable metadata block for execute-phase to prefer over runtime harness metadata. (#1297) diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index 90739fc76..d86e4a990 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -103,6 +103,24 @@ PLAN_START_EPOCH=$(date +%s) ``` + +If running inside a git worktree, capture authoritative worktree identity before +any task commit changes HEAD. The execute-phase orchestrator consumes this from +your final `` return block to build the wave cleanup manifest +without relying on runtime harness metadata (#1297). + +```bash +GSD_WORKTREE_PATH="" +GSD_WORKTREE_BRANCH="" +GSD_WORKTREE_EXPECTED_BASE="" +if [ -f .git ]; then + GSD_WORKTREE_PATH=$(git rev-parse --show-toplevel) + GSD_WORKTREE_BRANCH=$(git rev-parse --abbrev-ref HEAD) + GSD_WORKTREE_EXPECTED_BASE=$(git rev-parse HEAD) +fi +``` + + ```bash grep -n "type=\"checkpoint" [plan-path] @@ -761,6 +779,10 @@ into the user's project history. **Tasks:** {completed}/{total} **SUMMARY:** {path to SUMMARY.md} + +{"agent_id":"{phase}-{plan}","worktree_path":"${GSD_WORKTREE_PATH:-}","branch":"${GSD_WORKTREE_BRANCH:-}","expected_base":"${GSD_WORKTREE_EXPECTED_BASE:-}"} + + **Commits:** - {hash}: {message} - {hash}: {message} diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index f220d8840..1685336d4 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -635,6 +635,7 @@ increases monotonically across waves. `{status}` is `complete` (success), this commit — the orchestrator force-removes the worktree after you return, and any uncommitted SUMMARY.md will be permanently lost (#2070). REQUIRED ORDER: Write SUMMARY.md → commit → only then any narration. No text between Write and commit (truncation risk; #2070 rescue is not primary defense). + @@ -682,7 +683,7 @@ increases monotonically across waves. `{status}` is `complete` (success), ) ``` - Immediately after each worktree `Agent()` spawn returns metadata, atomically append `{agent_id, worktree_path, branch, expected_base}` to `WAVE_WORKTREE_MANIFEST`. If any field is missing, stop and ask for recovery instead of scanning all agent worktrees. + After each `Agent()` returns, parse executor-returned worktree metadata (``) before harness metadata, then atomically append `{agent_id, worktree_path, branch, expected_base}` to `WAVE_WORKTREE_MANIFEST`. Missing: stop and ask for recovery instead of scanning worktrees. > **Worktree recovery policy (#48 + #1292):** See `execute-phase/steps/worktree-recovery-policy.md` — FAIL-CLOSED rule for base/HEAD-namespace mismatches AND isolated-run fail-safe recovery. diff --git a/tests/agent-size-baseline.json b/tests/agent-size-baseline.json index e320d007c..0c96ebb56 100644 --- a/tests/agent-size-baseline.json +++ b/tests/agent-size-baseline.json @@ -14,7 +14,7 @@ "gsd-domain-researcher.md": 6938, "gsd-eval-auditor.md": 7761, "gsd-eval-planner.md": 7008, - "gsd-executor.md": 42519, + "gsd-executor.md": 43343, "gsd-framework-selector.md": 6778, "gsd-integration-checker.md": 15148, "gsd-intel-updater.md": 18122, diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index 13d07faf6..258c964a0 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -24,7 +24,7 @@ "docs-update.md": 54770, "edit-phase.md": 12883, "eval-review.md": 9923, - "execute-phase.md": 93122, + "execute-phase.md": 93157, "execute-plan.md": 31365, "explore.md": 10497, "extract-learnings.md": 12849, diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs index 5a87ef393..19f45cb53 100644 --- a/tests/worktree-cleanup.test.cjs +++ b/tests/worktree-cleanup.test.cjs @@ -684,6 +684,29 @@ describe('bug #3384: worktree cleanup workflow contracts', () => { assert.doesNotMatch(content, /done < <\(git worktree list --porcelain \| grep "\^worktree " \| grep "\\\.claude\/worktrees\/agent-"/); }); + test('#1297 gsd-executor self-reports authoritative worktree metadata', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf8'); + assert.match(content, //); + assert.match(content, /git rev-parse --show-toplevel/); + assert.match(content, /git rev-parse --abbrev-ref HEAD/); + assert.match(content, /GSD_WORKTREE_EXPECTED_BASE=\$\(git rev-parse HEAD\)/); + assert.match(content, //); + assert.match(content, /"worktree_path":/); + assert.match(content, /"branch":/); + assert.match(content, /"expected_base":/); + }); + + test('#1297 execute-phase consumes executor-returned worktree metadata before harness metadata', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf8'); + assert.match(content, //); + assert.match(content, /executor-returned worktree metadata/i); + assert.match(content, /harness metadata/i); + assert.ok( + content.indexOf('executor-returned worktree metadata') < content.indexOf('harness metadata'), + 'execute-phase must prefer executor-returned worktree metadata before runtime harness metadata (#1297)' + ); + }); + test('quick contract requires a cleanup manifest instead of global worktree discovery', () => { const content = fs.readFileSync(QUICK_PATH, 'utf8'); assert.match(content, /WAVE_WORKTREE_MANIFEST|QUICK_WORKTREE_MANIFEST/); From 284dc7bc44319c7377ebb63c198d73069303e551 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 14:47:17 -0400 Subject: [PATCH 3/5] fix: resume UAT checkpoint from paused placeholder (#1350) --- .changeset/1300-uat-paused-checkpoint.md | 6 ++ src/uat.cts | 64 ++++++++++++++++ tests/uat.test.cjs | 93 ++++++++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 .changeset/1300-uat-paused-checkpoint.md diff --git a/.changeset/1300-uat-paused-checkpoint.md b/.changeset/1300-uat-paused-checkpoint.md new file mode 100644 index 000000000..95c316e56 --- /dev/null +++ b/.changeset/1300-uat-paused-checkpoint.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1350 +--- + +**UAT resume now accepts paused checkpoints** — `uat render-checkpoint` treats a non-structured paused `Current Test` placeholder as a resume signal and derives the checkpoint from the first pending UAT test instead of failing as malformed. (#1300) diff --git a/src/uat.cts b/src/uat.cts index 918948aa8..80d9c7ba2 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -200,6 +200,13 @@ function parseCurrentTest(content: string): CurrentTest { const expectedInlineMatch = section.match(/^expected:\s*(.+)\s*$/m); if (!numberMatch || !nameMatch || (!expectedBlockMatch && !expectedInlineMatch)) { + if (!numberMatch && !nameMatch && !expectedBlockMatch && !expectedInlineMatch) { + const pendingTest = parseFirstPendingTest(content); + if (pendingTest) { + return pendingTest; + } + error('Current Test section is non-structured and no pending UAT test remains to resume'); + } error('Current Test section is malformed'); } @@ -222,6 +229,63 @@ function parseCurrentTest(content: string): CurrentTest { }; } +function parseFirstPendingTest(content: string): CurrentTest | null { + const testsMatch = content.match(/##\s*Tests\s*\n([\s\S]*?)(?=\n##\s|$)/i); + if (!testsMatch) { + return null; + } + + const testsSection = testsMatch[1]; + const headingPattern = /^###\s*(\d+)\.\s*([^\n]+)\s*$/gm; + const headings: Array<{ index: number; number: number; name: string }> = []; + let headingMatch: RegExpExecArray | null; + while ((headingMatch = headingPattern.exec(testsSection)) !== null) { + headings.push({ + index: headingMatch.index, + number: parseInt(headingMatch[1], 10), + name: headingMatch[2].trim(), + }); + } + + for (let i = 0; i < headings.length; i += 1) { + const current = headings[i]; + const next = headings[i + 1]; + const block = testsSection.slice(current.index, next ? next.index : undefined); + if (!/^result:\s*\[?pending\]?\s*$/im.test(block)) { + continue; + } + + const expected = parseExpectedFromTestBlock(block); + if (!expected) { + error(`Pending UAT test ${current.number} is missing an expected field`); + } + + return { + complete: false, + number: current.number, + name: sanitizeForDisplay(current.name), + expected: sanitizeForDisplay(expected), + }; + } + + return null; +} + +function parseExpectedFromTestBlock(block: string): string | null { + const expectedBlockMatch = block.match(/^expected:\s*\|\n([\s\S]*?)(?=^\w[\w-]*:\s)/m) + || block.match(/^expected:\s*\|\n([\s\S]+)/m); + if (expectedBlockMatch) { + return expectedBlockMatch[1] + .split('\n') + .map((line: string) => line.replace(/^ {2}/, '')) + .join('\n') + .trim(); + } + + const expectedInlineMatch = block.match(/^expected:\s*(.+)\s*$/m); + return expectedInlineMatch ? expectedInlineMatch[1].trim() : null; +} + // ─── buildCheckpoint ────────────────────────────────────────────────────────── function buildCheckpoint(currentTest: { number: number; name: string; expected: string }): string { diff --git a/tests/uat.test.cjs b/tests/uat.test.cjs index 3f25601ca..2db573b40 100644 --- a/tests/uat.test.cjs +++ b/tests/uat.test.cjs @@ -533,6 +533,99 @@ expected: | assert.ok(result.output.includes('It ends at the section boundary.')); }); + test('resumes paused Current Test placeholder from first pending test (#1300)', () => { + fs.writeFileSync(uatPath, [ + '---', + 'status: partial', + 'phase: 01-test-phase', + 'started: 2026-06-15T00:00:00Z', + 'updated: 2026-06-15T00:00:00Z', + '---', + '', + '## Current Test', + '', + '[testing paused — 2 items outstanding]', + '', + '## Tests', + '', + '### 1. First test', + 'expected: something observable', + 'result: pass', + '', + '### 2. Second test', + 'expected: another observable thing', + 'result: [pending]', + '', + '## Summary', + '', + 'total: 2', + 'passed: 1', + 'issues: 0', + 'pending: 1', + 'skipped: 0', + 'blocked: 0', + '', + '## Gaps', + '', + '[none yet]', + ].join('\n')); + + const result = runGsdTools(['uat', 'render-checkpoint', '--file', '.planning/phases/01-test-phase/01-UAT.md'], tmpDir); + assert.strictEqual(result.success, true, `render-checkpoint failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.test_number, 2); + assert.strictEqual(output.test_name, 'Second test'); + assert.strictEqual(output.file_path, '.planning/phases/01-test-phase/01-UAT.md'); + }); + + test('raw checkpoint mode accepts paused Current Test placeholder (#1300)', () => { + fs.writeFileSync(uatPath, [ + '---', + 'status: partial', + 'phase: 01-test-phase', + '---', + '', + '## Current Test', + '', + '[testing paused — 1 item outstanding]', + '', + '## Tests', + '', + '### 1. First pending test', + 'expected: raw mode checkpoint is available', + 'result: [pending]', + ].join('\n')); + + const result = runGsdTools(['uat', 'render-checkpoint', '--file', '.planning/phases/01-test-phase/01-UAT.md', '--raw'], tmpDir); + assert.strictEqual(result.success, true, `render-checkpoint failed: ${result.error}`); + assert.ok(result.output.length > 0, 'raw mode must emit a checkpoint'); + }); + + test('non-structured Current Test with no pending tests reports actionable resume error (#1300)', () => { + fs.writeFileSync(uatPath, [ + '---', + 'status: partial', + 'phase: 01-test-phase', + '---', + '', + '## Current Test', + '', + '[testing paused — 0 items outstanding]', + '', + '## Tests', + '', + '### 1. Already handled test', + 'expected: completed behavior', + 'result: pass', + ].join('\n')); + + const result = runGsdTools(['uat', 'render-checkpoint', '--file', '.planning/phases/01-test-phase/01-UAT.md'], tmpDir); + assert.strictEqual(result.success, false, 'Should fail when a paused placeholder has no pending test to resume'); + assert.ok(result.error.includes('no pending UAT test remains')); + assert.ok(!result.error.includes('Current Test section is malformed')); + }); + test('fails when testing is already complete', () => { fs.writeFileSync(uatPath, `--- status: complete From c20d741dc9560d5ef1d308831cf82d4d69632d5c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 15:11:23 -0400 Subject: [PATCH 4/5] fix(#1316): preserve prose STATE phase names (#1351) --- .changeset/1316-state-prose-phase-name.md | 6 ++ src/phase.cts | 60 ++++++++----- src/state.cts | 59 ++++++++++--- tests/phase.test.cjs | 100 +++++++++++++++++++++- tests/state.test.cjs | 8 +- 5 files changed, 194 insertions(+), 39 deletions(-) create mode 100644 .changeset/1316-state-prose-phase-name.md diff --git a/.changeset/1316-state-prose-phase-name.md b/.changeset/1316-state-prose-phase-name.md new file mode 100644 index 000000000..1757416e1 --- /dev/null +++ b/.changeset/1316-state-prose-phase-name.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1351 +--- + +**`phase complete` now preserves prose-block STATE phase names** — template-shaped `Current Position` prose now advances with the next phase name, avoids missing-field warnings, and keeps `Last activity:` on the template em-dash delimiter. (#1316) diff --git a/src/phase.cts b/src/phase.cts index 576c1ccc6..832f0444e 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1342,6 +1342,21 @@ function writePlanningFileSet(writes: WriteSpec[]): void { } } +function phaseDisplayNameFromRoadmap(roadmapContent: string | null, phaseNum: string | null): string | null { + if (!roadmapContent || !phaseNum) return null; + const phaseEscaped = phaseMarkdownRegexSource(phaseNum); + const heading = roadmapContent.match(new RegExp(`^#{2,4}\\s*Phase\\s+${phaseEscaped}\\s*:\\s*([^\\n]+)`, 'im')); + if (!heading) return null; + const name = heading[1].replace(/\(INSERTED\)/i, '').trim(); + return name || null; +} + +function phaseDisplayNameFromSlug(slug: string | null): string | null { + if (!slug) return null; + const name = slug.replace(/-/g, ' ').trim(); + return name || null; +} + function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { if (!phaseNum) { error('phase number required for phase complete'); @@ -1642,6 +1657,9 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { let stateContent = originalStateContent; const phaseValue = nextPhaseNum || phaseNum; + const nextPhaseDisplayName = + phaseDisplayNameFromRoadmap(roadmapContent, nextPhaseNum) ?? + phaseDisplayNameFromSlug(nextPhaseName); const existingPhaseField = stateExtractField(stateContent, 'Current Phase') || stateExtractField(stateContent, 'Phase'); @@ -1651,12 +1669,14 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { const nameMatch = existingPhaseField.match(/\(([^)]+)\)/); if (totalMatch) { const total = totalMatch[1]; - const nameStr = nextPhaseName - ? ` (${nextPhaseName.replace(/-/g, ' ')})` + const nameStr = nextPhaseDisplayName + ? ` (${nextPhaseDisplayName})` : nameMatch ? ` (${nameMatch[1]})` : ''; newPhaseValue = `${phaseValue} of ${total}${nameStr}`; + } else if (nextPhaseDisplayName) { + newPhaseValue = `${phaseValue} — ${nextPhaseDisplayName}`; } } stateContent = stateReplaceFieldWithFallback( @@ -1666,13 +1686,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { newPhaseValue, ); - if (nextPhaseName) { - stateContent = stateReplaceFieldWithFallback( - stateContent, - 'Current Phase Name', - null, - nextPhaseName.replace(/-/g, ' '), - ); + if (nextPhaseDisplayName) { + stateContent = + stateReplaceField(stateContent, 'Current Phase Name', nextPhaseDisplayName) || + stateContent; } stateContent = stateReplaceFieldWithFallback( @@ -1689,19 +1706,20 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { 'Not started', ); - stateContent = stateReplaceFieldWithFallback( - stateContent, - 'Last Activity', - 'Last activity', - today, - ); + const lastActivityDescription = `Phase ${phaseNum} complete${nextPhaseNum ? `, transitioned to Phase ${nextPhaseNum}` : ''}`; + if (/^Last activity:/m.test(stateContent)) { + stateContent = + stateReplaceField(stateContent, 'Last activity', `${today} — ${lastActivityDescription}`) || + stateContent; + } else { + stateContent = + stateReplaceField(stateContent, 'Last Activity', today) || + stateContent; + } - stateContent = stateReplaceFieldWithFallback( - stateContent, - 'Last Activity Description', - null, - `Phase ${phaseNum} complete${nextPhaseNum ? `, transitioned to Phase ${nextPhaseNum}` : ''}`, - ); + stateContent = + stateReplaceField(stateContent, 'Last Activity Description', lastActivityDescription) || + stateContent; const completedRaw = stateExtractField(stateContent, 'Completed Phases'); if (completedRaw !== null) { diff --git a/src/state.cts b/src/state.cts index d51a0bf50..a950540b0 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1050,6 +1050,31 @@ function matchSessionSection(body: string): RegExpMatchArray | null { || body.match(/(?:^|\n)##[ \t]*Session Continuity[ \t]*\n([\s\S]*?)(?=\n##|$)/i); } +function parseProsePhaseField(value: string | null): { phase: string | null; name: string | null } { + if (!value) return { phase: null, name: null }; + const phaseMatch = value.match(/\b(\d+[A-Z]?(?:\.\d+)*)\b/i); + const parenName = value.match(/\(([^)]+)\)/); + const dashName = value.match(/—\s*([^(\n]+?)(?:\s*\(|$)/); + const rawName = parenName?.[1] ?? dashName?.[1] ?? null; + const name = rawName && !/^(?:complete|executing|not started)$/i.test(rawName.trim()) + ? rawName.trim() + : null; + return { + phase: phaseMatch ? phaseMatch[1] : null, + name, + }; +} + +function parseProseLastActivityField(value: string | null): { date: string | null; description: string | null } { + if (!value) return { date: null, description: null }; + const match = value.match(/^(\d{4}-\d{2}-\d{2})(?:\s+[—-]{1,2}\s+(.+))?$/); + if (!match) return { date: value, description: null }; + return { + date: match[1], + description: match[2]?.trim() || null, + }; +} + function cmdStateSnapshot(cwd: string, raw: boolean): void { const statePath = planningPaths(cwd).state; @@ -1080,15 +1105,18 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void { }; // Extract basic fields — frontmatter keys take precedence over body - const currentPhase = fmScalar('current_phase') ?? stateExtractField(body, 'Current Phase'); - const currentPhaseName = fmScalar('current_phase_name') ?? stateExtractField(body, 'Current Phase Name'); + const prosePhase = parseProsePhaseField(stateExtractField(body, 'Phase')); + const currentPhase = fmScalar('current_phase') ?? stateExtractField(body, 'Current Phase') ?? prosePhase.phase; + const currentPhaseName = fmScalar('current_phase_name') ?? stateExtractField(body, 'Current Phase Name') ?? prosePhase.name; const totalPhasesRaw = fmScalar('total_phases') ?? stateExtractField(body, 'Total Phases'); const currentPlan = fmScalar('current_plan') ?? stateExtractField(body, 'Current Plan'); const totalPlansRaw = fmScalar('total_plans_in_phase') ?? stateExtractField(body, 'Total Plans in Phase'); const status = fmScalar('status') ?? stateExtractField(body, 'Status'); const progressRaw = fmScalar('progress') ?? stateExtractField(body, 'Progress'); - const lastActivity = fmScalar('last_activity') ?? stateExtractField(body, 'Last Activity'); - const lastActivityDesc = fmScalar('last_activity_desc') ?? stateExtractField(body, 'Last Activity Description'); + const rawLastActivity = stateExtractField(body, 'Last Activity') ?? stateExtractField(body, 'Last activity'); + const proseLastActivity = parseProseLastActivityField(rawLastActivity); + const lastActivity = fmScalar('last_activity') ?? proseLastActivity.date ?? rawLastActivity; + const lastActivityDesc = fmScalar('last_activity_desc') ?? stateExtractField(body, 'Last Activity Description') ?? proseLastActivity.description; const pausedAt = fmScalar('paused_at') ?? stateExtractField(body, 'Paused At'); // Parse numeric fields @@ -1180,14 +1208,18 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void { * reliably via `state json` instead of fragile regex parsing. */ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Record { - const currentPhase = stateExtractField(bodyContent, 'Current Phase'); - const currentPhaseName = stateExtractField(bodyContent, 'Current Phase Name'); + const prosePhase = parseProsePhaseField(stateExtractField(bodyContent, 'Phase')); + const currentPhase = stateExtractField(bodyContent, 'Current Phase') ?? prosePhase.phase; + const currentPhaseName = stateExtractField(bodyContent, 'Current Phase Name') ?? prosePhase.name; const currentPlan = stateExtractField(bodyContent, 'Current Plan'); const totalPhasesRaw = stateExtractField(bodyContent, 'Total Phases'); const totalPlansRaw = stateExtractField(bodyContent, 'Total Plans in Phase'); const status = stateExtractField(bodyContent, 'Status'); const progressRaw = stateExtractField(bodyContent, 'Progress'); - const lastActivity = stateExtractField(bodyContent, 'Last Activity'); + const rawLastActivity = stateExtractField(bodyContent, 'Last Activity') ?? stateExtractField(bodyContent, 'Last activity'); + const proseLastActivity = parseProseLastActivityField(rawLastActivity); + const lastActivity = proseLastActivity.date ?? rawLastActivity; + const lastActivityDesc = stateExtractField(bodyContent, 'Last Activity Description') ?? proseLastActivity.description; // Bug #2444: scope Stopped At extraction to the ## Session section so that // historical "Stopped at:" prose elsewhere in the body (e.g. in a // Session Continuity Archive section) never overwrites the current value. @@ -1326,6 +1358,7 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re if (pausedAt) fm['paused_at'] = pausedAt; fm['last_updated'] = realClock.nowIso(); if (lastActivity) fm['last_activity'] = lastActivity; + if (lastActivityDesc) fm['last_activity_desc'] = lastActivityDesc; const progress: Record = {}; if (totalPhases !== null) progress['total_phases'] = totalPhases; @@ -1889,13 +1922,13 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName } // Update Last activity line if present - const newActivity = `Last activity: ${today} -- Phase ${phaseNumber} execution started`; + const newActivity = `Last activity: ${today} — Phase ${phaseNumber} execution started`; if (/^Last activity:/im.test(posBody)) { posBody = posBody.replace(/^Last activity:.*$/im, newActivity); } else { // Pipe-table format in Current Position (#1255) // Value must match the inline branch (date + narrative), not bare date. - const activityValue = `${today} -- Phase ${phaseNumber} execution started`; + const activityValue = `${today} — Phase ${phaseNumber} execution started`; const replaced = stateReplaceField(posBody, 'Last Activity', activityValue) ?? stateReplaceField(posBody, 'Last activity', activityValue); if (replaced !== null) posBody = replaced; @@ -1912,7 +1945,7 @@ function cmdStateBeginPhase(cwd: string, phaseNumber: string | number, phaseName if (positionMatch) { const header = positionMatch[1]; let posBody = positionMatch[2]; - const resumeActivity = `Last activity: ${today} -- Phase ${phaseNumber} execution resumed (wave continue)`; + const resumeActivity = `Last activity: ${today} — Phase ${phaseNumber} execution resumed (wave continue)`; if (/^Last activity:/im.test(posBody)) { posBody = posBody.replace(/^Last activity:.*$/im, resumeActivity); body = body.replace(positionPattern, () => `${header}${posBody}`); @@ -2083,7 +2116,7 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number, planCou // Update Current Position section body = updateCurrentPositionFields(body, { status: 'Ready to execute', - lastActivity: `${today} -- Phase ${phaseNumber} planning complete`, + lastActivity: `${today} — Phase ${phaseNumber} planning complete`, }); return reassemble(body); @@ -2659,13 +2692,13 @@ function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string } // Update Last activity line if present - const newActivity = `Last activity: ${today} -- Phase ${currentPhase} marked complete`; + const newActivity = `Last activity: ${today} — Phase ${currentPhase} marked complete`; if (/^Last activity:/im.test(posBody)) { posBody = posBody.replace(/^Last activity:.*$/im, newActivity); } else { // Pipe-table format in Current Position (#1255) // Value must match the inline branch (date + narrative), not bare date. - const activityValue = `${today} -- Phase ${currentPhase} marked complete`; + const activityValue = `${today} — Phase ${currentPhase} marked complete`; const replaced = stateReplaceField(posBody, 'Last Activity', activityValue) ?? stateReplaceField(posBody, 'Last activity', activityValue); if (replaced !== null) posBody = replaced; diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 2c0c7a2f8..204557c45 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -19,7 +19,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { execFileSync, spawnSync } = require('node:child_process'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); const GSD_TOOLS_BIN = path.resolve(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -4279,6 +4279,71 @@ describe('bug-3287 — init plan-phase exposes expected_phase_dir with project_c return { planningDir, phase5Dir }; } + function setupPhase1316Project(tmpDir) { + const planningDir = path.join(tmpDir, '.planning'); + const phasesDir = path.join(planningDir, 'phases'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.mkdirSync(phasesDir, { recursive: true }); + + fs.writeFileSync( + path.join(planningDir, 'ROADMAP.md'), + [ + '# Roadmap', + '', + '## Current Milestone: v3.0', + '', + '- [ ] Phase 32: Backlog-Closeout Lib Extraction', + '- [ ] Phase 33: Follow Up Implementation', + '', + '### Phase 32: Backlog-Closeout Lib Extraction', + '**Goal:** Complete closeout extraction', + '**Plans:** 1 plans', + '', + '### Phase 33: Follow Up Implementation', + '**Goal:** Continue implementation', + ].join('\n'), + ); + + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'status: executing', + 'current_phase: "32"', + 'last_activity: "2026-06-14"', + 'progress:', + ' total_phases: 2', + ' completed_phases: 0', + ' total_plans: 1', + ' completed_plans: 0', + ' percent: 0', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 32 — Backlog-Closeout Lib Extraction', + 'Plan: 1 of 1', + 'Status: Executing Phase 32', + 'Last activity: 2026-06-14 — recorded planning complete', + '', + '## Session', + '', + 'Last session: 2026-06-14T00:00:00.000Z', + ].join('\n'), + ); + + const phase32Dir = path.join(phasesDir, '32-backlog-closeout-lib-extraction'); + fs.mkdirSync(phase32Dir, { recursive: true }); + fs.writeFileSync(path.join(phase32Dir, '32-01-PLAN.md'), '# Plan', 'utf8'); + fs.writeFileSync(path.join(phase32Dir, '32-01-SUMMARY.md'), '# Summary', 'utf8'); + fs.mkdirSync(path.join(phasesDir, '33-follow-up-implementation'), { recursive: true }); + + return { planningDir }; + } + describe('bug #3517: phase.complete leaves STATE.md with stale fields', () => { let tmpDir; @@ -4434,6 +4499,39 @@ describe('bug-3287 — init plan-phase exposes expected_phase_dir with project_c assert.match(state, /Phase:\s*0?6\b/, 'numeric Phase line should advance to phase 6'); }); + test('prose-block STATE keeps next phase name without field-miss warnings (#1316)', () => { + const { planningDir } = setupPhase1316Project(tmpDir); + + const result = spawnSync(process.execPath, [GSD_TOOLS_BIN, 'phase', 'complete', '32'], { + cwd: tmpDir, + encoding: 'utf8', + env: process.env, + }); + + assert.strictEqual(result.status, 0, `phase complete failed: ${result.stderr || result.stdout}`); + assert.ok( + !result.stderr.includes('Current Phase Name'), + `phase.complete must not warn about missing Current Phase Name on prose-block STATE.md; stderr:\n${result.stderr}`, + ); + assert.ok( + !result.stderr.includes('Last Activity Description'), + `phase.complete must not warn about missing Last Activity Description on prose-block STATE.md; stderr:\n${result.stderr}`, + ); + + const state = fs.readFileSync(path.join(planningDir, 'STATE.md'), 'utf8'); + assert.match(state, /current_phase:\s*"?33"?/, 'current_phase frontmatter must advance to 33'); + assert.match( + state, + /^Phase:\s*33\s+—\s+Follow Up Implementation\b/m, + `Current Position Phase line must keep the next phase name; state:\n${state}`, + ); + assert.match( + state, + /^Last activity:\s*\d{4}-\d{2}-\d{2}\s+—\s+Phase 32 complete/m, + `Last activity line must use the template em-dash delimiter with narrative; state:\n${state}`, + ); + }); + test('body By Phase table row for completed phase shows correct plan count', () => { setupPhase3517Project(tmpDir); const statePath = path.join(tmpDir, '.planning', 'STATE.md'); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 4c7098306..8306e8c3d 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -3825,8 +3825,8 @@ describe('#1255 — begin/complete-phase advance status for pipe-table STATE.md' // Last activity cell must include date + narrative (not bare date) assert.ok( - /\|\s*Last activity\s*\|[^|]*--\s*Phase 1 execution started\s*\|/i.test(cpSection), - `Current Position Last activity cell must include narrative '-- Phase 1 execution started'; got Current Position:\n${cpSection}` + /\|\s*Last activity\s*\|[^|]*—\s*Phase 1 execution started\s*\|/i.test(cpSection), + `Current Position Last activity cell must include narrative '— Phase 1 execution started'; got Current Position:\n${cpSection}` ); } finally { cleanup(dir); @@ -3909,8 +3909,8 @@ describe('#1255 — begin/complete-phase advance status for pipe-table STATE.md' // Bug 2: Last activity cell must include date + narrative (not bare date) assert.ok( - /\|\s*Last activity\s*\|[^|]*--\s*Phase 1 marked complete\s*\|/i.test(cpSection), - `Current Position Last activity cell must include narrative '-- Phase 1 marked complete'; got Current Position:\n${cpSection}` + /\|\s*Last activity\s*\|[^|]*—\s*Phase 1 marked complete\s*\|/i.test(cpSection), + `Current Position Last activity cell must include narrative '— Phase 1 marked complete'; got Current Position:\n${cpSection}` ); } finally { cleanup(dir); From a0dbf8bbdf3853eef156fa00d1de5f1475f8c337 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 15:30:17 -0400 Subject: [PATCH 5/5] fix(#1319): use portable Claude skill effort (#1352) --- .changeset/1319-claude-skill-effort-max.md | 6 ++ bin/install.js | 6 +- commands/gsd/autonomous.md | 2 +- commands/gsd/execute-phase.md | 2 +- commands/gsd/plan-phase.md | 2 +- docs/COMMANDS.md | 2 +- docs/explanation/context-engineering.md | 2 +- src/runtime-artifact-conversion.cts | 6 +- ...h-769-context-fork-effort.install.test.cjs | 82 +++++++++++-------- 9 files changed, 69 insertions(+), 41 deletions(-) create mode 100644 .changeset/1319-claude-skill-effort-max.md diff --git a/.changeset/1319-claude-skill-effort-max.md b/.changeset/1319-claude-skill-effort-max.md new file mode 100644 index 000000000..ff4aa71ed --- /dev/null +++ b/.changeset/1319-claude-skill-effort-max.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1352 +--- + +**Claude skill installs now avoid rejected `xhigh` effort frontmatter** — heavyweight GSD skills now ship with portable `effort: max`, and the Claude skill converter normalizes any remaining `xhigh` source effort before writing `SKILL.md`. (#1319) diff --git a/bin/install.js b/bin/install.js index 2ebccf1e4..9c71521ee 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1796,6 +1796,10 @@ function skillFrontmatterName(skillDirName) { return skillDirName; } +function normalizeClaudeSkillEffort(effort) { + return effort === 'xhigh' ? 'max' : effort; +} + /** * Qwen Code skills accept an optional numeric `priority` frontmatter field. * Per the Qwen skills spec (qwen-code/docs/users/features/skills.md, verified @@ -1892,7 +1896,7 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c // token-budget tier). Fields are Claude-specific; unknown frontmatter // fields are silently ignored by other runtimes (backward-compatible). if (context) fm += `context: ${context}\n`; - if (effort) fm += `effort: ${effort}\n`; + if (effort) fm += `effort: ${normalizeClaudeSkillEffort(effort)}\n`; if (toolsBlock) fm += toolsBlock; fm += '---'; diff --git a/commands/gsd/autonomous.md b/commands/gsd/autonomous.md index b9aaca7cf..1d830dd6b 100644 --- a/commands/gsd/autonomous.md +++ b/commands/gsd/autonomous.md @@ -2,7 +2,7 @@ name: gsd:autonomous description: Run all remaining phases autonomously — discuss→plan→execute per phase argument-hint: "[--from N] [--to N] [--only N] [--interactive] [--converge]" -effort: xhigh +effort: max allowed-tools: - Read - Write diff --git a/commands/gsd/execute-phase.md b/commands/gsd/execute-phase.md index c7eb7bdca..5f0ed2c87 100644 --- a/commands/gsd/execute-phase.md +++ b/commands/gsd/execute-phase.md @@ -2,7 +2,7 @@ name: gsd:execute-phase description: Execute all plans in a phase with wave-based parallelization argument-hint: " [--wave N] [--gaps-only] [--interactive] [--tdd]" -effort: xhigh +effort: max allowed-tools: - Read - Write diff --git a/commands/gsd/plan-phase.md b/commands/gsd/plan-phase.md index d4f071145..61396ce2e 100644 --- a/commands/gsd/plan-phase.md +++ b/commands/gsd/plan-phase.md @@ -2,7 +2,7 @@ name: gsd:plan-phase description: Create detailed phase plan (PLAN.md) with verification loop argument-hint: "[phase] [--auto] [--research] [--skip-research] [--research-phase ] [--view] [--gaps] [--skip-verify] [--prd ] [--ingest ] [--ingest-format ] [--reviews] [--text] [--tdd] [--mvp]" -effort: xhigh +effort: max allowed-tools: - Read - Write diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index a05897c3e..b5511fcb5 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -14,7 +14,7 @@ The hyphen and colon forms are *runtime-specific spellings of the same command*. ### Skill Runtime Behavior (Claude Code) -Heavy workflow skills (`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous`) declare `effort: xhigh`, signalling maximum token budget to the runtime. These skills are spawning orchestrators — they must run at top level so they retain the `Agent` tool needed to spawn subagents. They do **not** carry `context: fork` (see #921). +Heavy workflow skills (`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous`) declare `effort: max`, signalling maximum token budget to the runtime. These skills are spawning orchestrators — they must run at top level so they retain the `Agent` tool needed to spawn subagents. They do **not** carry `context: fork` (see #921). Quick-status skills (`/gsd-progress`, `/gsd-stats`) declare `effort: low`, directing the runtime to use a minimal token budget for fast reads. diff --git a/docs/explanation/context-engineering.md b/docs/explanation/context-engineering.md index 77869c33c..a9a84a239 100644 --- a/docs/explanation/context-engineering.md +++ b/docs/explanation/context-engineering.md @@ -92,7 +92,7 @@ Requiring a `/clear` to pick up a config edit would destroy the very continuity ### Effort signals for heavy and light skills -Beyond passive monitoring, GSD uses `effort:` frontmatter to signal the token budget appropriate for each skill. Heavy orchestrator skills (`plan-phase`, `execute-phase`, `autonomous`) declare `effort: xhigh`; quick-status skills (`progress`, `stats`) declare `effort: low`. +Beyond passive monitoring, GSD uses `effort:` frontmatter to signal the token budget appropriate for each skill. Heavy orchestrator skills (`plan-phase`, `execute-phase`, `autonomous`) declare `effort: max`; quick-status skills (`progress`, `stats`) declare `effort: low`. Note: an earlier version of GSD also applied `context: fork` to these three heavy skills to protect the main session's context budget. This was removed (#921) because `plan-phase`, `execute-phase`, and `autonomous` are **spawning orchestrators** — their core function is to spawn subagents (`gsd-planner`, `gsd-executor`, etc.), and a forked subagent context does not have the `Agent` tool. Context isolation for these skills comes from the subagents they spawn, not from forking the orchestrator itself. diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index a3f998897..fd9a8072a 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -309,6 +309,10 @@ function skillFrontmatterName(skillDirName) { return skillDirName; } +function normalizeClaudeSkillEffort(effort) { + return effort === 'xhigh' ? 'max' : effort; +} + /** * Qwen Code skills accept an optional numeric `priority` frontmatter field. * Per the Qwen skills spec (qwen-code/docs/users/features/skills.md, verified @@ -405,7 +409,7 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c // token-budget tier). Fields are Claude-specific; unknown frontmatter // fields are silently ignored by other runtimes (backward-compatible). if (context) fm += `context: ${context}\n`; - if (effort) fm += `effort: ${effort}\n`; + if (effort) fm += `effort: ${normalizeClaudeSkillEffort(effort)}\n`; if (toolsBlock) fm += toolsBlock; fm += '---'; diff --git a/tests/enh-769-context-fork-effort.install.test.cjs b/tests/enh-769-context-fork-effort.install.test.cjs index c2c69b5ff..f552345b9 100644 --- a/tests/enh-769-context-fork-effort.install.test.cjs +++ b/tests/enh-769-context-fork-effort.install.test.cjs @@ -10,23 +10,23 @@ * Context: context:fork was added by #769 to protect context budget, but * plan-phase, execute-phase, and autonomous are spawning orchestrators — a * forked subagent has no Agent/Task tool, breaking their core function. - * effort: xhigh is preserved; context: fork is removed from these three. + * effort: max is preserved; context: fork is removed from these three. * The converter still passes context: fork through if a source file has it * (for any future leaf skill that legitimately needs isolation). * * Verifies: - * 1. Source commands/gsd/autonomous.md does NOT have context: fork, has effort: xhigh - * 2. Source commands/gsd/execute-phase.md does NOT have context: fork, has effort: xhigh - * 3. Source commands/gsd/plan-phase.md does NOT have context: fork, has effort: xhigh + * 1. Source commands/gsd/autonomous.md does NOT have context: fork, has effort: max + * 2. Source commands/gsd/execute-phase.md does NOT have context: fork, has effort: max + * 3. Source commands/gsd/plan-phase.md does NOT have context: fork, has effort: max * 4. Source commands/gsd/progress.md has effort: low * 5. Source commands/gsd/stats.md has effort: low - * 6. Claude global install: SKILL.md for autonomous has effort: xhigh, NOT context: fork - * 7. Claude global install: SKILL.md for execute-phase has effort: xhigh, NOT context: fork - * 8. Claude global install: SKILL.md for plan-phase has effort: xhigh, NOT context: fork + * 6. Claude global install: SKILL.md for autonomous has effort: max, NOT context: fork + * 7. Claude global install: SKILL.md for execute-phase has effort: max, NOT context: fork + * 8. Claude global install: SKILL.md for plan-phase has effort: max, NOT context: fork * 9. Claude global install: SKILL.md for progress has effort: low * 10. Claude global install: SKILL.md for stats has effort: low * 11. convertClaudeCommandToClaudeSkill still passes context: fork through (for non-orchestrator skills) - * 12. convertClaudeCommandToClaudeSkill preserves effort: field + * 12. convertClaudeCommandToClaudeSkill emits portable effort: field values */ 'use strict'; @@ -107,18 +107,20 @@ function runClaudeGlobalInstall(claudeHome) { // #921/#922: spawning orchestrators must NOT carry context: fork — a forked // subagent has no Agent/Task tool, making it impossible for orchestrators to // spawn their required subagents. context: fork is appropriate only for leaf -// skills that do not themselves dispatch agents. effort: xhigh is preserved. -describe('#769/#921 source commands: spawning orchestrators have effort: xhigh but NOT context: fork', () => { +// skills that do not themselves dispatch agents. effort: max is portable across Claude Code models. +describe('#769/#921/#1319 source commands: spawning orchestrators have effort: max but NOT context: fork', () => { test('commands/gsd/autonomous.md does NOT have context: fork (#921)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'autonomous.md')); assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, `autonomous.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('commands/gsd/autonomous.md has effort: xhigh', () => { + test('commands/gsd/autonomous.md has effort: max (#1319)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'autonomous.md')); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `autonomous.md frontmatter must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `autonomous.md frontmatter must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `autonomous.md frontmatter must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); test('commands/gsd/execute-phase.md does NOT have context: fork (#921)', () => { @@ -127,10 +129,12 @@ describe('#769/#921 source commands: spawning orchestrators have effort: xhigh b `execute-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('commands/gsd/execute-phase.md has effort: xhigh', () => { + test('commands/gsd/execute-phase.md has effort: max (#1319)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'execute-phase.md')); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `execute-phase.md frontmatter must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `execute-phase.md frontmatter must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `execute-phase.md frontmatter must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); test('commands/gsd/plan-phase.md does NOT have context: fork (#921)', () => { @@ -139,10 +143,12 @@ describe('#769/#921 source commands: spawning orchestrators have effort: xhigh b `plan-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('commands/gsd/plan-phase.md has effort: xhigh', () => { + test('commands/gsd/plan-phase.md has effort: max (#1319)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'plan-phase.md')); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `plan-phase.md frontmatter must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `plan-phase.md frontmatter must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `plan-phase.md frontmatter must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); }); @@ -162,7 +168,7 @@ describe('#769 source commands: quick-status skills have effort: low', () => { // ─── describe 2: convertClaudeCommandToClaudeSkill preserves new fields ─────── -describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort fields', () => { +describe('#769/#1319 convertClaudeCommandToClaudeSkill: preserves context and emits portable effort fields', () => { test('preserves context: fork in emitted SKILL.md frontmatter', () => { const input = [ '---', @@ -186,7 +192,7 @@ describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort f `SKILL.md frontmatter must include context: fork\nActual frontmatter:\n${fm}`); }); - test('preserves effort: xhigh in emitted SKILL.md frontmatter', () => { + test('normalizes effort: xhigh to effort: max in emitted SKILL.md frontmatter (#1319)', () => { const input = [ '---', 'name: gsd:test-heavy', @@ -205,8 +211,10 @@ describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort f const end = result.indexOf('---', 3); const fm = result.substring(3, end); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `SKILL.md frontmatter must include effort: xhigh\nActual frontmatter:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `SKILL.md frontmatter must include portable effort: max\nActual frontmatter:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `SKILL.md frontmatter must not include rejected effort: xhigh (#1319)\nActual frontmatter:\n${fm}`); }); test('preserves effort: low in emitted SKILL.md frontmatter', () => { @@ -256,8 +264,8 @@ describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort f // ─── describe 3: Claude global install — SKILL.md files include new fields ──── // #921/#922: after install, spawning orchestrators must NOT carry context: fork -// in their emitted SKILL.md. effort: xhigh is still emitted (preserved from source). -describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files have effort: xhigh but NOT context: fork', () => { +// in their emitted SKILL.md. #1319: heavyweight skills must use portable max effort. +describe('#769/#921/#1319 Claude global install: spawning-orchestrator SKILL.md files have effort: max but NOT context: fork', () => { let tmpDir; let claudeHome; @@ -279,12 +287,14 @@ describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files `gsd-autonomous is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('gsd-autonomous SKILL.md has effort: xhigh after global install', () => { + test('gsd-autonomous SKILL.md has effort: max after global install (#1319)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'autonomous'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `gsd-autonomous SKILL.md must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `gsd-autonomous SKILL.md must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `gsd-autonomous SKILL.md must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); test('gsd-execute-phase SKILL.md does NOT have context: fork after global install (#921)', () => { @@ -295,12 +305,14 @@ describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files `gsd-execute-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('gsd-execute-phase SKILL.md has effort: xhigh after global install', () => { + test('gsd-execute-phase SKILL.md has effort: max after global install (#1319)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'execute-phase'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `gsd-execute-phase SKILL.md must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `gsd-execute-phase SKILL.md must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `gsd-execute-phase SKILL.md must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); test('gsd-plan-phase SKILL.md does NOT have context: fork after global install (#921)', () => { @@ -311,12 +323,14 @@ describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files `gsd-plan-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); - test('gsd-plan-phase SKILL.md has effort: xhigh after global install', () => { + test('gsd-plan-phase SKILL.md has effort: max after global install (#1319)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'plan-phase'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^effort:[ \t]*xhigh$/m, - `gsd-plan-phase SKILL.md must have effort: xhigh\nActual:\n${fm}`); + assert.match(fm, /^effort:[ \t]*max$/m, + `gsd-plan-phase SKILL.md must have effort: max\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^effort:[ \t]*xhigh$/m, + `gsd-plan-phase SKILL.md must not have rejected effort: xhigh (#1319)\nActual:\n${fm}`); }); test('gsd-progress SKILL.md has effort: low after global install', () => {