From 16d9b2e8e73d77240ad9962cad4ce8df5af26391 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 9 Jun 2026 12:37:32 -0400 Subject: [PATCH] fix(#936): run plan-phase inline in convergence; guard against nested spawner wraps (#939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both sites in plan-review-convergence.md that wrapped gsd-plan-phase in Agent() (initial planning + replan loop) are now bare Skill() calls at depth 0. On Claude Code, a depth-1 Agent has no Agent tool so wrapped plan-phase could never spawn gsd-planner/gsd-plan-checker — the replan loop silently produced no revised plan when HIGHs were found. Running plan-phase inline from the depth-0 orchestrator (which retains the Agent tool) restores the full sub-agent chain. A full audit of all workflow files confirmed these two sites were the only instances of the anti-pattern (no other workflow wraps a spawner orchestrator in Agent() without a RUNTIME carve-out). Added structural guard test bug-936-no-nested-spawner-wrap.test.cjs that dynamically derives the spawner set (workflows containing subagent_type=) and asserts no workflow wraps a spawner inside Agent() without a RUNTIME != claude carve-out — prevents silent regression. Test passes on fixed code, would fail on pre-fix code at the two de-wrapped sites. Also applied two low-severity prose nits flagged in review: - commands/gsd/plan-review-convergence.md: orchestrator role updated to describe inline plan-phase + Agent for review (was generic "spawn Agents") - gsd-core/workflows/plan-review-convergence.md success_criteria: narrowed "Each Agent fully completes" to the review Agent (plan-phase is inline now) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (cherry picked from commit e4e8a9fcb8cd746842dae1a3083d26d5a3c5e895) --- .../936-convergence-inline-plan-phase.md | 5 + commands/gsd/plan-review-convergence.md | 6 +- gsd-core/workflows/plan-review-convergence.md | 48 ++-- tests/bug-936-no-nested-spawner-wrap.test.cjs | 207 ++++++++++++++++++ tests/plan-review-convergence.test.cjs | 96 +++++++- 5 files changed, 326 insertions(+), 36 deletions(-) create mode 100644 .changeset/936-convergence-inline-plan-phase.md create mode 100644 tests/bug-936-no-nested-spawner-wrap.test.cjs diff --git a/.changeset/936-convergence-inline-plan-phase.md b/.changeset/936-convergence-inline-plan-phase.md new file mode 100644 index 000000000..5ebdf7df6 --- /dev/null +++ b/.changeset/936-convergence-inline-plan-phase.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 0 +--- +**`plan-review-convergence` now runs `gsd-plan-phase` inline instead of inside `Agent()`** — both sites that previously wrapped `gsd-plan-phase` in `Agent()` (initial planning + replan loop) have been changed to bare `Skill()` calls at depth 0. On Claude Code, a depth-1 Agent has no Agent tool, so a wrapped `plan-phase` could never spawn `gsd-planner` or `gsd-plan-checker` — the replan loop silently failed to produce a revised plan whenever HIGH concerns were found. Running plan-phase inline from the depth-0 orchestrator (which retains the Agent tool) restores the full planner→checker sub-agent chain. A new structural guard test (`bug-936-no-nested-spawner-wrap.test.cjs`) statically scans all workflow files and fails if any workflow wraps a spawner orchestrator in `Agent()` without a `RUNTIME != claude` carve-out, preventing regression. (#936) diff --git a/commands/gsd/plan-review-convergence.md b/commands/gsd/plan-review-convergence.md index e75eaf46b..c13defcf9 100644 --- a/commands/gsd/plan-review-convergence.md +++ b/commands/gsd/plan-review-convergence.md @@ -17,11 +17,11 @@ requires: [phase, review] Cross-AI plan convergence loop — an outer revision gate around gsd-review and gsd-planner. Repeatedly: review plans with external AI CLIs → if HIGH concerns found → replan with --reviews feedback → re-review. Stops when no HIGH concerns remain or max cycles reached. -**Flow:** Agent→Skill("gsd-plan-phase") → Agent→Skill("gsd-review") → check HIGHs → Agent→Skill("gsd-plan-phase --reviews") → Agent→Skill("gsd-review") → ... → Converge or escalate +**Flow:** Skill("gsd-plan-phase") → Agent→Skill("gsd-review") → check HIGHs → Skill("gsd-plan-phase --reviews") → Agent→Skill("gsd-review") → ... → Converge or escalate -Replaces gsd-plan-phase's internal gsd-plan-checker with external AI reviewers (codex, gemini, etc.). Each step runs inside an isolated Agent that calls the corresponding existing Skill — orchestrator only does loop control. +Replaces gsd-plan-phase's internal gsd-plan-checker with external AI reviewers (codex, gemini, etc.). Plan-phase runs **inline** (bare Skill at depth 0) so it can spawn gsd-planner/gsd-plan-checker at depth 1. Review runs inside an isolated Agent (gsd-review is a Bash leaf — no sub-agents needed). Orchestrator only does loop control. -**Orchestrator role:** Parse arguments, validate phase, spawn Agents for existing Skills, check HIGHs, stall detection, escalation gate. +**Orchestrator role:** Parse arguments, validate phase, run plan-phase inline (Skill at depth 0), spawn an Agent for gsd-review, check HIGHs, stall detection, escalation gate. diff --git a/gsd-core/workflows/plan-review-convergence.md b/gsd-core/workflows/plan-review-convergence.md index 953a2f350..552278d64 100644 --- a/gsd-core/workflows/plan-review-convergence.md +++ b/gsd-core/workflows/plan-review-convergence.md @@ -1,7 +1,8 @@ Cross-AI plan convergence loop — automates the manual chain: gsd-plan-phase N → gsd-review N --codex → gsd-plan-phase N --reviews → gsd-review N --codex → ... -Each step runs inside an isolated Agent that calls the corresponding Skill. +Plan-phase runs inline (bare Skill at depth 0) so it can spawn gsd-planner/gsd-plan-checker at depth 1. +Review runs inside an isolated Agent (leaf skill — Bash only, no sub-agents needed). Orchestrator only does: init, loop control, parse CYCLE_SUMMARY for HIGH count, stall detection, escalation. @@ -98,21 +99,15 @@ Display startup banner: **If `has_plans` is false:** -Display: `◆ No plans found — spawning initial planning agent... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` +Display: `◆ No plans found — running initial planning inline... (plan-phase runs here in the orchestrator — no output until planning is complete, ~1–5 min; expected, not a freeze)` ```text -Agent( - description="Initial planning Phase {PHASE}", - prompt="Run /gsd:plan-phase for Phase {PHASE}. - -Execute: Skill(skill='gsd-plan-phase', args='{PHASE} {GSD_WS}') - -Complete the full planning workflow. Do NOT return until planning is complete and PLAN.md files are committed.", - mode="auto" -) +Skill(skill="gsd-plan-phase", args="{PHASE} {GSD_WS}") ``` -After agent returns, verify plans were created: +Run plan-phase **inline** (do NOT wrap it in Agent()). The convergence orchestrator runs at depth 0 with Agent available, so inline plan-phase can spawn gsd-planner and gsd-plan-checker at depth 1 — the one level of nesting that works on Claude Code. Wrapping plan-phase in Agent() would push it to depth 1 where the Agent tool is absent, preventing it from spawning any sub-agents. Wait until plan-phase completes and PLAN.md files are committed before continuing. + +After plan-phase completes, verify plans were created: ```bash PLAN_COUNT=$(ls ${phase_dir}/${padded_phase}-*-PLAN.md 2>/dev/null | wc -l) ``` @@ -302,44 +297,35 @@ To restart loop: /gsd:plan-review-convergence {PHASE} {REVIEWER_FLAGS} ``` Exit workflow. -### 5d. Replan (Spawn Agent) +### 5d. Replan (Inline) **If under max cycles:** Update `prev_high_count = HIGH_COUNT`. -Display: `◆ Spawning replan agent with review feedback... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` +Display: `◆ Replanning inline with review feedback... (plan-phase runs here in the orchestrator — no output until replanning is complete, ~1–5 min; expected, not a freeze)` ```text -Agent( - description="Replan Phase {PHASE} with review feedback cycle {cycle}", - prompt="Run /gsd:plan-phase with --reviews for Phase {PHASE}. - -Execute: Skill(skill='gsd-plan-phase', args='{PHASE} --reviews --skip-research {GSD_WS}') - -This will replan incorporating cross-AI review feedback from REVIEWS.md. -Do NOT return until replanning is complete and updated PLAN.md files are committed. - -IMPORTANT: When gsd-plan-phase outputs '## PLANNING COMPLETE', that means replanning is done. Return at that point.", - mode="auto" -) +Skill(skill="gsd-plan-phase", args="{PHASE} --reviews --skip-research {GSD_WS}") ``` -After agent returns → go back to **step 5a** (review again). +Run plan-phase **inline** (do NOT wrap it in Agent()). Same rationale as step 4: the convergence orchestrator runs at depth 0 with Agent available, so inline plan-phase can spawn gsd-planner and gsd-plan-checker at depth 1. Wrapping in Agent() pushes plan-phase to depth 1 where the Agent tool is absent — the replan loop can never produce a revised plan when HIGHs are found. This is the root cause of bug #936. Wait until plan-phase completes (outputs '## PLANNING COMPLETE') and updated PLAN.md files are committed before continuing. + +After plan-phase completes → go back to **step 5a** (review again). - [ ] Config gate checked before running — exits with enable instructions if workflow.plan_review_convergence is false -- [ ] Initial planning via Agent → Skill("gsd-plan-phase") if no plans exist -- [ ] Review via Agent → Skill("gsd-review") — isolated, not inline; {GSD_WS} forwarded -- [ ] Replan via Agent → Skill("gsd-plan-phase --reviews") — isolated, not inline +- [ ] Initial planning via inline Skill("gsd-plan-phase") if no plans exist — NOT wrapped in Agent() (bug #936: depth-1 Agent has no Agent tool) +- [ ] Review via Agent → Skill("gsd-review") — isolated Agent is correct; gsd-review is a Bash leaf with no sub-agent spawns; {GSD_WS} forwarded +- [ ] Replan via inline Skill("gsd-plan-phase --reviews") — NOT wrapped in Agent(); inline lets plan-phase spawn gsd-planner/gsd-plan-checker at depth 1 - [ ] Orchestrator only does: init, config gate, loop control, parse CYCLE_SUMMARY for HIGH count, stall detection, escalation - [ ] HIGH count extracted from review agent's CYCLE_SUMMARY return message (not by grepping REVIEWS.md) - [ ] Review agent prompt defines CYCLE_SUMMARY: current_high= contract with PARTIALLY/FULLY RESOLVED definitions - [ ] Abort with clear error if CYCLE_SUMMARY is absent; distinguish malformed from absent - [ ] Warn if HIGH_COUNT > 0 but ## Current HIGH Concerns section is absent from return message -- [ ] Each Agent fully completes its Skill before returning +- [ ] The review Agent fully completes gsd-review before returning (plan-phase runs inline — no Agent wrap) - [ ] Loop exits on: no HIGH concerns (converged) OR max cycles (escalation) - [ ] Stall detection reported when HIGH count not decreasing - [ ] STATE.md updated on convergence completion diff --git a/tests/bug-936-no-nested-spawner-wrap.test.cjs b/tests/bug-936-no-nested-spawner-wrap.test.cjs new file mode 100644 index 000000000..27bb440bd --- /dev/null +++ b/tests/bug-936-no-nested-spawner-wrap.test.cjs @@ -0,0 +1,207 @@ +'use strict'; +/** + * Structural guard — bug(#936): plan-review-convergence wrapped gsd-plan-phase + * in Agent() at TWO sites (initial planning + replan). On Claude Code, a depth-1 + * Agent has no Agent tool, so plan-phase cannot spawn gsd-planner / gsd-plan-checker + * → the replan loop never works when HIGHs are found. + * + * Fix: run plan-phase INLINE (bare Skill()) from the convergence orchestrator, + * which runs at depth 0 and has Agent available — exactly how autonomous.md, + * manager.md, and discuss-phase-assumptions.md already chain plan-phase. + * + * This guard dynamically derives the set of "spawner" workflows (those containing + * `subagent_type=`) and asserts that NO workflow wraps a spawner inside Agent() + * UNLESS the wrapping block includes a RUNTIME != claude carve-out (the #853 + * pattern already applied to autonomous.md / manager.md). + */ + +// allow-test-rule: source-text-is-the-product +// The workflow markdown IS the runtime instruction — static guards over +// workflow text are the canonical regression-test mechanism (per CONTRIBUTING +// exception matrix and tests/bug-853-bg-dispatch-runtime-gating.test.cjs). + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); + +// ── 1. Derive spawner skill names dynamically ────────────────────────────── +// A "spawner" workflow is one that contains `subagent_type=` — it NEEDS the +// Agent tool to run and therefore cannot safely be wrapped in another Agent() +// on Claude Code (where depth-1 agents have no Agent tool). + +// Recursively collect all *.md files under WORKFLOWS_DIR (covers nested fragments +// like discuss-phase/modes/*.md and execute-phase/steps/*.md). +function collectWorkflowFiles(dir) { + const entries = fs.readdirSync(dir, { withFileTypes: true }); + const results = []; + for (const e of entries) { + const fullPath = path.join(dir, e.name); + if (e.isDirectory()) { + results.push(...collectWorkflowFiles(fullPath)); + } else if (e.name.endsWith('.md')) { + results.push({ + name: path.relative(WORKFLOWS_DIR, fullPath), + path: fullPath, + content: fs.readFileSync(fullPath, 'utf8'), + }); + } + } + return results; +} + +const allWorkflowFiles = collectWorkflowFiles(WORKFLOWS_DIR); + +// Map: base-slug → workflow filename (e.g. "plan-phase" → "plan-phase.md") +// Skill() calls use the "gsd-" convention in all workflow files. +// We build BOTH the bare slug set and the gsd-prefixed skill-name set. +const SPAWNER_BASE_SLUGS = new Set( + allWorkflowFiles + .filter((w) => w.content.includes('subagent_type=')) + .map((w) => w.name.replace(/\.md$/, '')) +); + +// Skill invocations use "gsd-" (e.g. gsd-plan-phase, gsd-execute-phase). +// Build the regex from the prefixed names so it actually matches what workflows write. +const SPAWNER_GSD_NAMES = new Set([...SPAWNER_BASE_SLUGS].map((s) => `gsd-${s}`)); + +// Build a regex that matches Skill(skill='gsd-') or Skill(skill="gsd-") +const spawnerPattern = new RegExp( + `Skill\\(\\s*skill=['"](?:${[...SPAWNER_GSD_NAMES].join('|')})['"]`, + 's' +); + +// ── 2. Helper: extract Agent() blocks from a workflow ───────────────────── +// Each block starts at "Agent(" and ends at the balancing ")". We collect +// the text of each such block together with the surrounding context (a 400 +// char window before the block) so we can check for RUNTIME carve-outs. + +function extractAgentBlocks(content) { + const blocks = []; + let pos = 0; + while (pos < content.length) { + const start = content.indexOf('Agent(', pos); + if (start === -1) break; + // Walk forward to find the balancing closing paren + let depth = 0; + let i = start + 'Agent('.length - 1; // at the '(' + for (; i < content.length; i++) { + if (content[i] === '(') depth++; + else if (content[i] === ')') { + depth--; + if (depth === 0) break; + } + } + const end = i + 1; + const blockText = content.slice(start, end); + // Capture context: 400 chars before the block (for RUNTIME gate detection) + const contextBefore = content.slice(Math.max(0, start - 400), start); + blocks.push({ start, end, blockText, contextBefore }); + pos = end; + } + return blocks; +} + +// ── 3. Helper: does a block have a RUNTIME != claude carve-out nearby? ──── +// The #853 pattern looks like: "RUNTIME is `claude`" in a preceding condition +// that switches to inline Skill() instead of the Agent() block. A block is +// considered guarded when the 400-char context window before it (or the block +// body itself for block-internal guards) contains any of these markers. + +function hasRuntimeCarveout(block) { + const haystack = block.contextBefore + block.blockText; + return ( + /RUNTIME[^`\n]{0,30}(?:!=|≠|is not|!==)\s*[`'"]?claude/i.test(haystack) || + /RUNTIME[^`\n]{0,30}claude[^`\n]{0,30}(?:inline|not.*Agent|do NOT)/i.test(haystack) || + /If `RUNTIME` is `claude`/i.test(haystack) || + /On Claude Code.*inline/is.test(haystack) + ); +} + +// ── 4. The guard: scan every workflow for unguarded Agent→spawner wraps ─── + +describe('bug-936 — no workflow wraps a spawner skill inside Agent() without a RUNTIME carve-out', () => { + test('spawner set is non-empty (self-check: subagent_type= grep must find files)', () => { + assert.ok(SPAWNER_BASE_SLUGS.size > 0, `No spawner workflows found in ${WORKFLOWS_DIR} — SPAWNER_BASE_SLUGS derivation is broken`); + // plan-phase must be a spawner (base slug) + assert.ok(SPAWNER_BASE_SLUGS.has('plan-phase'), 'plan-phase.md must be in the spawner set (contains subagent_type=)'); + // gsd-plan-phase must be in the prefixed set used by the regex + assert.ok(SPAWNER_GSD_NAMES.has('gsd-plan-phase'), 'gsd-plan-phase must be in SPAWNER_GSD_NAMES — the prefixed form used in Skill() calls'); + }); + + for (const wf of allWorkflowFiles) { + // Only scan files that have at least one Agent( call + if (!wf.content.includes('Agent(')) continue; + + test(`${wf.name}: no Agent() block wraps a spawner Skill without a RUNTIME carve-out`, () => { + const blocks = extractAgentBlocks(wf.content); + const violations = blocks.filter((b) => { + const wrapsSpawner = spawnerPattern.test(b.blockText); + if (!wrapsSpawner) return false; + return !hasRuntimeCarveout(b); + }); + + assert.deepStrictEqual( + violations.map((v) => v.blockText.slice(0, 120).replace(/\n/g, '\\n')), + [], + `${wf.name} wraps a spawner Skill inside Agent() without a RUNTIME != claude carve-out.\n` + + `Fix: run the spawner Skill inline (bare Skill() call at depth 0) OR add a RUNTIME gate.\n` + + `See: bug #936, tests/bug-853-bg-dispatch-runtime-gating.test.cjs for the guarded pattern.` + ); + }); + } +}); + +// ── 5. Focused regression: plan-review-convergence never wraps plan-phase ─ + +describe('bug-936 — plan-review-convergence runs plan-phase inline, not inside Agent()', () => { + const CONVERGENCE = fs.readFileSync( + path.join(WORKFLOWS_DIR, 'plan-review-convergence.md'), + 'utf8' + ); + + test('plan-review-convergence does NOT wrap gsd-plan-phase inside Agent()', () => { + // The anti-pattern: Agent( block whose body contains Skill(skill='gsd-plan-phase') + const blocks = extractAgentBlocks(CONVERGENCE); + const wrapping = blocks.filter((b) => + /Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText) && + !hasRuntimeCarveout(b) + ); + assert.deepStrictEqual( + wrapping.map((v) => v.blockText.slice(0, 120).replace(/\n/g, '\\n')), + [], + 'plan-review-convergence must NOT wrap gsd-plan-phase inside Agent(). ' + + 'Run it inline (bare Skill() at depth 0) so it can spawn gsd-planner/gsd-plan-checker. ' + + 'See: bug #936' + ); + }); + + test('plan-review-convergence calls gsd-plan-phase inline (bare Skill call outside Agent block)', () => { + // After the fix: at least one bare Skill(skill="gsd-plan-phase") must appear + // outside any Agent( block — that is the inline call from the depth-0 orchestrator. + const blocks = extractAgentBlocks(CONVERGENCE); + // Remove all Agent block ranges from the text + let masked = CONVERGENCE; + // Work from end to start so offsets stay valid + const sorted = [...blocks].sort((a, b) => b.start - a.start); + for (const b of sorted) { + masked = masked.slice(0, b.start) + ' '.repeat(b.end - b.start) + masked.slice(b.end); + } + const hasInlineCall = /Skill\(\s*skill=["']gsd-plan-phase["']/.test(masked); + assert.ok( + hasInlineCall, + 'plan-review-convergence must contain at least one bare Skill(skill="gsd-plan-phase") ' + + 'outside any Agent() block — this is the inline call that lets plan-phase spawn its sub-agents. ' + + 'See: bug #936' + ); + }); + + test('plan-review-convergence still wraps gsd-review inside Agent() (leaf — isolation is correct)', () => { + // gsd-review is a leaf (shells out via Bash, no subagent_type) so the Agent wrap is fine and intentional. + const blocks = extractAgentBlocks(CONVERGENCE); + const reviewWrap = blocks.some((b) => /Skill\(\s*skill=['"]gsd-review['"]/.test(b.blockText)); + assert.ok(reviewWrap, 'gsd-review must still be wrapped in Agent() — it is a Bash leaf and isolation is intentional'); + }); +}); diff --git a/tests/plan-review-convergence.test.cjs b/tests/plan-review-convergence.test.cjs index cbf880f3b..2f3c08b1a 100644 --- a/tests/plan-review-convergence.test.cjs +++ b/tests/plan-review-convergence.test.cjs @@ -186,10 +186,10 @@ describe('plan-review-convergence workflow: initial planning gate (#2306)', () = ); }); - test('workflow spawns isolated planning agent when no plans exist', () => { + test('workflow runs gsd-plan-phase when no plans exist', () => { assert.ok( workflow.includes('gsd-plan-phase'), - 'workflow must spawn Agent → gsd-plan-phase when no plans exist' + 'workflow must invoke gsd-plan-phase when no plans exist' ); }); @@ -651,3 +651,95 @@ describe('plan-review-convergence local model CONFIGURATION.md documentation (#2 ); }); }); + +// ─── Bug #936: plan-phase must run inline, not inside Agent() ───────────── +// +// Regression guard: inverted from the pre-#936 behavior that locked in the bug. +// On Claude Code a depth-1 Agent has no Agent tool, so gsd-plan-phase wrapped in +// Agent() cannot spawn gsd-planner / gsd-plan-checker → the replan loop breaks. +// Fix: run plan-phase inline (bare Skill()) from the depth-0 convergence orchestrator. +// +// These tests FAIL on pre-fix code and PASS after the fix. + +describe('plan-review-convergence workflow: inline plan-phase dispatch (#936)', () => { + const workflow = fs.readFileSync(WORKFLOW_PATH, 'utf8'); + + // Helper: extract Agent() block bodies from workflow text + function extractAgentBlocks(content) { + const blocks = []; + let pos = 0; + while (pos < content.length) { + const start = content.indexOf('Agent(', pos); + if (start === -1) break; + let depth = 0; + let i = start + 'Agent('.length - 1; + for (; i < content.length; i++) { + if (content[i] === '(') depth++; + else if (content[i] === ')') { depth--; if (depth === 0) break; } + } + blocks.push({ start, end: i + 1, blockText: content.slice(start, i + 1) }); + pos = i + 1; + } + return blocks; + } + + test('initial planning does NOT wrap gsd-plan-phase inside Agent() (#936 fix)', () => { + // Pre-fix: Agent( ... Skill('gsd-plan-phase') ... ) in step 4 + // Post-fix: bare Skill(skill="gsd-plan-phase") at orchestrator level + const blocks = extractAgentBlocks(workflow); + const wrapping = blocks.filter((b) => + /Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText) + ); + assert.deepStrictEqual( + wrapping.map((b) => b.blockText.slice(0, 80).replace(/\n/g, '\\n')), + [], + 'Initial planning must NOT wrap gsd-plan-phase inside Agent() — run it inline so ' + + 'it can spawn gsd-planner/gsd-plan-checker at depth 1. See: bug #936' + ); + }); + + test('replan step does NOT wrap gsd-plan-phase inside Agent() (#936 fix)', () => { + // Same check as above; explicitly named for the replan site (step 5d) + const blocks = extractAgentBlocks(workflow); + const wrapping = blocks.filter((b) => + /Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText) && + /--reviews/.test(b.blockText) + ); + assert.deepStrictEqual( + wrapping.map((b) => b.blockText.slice(0, 80).replace(/\n/g, '\\n')), + [], + 'Replan step must NOT wrap gsd-plan-phase inside Agent() — the replan loop can ' + + 'never produce a plan on Claude Code when plan-phase is at depth 1. See: bug #936' + ); + }); + + test('workflow calls gsd-plan-phase inline (bare Skill outside Agent block) (#936 fix)', () => { + // After the fix there must be at least one bare Skill(skill="gsd-plan-phase") + // OUTSIDE any Agent() block. + const blocks = extractAgentBlocks(workflow); + let masked = workflow; + const sorted = [...blocks].sort((a, b) => b.start - a.start); + for (const b of sorted) { + masked = masked.slice(0, b.start) + ' '.repeat(b.end - b.start) + masked.slice(b.end); + } + assert.ok( + /Skill\(\s*skill=["']gsd-plan-phase["']/.test(masked), + 'plan-review-convergence must contain at least one bare Skill(skill="gsd-plan-phase") ' + + 'outside any Agent() block — the inline call that preserves depth-0 Agent availability. See: bug #936' + ); + }); + + test('success_criteria describes inline plan-phase, not Agent → Skill (#936 fix)', () => { + const successBlock = workflow.slice(workflow.lastIndexOf('')); + // The broken criterion said "Initial planning via Agent → Skill" + assert.ok( + !successBlock.includes('via Agent → Skill("gsd-plan-phase")'), + 'success_criteria must NOT describe plan-phase as Agent → Skill — that was the broken pattern. See: bug #936' + ); + // The broken criterion said "isolated, not inline" for the replan + assert.ok( + !successBlock.includes('isolated, not inline'), + 'success_criteria must NOT say "isolated, not inline" for plan-phase — the fix makes it inline. See: bug #936' + ); + }); +});