diff --git a/.changeset/tidy-wasps-hum.md b/.changeset/tidy-wasps-hum.md new file mode 100644 index 000000000..4c49d4047 --- /dev/null +++ b/.changeset/tidy-wasps-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2887 +--- +**`/gsd-plan-phase --reviews` now actually replans in chunked mode instead of silently skipping every plan** — the per-plan resume-check skips existing plans for crash-resume, but now exempts `--reviews` (whose purpose is to replan with review feedback). Also fixed the outline resume-check, which looked for a marker the agent only returned (never wrote to the file), so the outline always re-ran. (#2762) diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index a4b6286af..4b2e2cedc 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -893,20 +893,17 @@ Agent( **Skip if `CHUNKED_MODE` is `false`.** -Chunked mode splits the single long-lived planner Agent run into a short outline Agent run followed by -N short per-plan Agent runs. Each run is bounded to ~3–5 min; each plan is committed individually -for crash resilience. If any run hangs and the terminal is force-killed, rerunning -`/gsd:plan-phase {N} --chunked` resumes from the last successfully committed plan. +Chunked mode splits the single planner run into a short outline run + N short per-plan +runs (~3–5 min each), committing each plan individually for crash resilience. Rerunning +`/gsd:plan-phase {N} --chunked` resumes from the last committed plan. -**Intended for new or in-progress chunked runs.** To recover plans already written by a prior -*non-chunked* run, use step 6's "Add more plans" or proceed directly to `/gsd:execute-phase` -— don't start a fresh chunked run over existing non-chunked plans. +For recovering plans from a prior *non-chunked* run, use step 6's "Add more plans" or +proceed to `/gsd:execute-phase` — don't start a fresh chunked run over them. ### 8.5.1 Outline Phase (outline-only mode, ~2 min) -**Resume detection:** If `${PHASE_DIR}/${PADDED_PHASE}-PLAN-OUTLINE.md` already exists **and -is valid** (contains the `## OUTLINE COMPLETE` marker), skip this sub-step — the outline -already exists from a previous run. Proceed directly to 8.5.2. +**Resume detection:** If `${PHASE_DIR}/${PADDED_PHASE}-PLAN-OUTLINE.md` exists and contains +the `## OUTLINE COMPLETE` marker (written by the outline agent — #2762), skip to 8.5.2. ```bash OUTLINE_FILE="${PHASE_DIR}/${PADDED_PHASE}-PLAN-OUTLINE.md" @@ -934,6 +931,8 @@ Agent( The outline must be a markdown table with columns: Plan ID | Objective | Wave | Depends On | Requirements + End the file with a final line `## OUTLINE COMPLETE` — §8.5.1's resume-check greps + the file for it, so it MUST be written here, not just returned. Return: ## OUTLINE COMPLETE with plan count.", subagent_type="gsd-planner", model="{planner_model}", @@ -951,14 +950,14 @@ Handle return: For each plan entry extracted from `PLAN-OUTLINE.md`: -1. **Resume check:** If `${PHASE_DIR}/{plan_id}-PLAN.md` already exists on disk **and has - valid YAML frontmatter** (opening `---` delimiter present), skip this plan (do not - overwrite completed work — resume safety). +1. **Resume check:** Skip if `${PHASE_DIR}/{plan_id}-PLAN.md` exists with valid frontmatter + (resume safety) — UNLESS `--reviews` is set, whose purpose is to REPLAN with review + feedback (§6), so existing plans are overwritten, not skipped (#2762). ```bash PLAN_FILE="${PHASE_DIR}/${plan_id}-PLAN.md" - if [[ -f "$PLAN_FILE" ]] && head -1 "$PLAN_FILE" | grep -q '^---'; then - continue # plan already written, skip + if [[ -f "$PLAN_FILE" ]] && head -1 "$PLAN_FILE" | grep -q '^---' && [[ "$ARGUMENTS" != *"--reviews"* ]]; then + continue # resume safety — NOT under --reviews (replan) fi ``` diff --git a/tests/issue-2762-plan-reviews-chunked.test.cjs b/tests/issue-2762-plan-reviews-chunked.test.cjs new file mode 100644 index 000000000..5a2ab415a --- /dev/null +++ b/tests/issue-2762-plan-reviews-chunked.test.cjs @@ -0,0 +1,58 @@ +// allow-test-rule: structural-implementation-guard (#2762) +'use strict'; + +// Regression guard for #2762: /gsd-plan-phase --reviews was a silent no-op in chunked +// mode. Two defects in plan-phase.md §8.5: +// A. §8.5.1 outline resume-check greps for a marker the agent only RETURNED (never +// wrote to the file) → outline always re-ran/overwrote (broke crash-resume). +// B. §8.5.2 per-plan resume-check skipped any plan with frontmatter, with no +// --reviews exception → --reviews skipped 100% of plans (contradicted §6's +// "go straight to replanning" contract). + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const MD = path.join(__dirname, '..', 'gsd-core', 'workflows', 'plan-phase.md'); +const read = () => fs.readFileSync(MD, 'utf8'); + +test('§8.5.1 outline agent writes the resume marker into the file (#2762 defect A)', () => { + const src = read(); + // The outline prompt must instruct writing ## OUTLINE COMPLETE into PLAN-OUTLINE.md + // (the §8.5.1 resume-check greps for it in the file). + const outlineSection = src.slice(src.indexOf('### 8.5.1'), src.indexOf('### 8.5.2')); + assert.ok( + /outline|PLAN-OUTLINE/i.test(outlineSection) && /End the file.*## OUTLINE COMPLETE|write.*## OUTLINE COMPLETE.*file/i.test(outlineSection.replace(/\s+/g, ' ')), + 'the outline agent prompt must instruct writing ## OUTLINE COMPLETE into the file (the resume-check greps the file for it) (#2762)' + ); +}); + +test('§8.5.2 per-plan resume-check does NOT skip under --reviews (#2762 defect B)', () => { + const src = read(); + const perPlanSection = src.slice(src.indexOf('### 8.5.2')); + // The resume-check bash must gate the skip on --reviews being ABSENT. + const bashMatch = perPlanSection.match(/PLAN_FILE=[\s\S]*?fi\s*\n/); + assert.ok(bashMatch, '§8.5.2 must contain the per-plan resume-check bash block'); + const bash = bashMatch[0]; + assert.ok( + /--reviews/.test(bash), + 'the per-plan resume-check must reference --reviews so it does NOT skip when replanning with review feedback (#2762)' + ); + // The skip must be conditional on --reviews being ABSENT (e.g. ARGUMENTS != *"--reviews"*). + assert.ok( + /!=\s*\*"--reviews"\*|!~.*--reviews|--reviews.*absent|not.*--reviews/i.test(bash), + 'the resume-check skip must be gated on --reviews being ABSENT (so --reviews overwrites/replans) (#2762)' + ); +}); + +test('§8.5.2 crash-resume (non-reviews) still skips written plans (#2762 negative space)', () => { + const src = read(); + const perPlanSection = src.slice(src.indexOf('### 8.5.2')); + const bashMatch = perPlanSection.match(/PLAN_FILE=[\s\S]*?fi\s*\n/); + const bash = bashMatch ? bashMatch[0] : ''; + assert.ok( + /head -1.*grep.*\^---|frontmatter/i.test(bash + perPlanSection.slice(0, 400)), + 'crash-resume (non-reviews) must still skip plans with valid frontmatter (resume safety preserved) (#2762)' + ); +});