* fix(#3297): project --gaps mode onto next-up execute command * chore(#3297): set changeset pr to 3453 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/happy-voles-chatter.md
Normal file
5
.changeset/happy-voles-chatter.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3453
|
||||
---
|
||||
plan-phase: a completed --gaps planning run's Next Up handoff now recommends /gsd:execute-phase <N> --gaps-only (matching the gap-closure scope just planned) instead of the whole-phase /gsd:execute-phase <N>. Standard and --reviews runs are unchanged.
|
||||
@@ -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 `<offer_next>` (existing behavior).
|
||||
<offer_next>
|
||||
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}
|
||||
|
||||
───────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -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."
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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 <offer_next> 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."
|
||||
}
|
||||
}
|
||||
@@ -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 `` `<offer_next>` ``
|
||||
// ("Route to <offer_next>") earlier in the file would otherwise match a
|
||||
// bare indexOf('<offer_next>') first.
|
||||
const start = content.indexOf('\n<offer_next>');
|
||||
const end = content.indexOf('\n</offer_next>', start);
|
||||
assert.notStrictEqual(start, -1, 'plan-phase.md must have an <offer_next> block at line start');
|
||||
assert.notStrictEqual(end, -1, 'plan-phase.md <offer_next> 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 <offer_next>)'
|
||||
);
|
||||
});
|
||||
|
||||
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),
|
||||
'<offer_next> 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\}/,
|
||||
'<offer_next> 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 <offer_next> (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'),
|
||||
'<offer_next> must reference the --gaps-only execute-phase scope (the literal must reach the handoff)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user