fix(#2762): chunked --reviews replans instead of no-op + outline resume marker written to file (#2887)
* test(#2762): chunked --reviews must replan, not no-op (outline marker + per-plan --reviews exception) * fix(#2762): chunked --reviews replans plans instead of skipping 100% + outline resume marker written to file Defect A: §8.5.1 outline resume-check greped for a marker the agent only RETURNED (never wrote to the file) → outline always re-ran (broke crash-resume). Fix: the outline agent writes ## OUTLINE COMPLETE into the file. Defect B: §8.5.2 per-plan resume-check skipped any plan with frontmatter, no --reviews exception → --reviews skipped 100% of plans (contradicted §6 'go straight to replanning'). Fix: gate the skip on --reviews being ABSENT. Crash-resume (non-reviews) still skips. Condensed adjacent §8.5 prose to keep plan-phase.md under the 94519B cap (net -33B). * chore(#2762): changeset fragment * chore(#2762): backfill changeset PR number (2887) --------- Co-authored-by: Test <test@example.com>
This commit is contained in:
5
.changeset/tidy-wasps-hum.md
Normal file
5
.changeset/tidy-wasps-hum.md
Normal file
@@ -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)
|
||||
@@ -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
|
||||
```
|
||||
|
||||
|
||||
58
tests/issue-2762-plan-reviews-chunked.test.cjs
Normal file
58
tests/issue-2762-plan-reviews-chunked.test.cjs
Normal file
@@ -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)'
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user