From 7d08d80234195c0f65acf46d7b1f13271a76f907 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 02:47:24 -0400 Subject: [PATCH] fix(#3297): project --gaps mode onto next-up execute command (#3453) * fix(#3297): project --gaps mode onto next-up execute command * chore(#3297): set changeset pr to 3453 --------- Co-authored-by: sim --- .changeset/happy-voles-chatter.md | 5 + gsd-core/workflows/plan-phase.md | 12 ++- .../3218-prompt-layer-plan-counts.json | 11 --- .../3297-planphase-gaps-next-up-scope.json | 6 ++ tests/next-up-clear-order.test.cjs | 91 +++++++++++++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 .changeset/happy-voles-chatter.md delete mode 100644 tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json create mode 100644 tests/emitted-drift-acks/3297-planphase-gaps-next-up-scope.json diff --git a/.changeset/happy-voles-chatter.md b/.changeset/happy-voles-chatter.md new file mode 100644 index 000000000..d91bbbf19 --- /dev/null +++ b/.changeset/happy-voles-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3453 +--- +plan-phase: a completed --gaps planning run's Next Up handoff now recommends /gsd:execute-phase --gaps-only (matching the gap-closure scope just planned) instead of the whole-phase /gsd:execute-phase . Standard and --reviews runs are unchanged. diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index 61d1eb5cc..58bd1f947 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -73,6 +73,14 @@ INGEST_PARAM=""; if [[ "$ARGUMENTS" =~ (^|[[:space:]])--ingest[[:space:]]+([^[:s RESEARCH_PHASE_PARAM=""; if [[ "$ARGUMENTS" =~ (^|[[:space:]])--research-phase[[:space:]]+([^[:space:]-][^[:space:]]*) ]]; then RESEARCH_PHASE_PARAM="--research-phase ${BASH_REMATCH[2]}"; fi REVIEWS_PARAM=""; if [[ "$ARGUMENTS" =~ (^|[[:space:]])--reviews([[:space:]]|$) ]]; then REVIEWS_PARAM="--reviews"; fi CHUNKED_PARAM=""; if [[ "$ARGUMENTS" =~ (^|[[:space:]])--chunked([[:space:]]|$) ]]; then CHUNKED_PARAM="--chunked"; fi +# Project the just-completed planning mode onto the execute-phase follow-up (#3297): +# a --gaps run creates gap_closure plans, so the Next Up handoff must point at +# execute-phase's matching --gaps-only scope rather than the whole-phase run. +# Standard and --reviews runs leave GAPS_EXEC_FLAG empty → their Next Up is unchanged. +GAPS_MODE=false +if [[ "$ARGUMENTS" =~ (^|[[:space:]])--gaps([[:space:]]|$) ]]; then GAPS_MODE=true; fi +GAPS_EXEC_FLAG="" +if [ "$GAPS_MODE" = "true" ]; then GAPS_EXEC_FLAG="--gaps-only"; fi INIT=$(gsd_run query init.plan-phase "$PHASE" $GRAN_PARAM $PRD_PARAM $INGEST_PARAM $RESEARCH_PHASE_PARAM $REVIEWS_PARAM $CHUNKED_PARAM) if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi AGENT_SKILLS_RESEARCHER=$(gsd_run query agent-skills gsd-phase-researcher) @@ -1472,6 +1480,8 @@ Route to `` (existing behavior). Output this markdown directly (not as a code block): +`${GAPS_EXEC_FLAG}` projects the just-completed planning mode onto the follow-up execute command (#3297): it expands to `--gaps-only` for a `--gaps` planning run (so the handoff points at execute-phase's gap-closure scope — only the newly created `gap_closure: true` plans — not the whole phase) and to empty for a standard or `--reviews` run (whole-phase scope, unchanged). Substitute it verbatim; when empty, collapse the extra space. + ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ GSD ► PHASE {X} PLANNED ✓ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -1494,7 +1504,7 @@ Verification: {Passed | Passed with override | Skipped} /clear then: -/gsd:execute-phase {X} ${GSD_WS} +/gsd:execute-phase {X} ${GAPS_EXEC_FLAG} ${GSD_WS} ─────────────────────────────────────────────────────────────── diff --git a/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json b/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json deleted file mode 100644 index 8f2b4a6c7..000000000 --- a/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "version": 1, - "paths": { - "plan-phase.md": { - "reason": "Issue #3218 (epic #3180 phase 8): grew the most of the four files because it migrated TWO sites off `ls *-PLAN.md | wc -l` (the step 9a and step 11a filesystem-fallback DISK_PLANS checks) to `gsd_run query find-phase` + jq, and because both sites now take the PHYSICAL plan set (`plan_count_all`, status:superseded included) for their 'did the planner/checker write files to disk?' question — the old glob undercounted on the nested plans/ layout and missed loosely-named files." - }, - "plan-review-convergence.md": { - "reason": "Issue #3218 (epic #3180 phase 8): replaced a raw `ls *-PLAN.md | wc -l` re-derivation with a `gsd_run query find-phase` call plus jq extraction of the count field, routing the plan count through scanPhasePlans (ADR-3180 §7.5) so superseded plans are excluded and the review converges on the live set." - } - } -} diff --git a/tests/emitted-drift-acks/3297-planphase-gaps-next-up-scope.json b/tests/emitted-drift-acks/3297-planphase-gaps-next-up-scope.json new file mode 100644 index 000000000..0deb1910f --- /dev/null +++ b/tests/emitted-drift-acks/3297-planphase-gaps-next-up-scope.json @@ -0,0 +1,6 @@ +{ + "version": 1, + "paths": { + "plan-phase.md": "#3297: the bash preamble now captures --gaps planning mode into GAPS_MODE and derives GAPS_EXEC_FLAG (empty or --gaps-only), and the Next Up command substitutes ${GAPS_EXEC_FLAG} into /gsd:execute-phase {X} so a completed --gaps planning run hands off execute-phase's matching gap-closure scope (--gaps-only) instead of the whole-phase run — the literal fix for the mode-blind handoff, plus the six-line gap-mode capture/derive block and the one-paragraph rendering note that tells the agent how to substitute the flag. Standard and --reviews runs leave the flag empty, so their rendered Next Up is byte-identical to before. Growth is the write side of the fix, not incidental prose. Supersedes the spent #3218 fragment (merged into next), which also named plan-phase.md and would otherwise double-ack the same path." + } +} diff --git a/tests/next-up-clear-order.test.cjs b/tests/next-up-clear-order.test.cjs index aa34c027c..7378b34a9 100644 --- a/tests/next-up-clear-order.test.cjs +++ b/tests/next-up-clear-order.test.cjs @@ -204,3 +204,94 @@ describe('bug #3083: resume-project next-step routing should not include /clear }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// #3297: plan-phase --gaps Next Up must project --gaps-only onto execute-phase +// ──────────────────────────────────────────────────────────────────────── + +describe('bug #3297: --gaps planning mode projects --gaps-only onto the Next Up execute command', () => { + const WORKFLOW = path.join(WORKFLOWS_DIR, 'plan-phase.md'); + + function offerNextBlock(content) { + // Anchor on the line-start tag: the prose reference `` `` `` + // ("Route to ") earlier in the file would otherwise match a + // bare indexOf('') first. + const start = content.indexOf('\n'); + const end = content.indexOf('\n', start); + assert.notStrictEqual(start, -1, 'plan-phase.md must have an block at line start'); + assert.notStrictEqual(end, -1, 'plan-phase.md must close at line start'); + return content.slice(start, end); + } + + const content = fs.readFileSync(WORKFLOW, 'utf-8'); + const offerNext = offerNextBlock(content); + + test('preamble captures --gaps planning mode into a persistent variable', () => { + // Precondition for any conditional render: the gap mode must survive from + // argument parsing to the completion screen. The file already captures + // --reviews / --chunked / --research-phase this way; --gaps must too. + assert.match( + content, + /GAPS_MODE=false[\s\S]*\$ARGUMENTS[\s\S]{0,80}?--gaps[\s\S]{0,40}?GAPS_MODE=true/, + 'plan-phase.md preamble must set GAPS_MODE from $ARGUMENTS --gaps (so the mode reaches )' + ); + }); + + test('preamble derives the execute-side --gaps-only flag (not the plan-side --gaps)', () => { + // The handoff must point at execute-phase's scope flag, which is --gaps-only + // (execute-phase.md:72/339/1426), NOT the plan-side --gaps. Asserting the + // literal guards against a copy-the-wrong-flag regression. + assert.match( + content, + /GAPS_EXEC_FLAG=""[\s\S]*GAPS_MODE[\s\S]{0,60}?GAPS_EXEC_FLAG="--gaps-only"/, + 'plan-phase.md preamble must derive GAPS_EXEC_FLAG="--gaps-only" from GAPS_MODE' + ); + }); + + test('the --gaps-only derivation is gated on GAPS_MODE alone (reviews mode unaffected)', () => { + // Acceptance: a --reviews planning run's Next Up is unchanged. The execute + // flag must be derived ONLY from --gaps, never from --reviews. Assert the + // derivation statement references GAPS_MODE and does NOT reference reviews. + const derivMatch = content.match(/GAPS_EXEC_FLAG=""[\s\S]{0,200}?GAPS_EXEC_FLAG="--gaps-only"/); + assert.ok(derivMatch, 'expected a GAPS_EXEC_FLAG derivation block'); + const deriv = derivMatch[0]; + assert.match(deriv, /GAPS_MODE/, 'derivation must be gated on GAPS_MODE'); + assert.doesNotMatch( + deriv, + /reviews/i, + 'derivation must not reference --reviews — reviews-mode Next Up stays unchanged' + ); + }); + + test('Next Up execute command substitutes the projected flag (${GAPS_EXEC_FLAG})', () => { + // The headline fix: the command line carries the flag placeholder so it + // renders --gaps-only in gap mode and the bare command otherwise. + assert.ok( + /\/gsd:execute-phase\s+\{X\}\s+\$\{GAPS_EXEC_FLAG\}/.test(offerNext), + ' execute-phase command must substitute ${GAPS_EXEC_FLAG} after {X}. Got:\n' + + offerNext.split('\n').filter((l) => /execute-phase/.test(l)).join('\n') + ); + }); + + test('Next Up still renders the bare command for standard runs (plain path unchanged)', () => { + // Acceptance: a standard (non-gap) run's Next Up is unchanged. The var is + // empty in the standard path, so the rendered command is byte-identical to + // today. Asserting the command still begins /gsd:execute-phase {X} (the bare + // form) and still carries ${GSD_WS} — the standard shape is preserved. + assert.match( + offerNext, + /\/gsd:execute-phase\s+\{X\}\s+\$\{GAPS_EXEC_FLAG\}\s+\$\{GSD_WS\}/, + ' command must keep the bare /gsd:execute-phase {X} form with ${GSD_WS}' + ); + }); + + test('Next Up block documents the --gaps-only projection (literal reaches the handoff)', () => { + // The gap-scoped flag literal must appear inside (via the + // rendering note that explains what ${GAPS_EXEC_FLAG} expands to), so a + // reader of the handoff sees the intended gap-closure scope. + assert.ok( + offerNext.includes('--gaps-only'), + ' must reference the --gaps-only execute-phase scope (the literal must reach the handoff)' + ); + }); +});