Merge pull request #2104 from gsd-build/feat/1874-plan-bounce
feat(plan-phase): add plan bounce hook (step 12.5)
This commit is contained in:
@@ -25,6 +25,9 @@ const VALID_CONFIG_KEYS = new Set([
|
||||
'workflow.use_worktrees',
|
||||
'workflow.code_review',
|
||||
'workflow.code_review_depth',
|
||||
'workflow.plan_bounce',
|
||||
'workflow.plan_bounce_script',
|
||||
'workflow.plan_bounce_passes',
|
||||
'git.branching_strategy', 'git.base_branch', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template',
|
||||
'planning.commit_docs', 'planning.search_gitignored',
|
||||
'workflow.subagent_timeout',
|
||||
@@ -155,6 +158,9 @@ function buildNewProjectConfig(userChoices) {
|
||||
skip_discuss: false,
|
||||
code_review: true,
|
||||
code_review_depth: 'standard',
|
||||
plan_bounce: false,
|
||||
plan_bounce_script: null,
|
||||
plan_bounce_passes: 2,
|
||||
},
|
||||
hooks: {
|
||||
context_warnings: true,
|
||||
|
||||
@@ -11,7 +11,10 @@
|
||||
"security_asvs_level": 1,
|
||||
"security_block_on": "high",
|
||||
"discuss_mode": "discuss",
|
||||
"research_before_questions": false
|
||||
"research_before_questions": false,
|
||||
"plan_bounce": false,
|
||||
"plan_bounce_script": null,
|
||||
"plan_bounce_passes": 2
|
||||
},
|
||||
"planning": {
|
||||
"commit_docs": true,
|
||||
|
||||
@@ -46,7 +46,7 @@ Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_
|
||||
|
||||
## 2. Parse and Normalize Arguments
|
||||
|
||||
Extract from $ARGUMENTS: phase number (integer or decimal like `2.1`), flags (`--research`, `--skip-research`, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd <filepath>`, `--reviews`, `--text`).
|
||||
Extract from $ARGUMENTS: phase number (integer or decimal like `2.1`), flags (`--research`, `--skip-research`, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd <filepath>`, `--reviews`, `--text`, `--bounce`, `--skip-bounce`).
|
||||
|
||||
Set `TEXT_MODE=true` if `--text` is present in $ARGUMENTS OR `text_mode` from init JSON is `true`. When `TEXT_MODE` is active, replace every `AskUserQuestion` call with a plain-text numbered list and ask the user to type their choice number. This is required for Claude Code remote sessions (`/rc` mode) where TUI menus don't work through the Claude App.
|
||||
|
||||
@@ -930,6 +930,77 @@ Display: `Max iterations reached. {N} issues remain:` + issue list
|
||||
|
||||
Offer: 1) Force proceed, 2) Provide guidance and retry, 3) Abandon
|
||||
|
||||
## 12.5. Plan Bounce (Optional External Refinement)
|
||||
|
||||
**Skip if:** `--skip-bounce` flag, `--gaps` flag, or bounce is not activated.
|
||||
|
||||
**Activation:** Bounce runs when `--bounce` flag is present OR `workflow.plan_bounce` config is `true`. The `--skip-bounce` flag always wins (disables bounce even if config enables it). The `--gaps` flag also disables bounce (gap-closure mode should not modify plans externally).
|
||||
|
||||
**Prerequisites:** `workflow.plan_bounce_script` must be set to a valid script path. If bounce is activated but no script is configured, display warning and skip:
|
||||
```
|
||||
⚠ Plan bounce activated but no script configured.
|
||||
Set workflow.plan_bounce_script to the path of your refinement script.
|
||||
Skipping bounce step.
|
||||
```
|
||||
|
||||
**Read pass count:**
|
||||
```bash
|
||||
BOUNCE_PASSES=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get workflow.plan_bounce_passes --default 2)
|
||||
BOUNCE_SCRIPT=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get workflow.plan_bounce_script)
|
||||
```
|
||||
|
||||
Display banner:
|
||||
```
|
||||
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
GSD ► BOUNCING PLANS (External Refinement)
|
||||
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
|
||||
Script: ${BOUNCE_SCRIPT}
|
||||
Max passes: ${BOUNCE_PASSES}
|
||||
```
|
||||
|
||||
**For each PLAN.md file in the phase directory:**
|
||||
|
||||
1. **Backup:** Copy `*-PLAN.md` to `*-PLAN.pre-bounce.md`
|
||||
```bash
|
||||
cp "${PLAN_FILE}" "${PLAN_FILE%.md}.pre-bounce.md"
|
||||
```
|
||||
|
||||
2. **Invoke bounce script:**
|
||||
```bash
|
||||
"${BOUNCE_SCRIPT}" "${PLAN_FILE}" "${BOUNCE_PASSES}"
|
||||
```
|
||||
|
||||
3. **Validate bounced plan — YAML frontmatter integrity:**
|
||||
After the script returns, check that the bounced file still has valid YAML frontmatter (opening and closing `---` delimiters with parseable content between them). If the bounced plan breaks YAML frontmatter validation, restore the original from the pre-bounce.md backup and continue to the next plan:
|
||||
```
|
||||
⚠ Bounced plan ${PLAN_FILE} has broken YAML frontmatter — restoring original from pre-bounce backup.
|
||||
```
|
||||
|
||||
4. **Handle script failure:** If the bounce script exits non-zero, restore the original plan from the pre-bounce.md backup and continue to the next plan:
|
||||
```
|
||||
⚠ Bounce script failed for ${PLAN_FILE} (exit code ${EXIT_CODE}) — restoring original from pre-bounce backup.
|
||||
```
|
||||
|
||||
**After all plans are bounced:**
|
||||
|
||||
5. **Re-run plan checker on bounced plans:** Spawn gsd-plan-checker (same as step 10) on all modified plans. If a bounced plan fails the checker, restore original from its pre-bounce.md backup:
|
||||
```
|
||||
⚠ Bounced plan ${PLAN_FILE} failed checker validation — restoring original from pre-bounce backup.
|
||||
```
|
||||
|
||||
6. **Commit surviving bounced plans:** If at least one plan survived both the frontmatter validation and the checker re-run, commit the changes:
|
||||
```bash
|
||||
node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" commit "refactor(${padded_phase}): bounce plans through external refinement" --files "${PHASE_DIR}/*-PLAN.md"
|
||||
```
|
||||
|
||||
Display summary:
|
||||
```
|
||||
Plan bounce complete: {survived}/{total} plans refined
|
||||
```
|
||||
|
||||
**Clean up:** Remove all `*-PLAN.pre-bounce.md` backup files after the bounce step completes (whether plans survived or were restored).
|
||||
|
||||
## 13. Requirements Coverage Gate
|
||||
|
||||
After plans pass the checker (or checker is skipped), verify that all phase requirements are covered by at least one plan.
|
||||
|
||||
170
tests/plan-bounce.test.cjs
Normal file
170
tests/plan-bounce.test.cjs
Normal file
@@ -0,0 +1,170 @@
|
||||
/**
|
||||
* Plan Bounce Tests
|
||||
*
|
||||
* Validates plan bounce hook feature (step 12.5 in plan-phase):
|
||||
* - Config key registration (workflow.plan_bounce, workflow.plan_bounce_script, workflow.plan_bounce_passes)
|
||||
* - Config template defaults
|
||||
* - Workflow step 12.5 content in plan-phase.md
|
||||
* - Flag handling (--bounce, --skip-bounce)
|
||||
* - Backup/restore pattern (pre-bounce.md)
|
||||
* - Frontmatter integrity validation
|
||||
* - Re-runs checker on bounced plans
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const GSD_ROOT = path.join(__dirname, '..', 'get-shit-done');
|
||||
const CONFIG_CJS_PATH = path.join(GSD_ROOT, 'bin', 'lib', 'config.cjs');
|
||||
const CONFIG_TEMPLATE_PATH = path.join(GSD_ROOT, 'templates', 'config.json');
|
||||
const PLAN_PHASE_PATH = path.join(GSD_ROOT, 'workflows', 'plan-phase.md');
|
||||
|
||||
describe('Plan Bounce: config keys', () => {
|
||||
test('workflow.plan_bounce is in VALID_CONFIG_KEYS', () => {
|
||||
const content = fs.readFileSync(CONFIG_CJS_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes("'workflow.plan_bounce'"),
|
||||
'VALID_CONFIG_KEYS should contain workflow.plan_bounce'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow.plan_bounce_script is in VALID_CONFIG_KEYS', () => {
|
||||
const content = fs.readFileSync(CONFIG_CJS_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes("'workflow.plan_bounce_script'"),
|
||||
'VALID_CONFIG_KEYS should contain workflow.plan_bounce_script'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow.plan_bounce_passes is in VALID_CONFIG_KEYS', () => {
|
||||
const content = fs.readFileSync(CONFIG_CJS_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes("'workflow.plan_bounce_passes'"),
|
||||
'VALID_CONFIG_KEYS should contain workflow.plan_bounce_passes'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Plan Bounce: config template defaults', () => {
|
||||
test('config template has plan_bounce default (false)', () => {
|
||||
const template = JSON.parse(fs.readFileSync(CONFIG_TEMPLATE_PATH, 'utf-8'));
|
||||
assert.strictEqual(
|
||||
template.workflow.plan_bounce,
|
||||
false,
|
||||
'config template workflow.plan_bounce should default to false'
|
||||
);
|
||||
});
|
||||
|
||||
test('config template has plan_bounce_script default (null)', () => {
|
||||
const template = JSON.parse(fs.readFileSync(CONFIG_TEMPLATE_PATH, 'utf-8'));
|
||||
assert.strictEqual(
|
||||
template.workflow.plan_bounce_script,
|
||||
null,
|
||||
'config template workflow.plan_bounce_script should default to null'
|
||||
);
|
||||
});
|
||||
|
||||
test('config template has plan_bounce_passes default (2)', () => {
|
||||
const template = JSON.parse(fs.readFileSync(CONFIG_TEMPLATE_PATH, 'utf-8'));
|
||||
assert.strictEqual(
|
||||
template.workflow.plan_bounce_passes,
|
||||
2,
|
||||
'config template workflow.plan_bounce_passes should default to 2'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Plan Bounce: plan-phase.md step 12.5', () => {
|
||||
let content;
|
||||
|
||||
test('plan-phase.md contains step 12.5', () => {
|
||||
content = fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('## 12.5'),
|
||||
'plan-phase.md should contain step 12.5'
|
||||
);
|
||||
});
|
||||
|
||||
test('step 12.5 references plan bounce', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
// The step title should mention bounce
|
||||
assert.ok(
|
||||
/## 12\.5.*[Bb]ounce/i.test(content),
|
||||
'step 12.5 should reference plan bounce in its title'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md has --bounce flag handling', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('--bounce'),
|
||||
'plan-phase.md should handle --bounce flag'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md has --skip-bounce flag handling', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('--skip-bounce'),
|
||||
'plan-phase.md should handle --skip-bounce flag'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md has backup pattern (pre-bounce.md)', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('pre-bounce.md'),
|
||||
'plan-phase.md should reference pre-bounce.md backup files'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md has frontmatter integrity validation for bounced plans', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
// Should mention YAML frontmatter validation after bounce
|
||||
assert.ok(
|
||||
/frontmatter.*bounced|bounced.*frontmatter|YAML.*bounce|bounce.*YAML/i.test(content),
|
||||
'plan-phase.md should validate frontmatter integrity on bounced plans'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md re-runs checker on bounced plans', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
// Should mention re-running plan checker after bounce
|
||||
assert.ok(
|
||||
/[Rr]e-run.*checker.*bounce|bounce.*checker.*re-run|checker.*bounced/i.test(content),
|
||||
'plan-phase.md should re-run plan checker on bounced plans'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md references plan_bounce config keys', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('plan_bounce_script'),
|
||||
'plan-phase.md should reference plan_bounce_script config'
|
||||
);
|
||||
assert.ok(
|
||||
content.includes('plan_bounce_passes'),
|
||||
'plan-phase.md should reference plan_bounce_passes config'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md disables bounce when --gaps flag is present', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
// Should mention that --gaps disables bounce
|
||||
assert.ok(
|
||||
/--gaps.*bounce|bounce.*--gaps/i.test(content),
|
||||
'plan-phase.md should disable bounce when --gaps flag is present'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md restores original on script failure', () => {
|
||||
content = content || fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
|
||||
// Should mention restoring from backup on failure
|
||||
assert.ok(
|
||||
/restore.*original|restore.*pre-bounce|original.*restore/i.test(content),
|
||||
'plan-phase.md should restore original plan on script failure'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user