From 999fbea7a3de3b4dbcc9fc6973a1935a32138fea Mon Sep 17 00:00:00 2001 From: Dicky Moore Date: Mon, 27 Apr 2026 17:20:17 +0100 Subject: [PATCH 1/4] fix: reconcile story and sprint status on closeout --- package.json | 2 +- .../bmad-code-review/steps/step-04-present.md | 10 +++ .../bmad-sprint-status/SKILL.md | 15 ++++- test/test-closeout-reconciliation.js | 66 +++++++++++++++++++ 4 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 test/test-closeout-reconciliation.js diff --git a/package.json b/package.json index 023b3c41f..3d44d917f 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "test": "npm run test:refs && npm run test:install && npm run test:channels && npm run lint && npm run lint:md && npm run format:check", "test:channels": "node test/test-installer-channels.js", "test:install": "node test/test-installation-components.js", - "test:refs": "node test/test-file-refs-csv.js", + "test:refs": "node test/test-file-refs-csv.js && node test/test-closeout-reconciliation.js", "validate:refs": "node tools/validate-file-refs.js --strict", "validate:skills": "node tools/validate-skills.js --strict" }, diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md index 1697c769c..b76448503 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md @@ -104,6 +104,14 @@ If `{sprint_status}` file exists: If `{sprint_status}` file does not exist, note that story status was updated in the story file only. +#### Final reconciliation + +Re-open the story file after saving and verify the top-level `Status:` field equals `{new_status}`. + +If `{sprint_status}` exists and `{story_key}` was found, re-open `{sprint_status}` after saving and verify `development_status[{story_key}]` also equals `{new_status}`. + +If either artifact does not match, HALT with a closeout reconciliation failure instead of reporting completion. + #### Completion summary > **Review Complete!** @@ -113,12 +121,14 @@ If `{sprint_status}` file does not exist, note that story status was updated in > **Action Items Created:** > **Deferred:** > **Dismissed:** +> **Reconciled:** story markdown and sprint tracker agree on `{new_status}` ### 7. Next steps Present the user with follow-up options: > **What would you like to do next?** +> > 1. **Start the next story** — run `dev-story` to pick up the next `ready-for-dev` story > 2. **Re-run code review** — address findings and review again > 3. **Done** — end the workflow diff --git a/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md b/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md index c52a84947..5b8d48608 100644 --- a/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md +++ b/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md @@ -64,9 +64,9 @@ Activation is complete. Begin the workflow below. ## Input Files -| Input | Path | Load Strategy | -|-------|------|---------------| -| Sprint status | `{sprint_status_file}` | FULL_LOAD | +| Input | Path | Load Strategy | +| ------------- | ---------------------- | ------------- | +| Sprint status | `{sprint_status_file}` | FULL_LOAD | ## Execution @@ -148,6 +148,7 @@ Enter corrections (e.g., "1=in-progress, 2=backlog") or "skip" to continue witho - IF any story has status "review": suggest `/bmad:bmm:workflows:code-review` - IF any story has status "in-progress" AND no stories have status "ready-for-dev": recommend staying focused on active story +- IF any story markdown file top-level `Status:` does not match its `development_status` entry: warn "story/tracker status drift detected" - IF all epics have status "backlog" AND no stories have status "ready-for-dev": prompt `/bmad:bmm:workflows:create-story` - IF `last_updated` timestamp is more than 7 days old (or `last_updated` is missing, fall back to `generated`): warn "sprint-status.yaml may be stale" - IF any story key doesn't match an epic pattern (e.g., story "5-1-..." but no "epic-5"): warn "orphaned story detected" @@ -289,6 +290,14 @@ If the command targets a story, set `story_key={{next_story_id}}` when prompted. Return +For each story entry in development_status, use `story_location` to open the matching story markdown file and compare its top-level `Status:` value with the tracker status when the tracker status is `review` or `done` + +is_valid = false +error = "Story/tracker status drift detected: {{drift_entries}}" +suggestion = "Reconcile story markdown Status fields with sprint-status.yaml before closeout" +Return + + is_valid = true message = "sprint-status.yaml valid: metadata complete, all statuses recognized" Run: `python3 {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow.on_complete` — if the resolved value is non-empty, follow it as the final terminal instruction before exiting. diff --git a/test/test-closeout-reconciliation.js b/test/test-closeout-reconciliation.js new file mode 100644 index 000000000..2998da48d --- /dev/null +++ b/test/test-closeout-reconciliation.js @@ -0,0 +1,66 @@ +/** + * Closeout reconciliation workflow tests + * + * Ensures the implementation workflows explicitly enforce reconciliation + * between story markdown Status fields and sprint-status.yaml before closeout. + * + * Usage: node test/test-closeout-reconciliation.js + */ + +const fs = require('node:fs'); + +const colors = { + reset: '\u001B[0m', + green: '\u001B[32m', + red: '\u001B[31m', + cyan: '\u001B[36m', + dim: '\u001B[2m', +}; + +let passed = 0; +let failed = 0; + +function assert(condition, testName, errorMessage = '') { + if (condition) { + console.log(`${colors.green}✓${colors.reset} ${testName}`); + passed++; + } else { + console.log(`${colors.red}✗${colors.reset} ${testName}`); + if (errorMessage) { + console.log(` ${colors.dim}${errorMessage}${colors.reset}`); + } + failed++; + } +} + +const codeReview = fs.readFileSync('src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md', 'utf8'); +const sprintStatus = fs.readFileSync('src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md', 'utf8'); + +console.log(`\n${colors.cyan}Closeout Reconciliation Tests${colors.reset}\n`); + +assert( + codeReview.includes('Re-open the story file after saving and verify the top-level `Status:` field equals `{new_status}`.'), + 'code-review reopens the story file to verify final status reconciliation', +); + +assert( + codeReview.includes('If either artifact does not match, HALT with a closeout reconciliation failure instead of reporting completion.'), + 'code-review halts when closeout reconciliation fails', +); + +assert(codeReview.includes('development_status[{story_key}]'), 'code-review verifies the sprint tracker entry during reconciliation'); + +assert( + codeReview.includes('story markdown and sprint tracker agree on `{new_status}`'), + 'code-review reports successful reconciliation in the completion summary', +); + +assert(sprintStatus.includes('story/tracker status drift detected'), 'sprint-status warns about story/tracker drift'); + +assert( + sprintStatus.includes('use `story_location` to open the matching story markdown file'), + 'sprint-status validate mode uses story_location when checking story files against tracker state', +); + +console.log(`\n${passed} passed, ${failed} failed\n`); +process.exit(failed > 0 ? 1 : 0); From 44796ab1168ccf2b986c058adba81723f9c7592f Mon Sep 17 00:00:00 2001 From: Dicky Moore Date: Mon, 27 Apr 2026 22:34:42 +0100 Subject: [PATCH 2/4] fix: tighten closeout reconciliation rules --- .../bmad-code-review/steps/step-04-present.md | 11 +++-- .../bmad-sprint-status/SKILL.md | 10 ++++- test/test-closeout-reconciliation.js | 41 ++++++++++++++++--- 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md index b76448503..fb8d84772 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md @@ -86,7 +86,8 @@ Skip this section if `{spec_file}` is not set. #### Determine new status based on review outcome -- If all `decision-needed` and `patch` findings were resolved (fixed or dismissed) AND no unresolved HIGH/MEDIUM issues remain: set `{new_status}` = `done`. Update the story file Status section to `done`. +- If the live journey release-gate closeout above found missing evidence, red gates, skipped gates, blocked gates, environment-blocked gates, or incomplete/expired product-owner deferrals: keep `{new_status}` = `in-progress` regardless of resolved findings. Update the story file Status section to `in-progress` and record the blocker in the story file. +- If all `decision-needed` and `patch` findings were resolved (fixed or dismissed) AND no unresolved HIGH/MEDIUM issues remain AND live-gate blockers are cleared (or have complete, unexpired product-owner deferral): set `{new_status}` = `done`. Update the story file Status section to `done`. - If `patch` findings were left as action items, or unresolved issues remain: set `{new_status}` = `in-progress`. Update the story file Status section to `in-progress`. Save the story file. @@ -108,9 +109,11 @@ If `{sprint_status}` file does not exist, note that story status was updated in Re-open the story file after saving and verify the top-level `Status:` field equals `{new_status}`. -If `{sprint_status}` exists and `{story_key}` was found, re-open `{sprint_status}` after saving and verify `development_status[{story_key}]` also equals `{new_status}`. +Set `{reconciliation_result}` = `story file verified; sprint tracker verification skipped`. -If either artifact does not match, HALT with a closeout reconciliation failure instead of reporting completion. +If `{sprint_status}` exists and `{story_key}` was found, re-open `{sprint_status}` after saving and verify `development_status[{story_key}]` also equals `{new_status}`. If it matches, set `{reconciliation_result}` = `story markdown and sprint tracker agree on {new_status}`. + +If the story file does not match `{new_status}`, or if `{sprint_status}` was verified and `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion. #### Completion summary @@ -121,7 +124,7 @@ If either artifact does not match, HALT with a closeout reconciliation failure i > **Action Items Created:** > **Deferred:** > **Dismissed:** -> **Reconciled:** story markdown and sprint tracker agree on `{new_status}` +> **Reconciled:** `{reconciliation_result}` ### 7. Next steps diff --git a/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md b/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md index 5b8d48608..0a94005db 100644 --- a/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md +++ b/src/bmm-skills/4-implementation/bmad-sprint-status/SKILL.md @@ -290,8 +290,14 @@ If the command targets a story, set `story_key={{next_story_id}}` when prompted. Return -For each story entry in development_status, use `story_location` to open the matching story markdown file and compare its top-level `Status:` value with the tracker status when the tracker status is `review` or `done` - +For each story entry in development_status where the tracker status is `review` or `done`: + +- Resolve the story path from `story_location` using `{project-root}` as the base for relative paths. +- Open the matching story markdown file and read the top-level `Status:` value. +- If the file is missing, unreadable, or the top-level `Status:` value is missing, record a drift entry with the path and reason. +- Compare the markdown `Status:` value with the tracker status and record mismatches as drift entries. + + is_valid = false error = "Story/tracker status drift detected: {{drift_entries}}" suggestion = "Reconcile story markdown Status fields with sprint-status.yaml before closeout" diff --git a/test/test-closeout-reconciliation.js b/test/test-closeout-reconciliation.js index 2998da48d..ca322de19 100644 --- a/test/test-closeout-reconciliation.js +++ b/test/test-closeout-reconciliation.js @@ -44,22 +44,51 @@ assert( ); assert( - codeReview.includes('If either artifact does not match, HALT with a closeout reconciliation failure instead of reporting completion.'), + codeReview.includes( + 'If the story file does not match `{new_status}`, or if `{sprint_status}` was verified and `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion.', + ), 'code-review halts when closeout reconciliation fails', ); assert(codeReview.includes('development_status[{story_key}]'), 'code-review verifies the sprint tracker entry during reconciliation'); assert( - codeReview.includes('story markdown and sprint tracker agree on `{new_status}`'), - 'code-review reports successful reconciliation in the completion summary', + codeReview.includes('Set `{reconciliation_result}` = `story file verified; sprint tracker verification skipped`.'), + 'code-review records when sprint tracker verification is skipped', ); -assert(sprintStatus.includes('story/tracker status drift detected'), 'sprint-status warns about story/tracker drift'); +assert( + codeReview.includes( + 'If the live journey release-gate closeout above found missing evidence, red gates, skipped gates, blocked gates, environment-blocked gates, or incomplete/expired product-owner deferrals: keep `{new_status}` = `in-progress` regardless of resolved findings.', + ), + 'code-review keeps live-gate blockers from being overwritten during final status selection', +); assert( - sprintStatus.includes('use `story_location` to open the matching story markdown file'), - 'sprint-status validate mode uses story_location when checking story files against tracker state', + codeReview.includes('> **Reconciled:** `{reconciliation_result}`'), + 'code-review reports reconciliation status conditionally in the completion summary', +); + +assert( + sprintStatus.includes('Resolve the story path from `story_location` using `{project-root}` as the base for relative paths.'), + 'sprint-status defines how story_location is resolved during validate mode', +); + +assert( + sprintStatus.includes( + 'If the file is missing, unreadable, or the top-level `Status:` value is missing, record a drift entry with the path and reason.', + ), + 'sprint-status treats missing story artifacts or missing Status values as validation failures', +); + +assert( + sprintStatus.includes('any drift_entries were recorded for review/done stories'), + 'sprint-status fails validate mode on any recorded reconciliation drift entries', +); + +assert( + sprintStatus.includes('story markdown Status fields with sprint-status.yaml before closeout'), + 'sprint-status reports reconciliation guidance when drift is detected', ); console.log(`\n${passed} passed, ${failed} failed\n`); From 8d807622807d2f96b82ce575764401c1d4518a9f Mon Sep 17 00:00:00 2001 From: Dicky Moore Date: Tue, 28 Apr 2026 08:36:00 +0100 Subject: [PATCH 3/4] test: separate workflow contract checks from refs --- package.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 3d44d917f..1304f79ca 100644 --- a/package.json +++ b/package.json @@ -41,10 +41,11 @@ "prepare": "command -v husky >/dev/null 2>&1 && husky || exit 0", "quality": "npm run format:check && npm run lint && npm run lint:md && npm run docs:build && npm run test:install && npm run validate:refs && npm run validate:skills", "rebundle": "node tools/installer/bundlers/bundle-web.js rebundle", - "test": "npm run test:refs && npm run test:install && npm run test:channels && npm run lint && npm run lint:md && npm run format:check", + "test": "npm run test:refs && npm run test:workflow-contracts && npm run test:install && npm run test:channels && npm run lint && npm run lint:md && npm run format:check", "test:channels": "node test/test-installer-channels.js", "test:install": "node test/test-installation-components.js", - "test:refs": "node test/test-file-refs-csv.js && node test/test-closeout-reconciliation.js", + "test:refs": "node test/test-file-refs-csv.js", + "test:workflow-contracts": "node test/test-closeout-reconciliation.js", "validate:refs": "node tools/validate-file-refs.js --strict", "validate:skills": "node tools/validate-skills.js --strict" }, From 54f0437922d12efe65a9f258717786f3eab5c63d Mon Sep 17 00:00:00 2001 From: Dicky Moore Date: Tue, 28 Apr 2026 08:55:33 +0100 Subject: [PATCH 4/4] fix: harden closeout reconciliation failure cases --- .../bmad-code-review/steps/step-04-present.md | 14 ++++++++--- test/test-closeout-reconciliation.js | 24 +++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md index fb8d84772..d5aec15aa 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md @@ -109,11 +109,19 @@ If `{sprint_status}` file does not exist, note that story status was updated in Re-open the story file after saving and verify the top-level `Status:` field equals `{new_status}`. -Set `{reconciliation_result}` = `story file verified; sprint tracker verification skipped`. +If the story file does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion. -If `{sprint_status}` exists and `{story_key}` was found, re-open `{sprint_status}` after saving and verify `development_status[{story_key}]` also equals `{new_status}`. If it matches, set `{reconciliation_result}` = `story markdown and sprint tracker agree on {new_status}`. +If `{sprint_status}` file does not exist, set `{reconciliation_result}` = `story file verified; sprint tracker not applicable`. -If the story file does not match `{new_status}`, or if `{sprint_status}` was verified and `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion. +If `{sprint_status}` exists but `{story_key}` was not found during sprint-status sync, HALT with a closeout reconciliation failure instead of reporting completion. + +If `{sprint_status}` exists and `{story_key}` was found, re-open `{sprint_status}` after saving and verify `development_status[{story_key}]` also equals `{new_status}`. + +If `development_status[{story_key}]` is missing, unreadable, or cannot be verified, HALT with a closeout reconciliation failure instead of reporting completion. + +If `development_status[{story_key}]` matches `{new_status}`, set `{reconciliation_result}` = `story markdown and sprint tracker agree on {new_status}`. + +If `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion. #### Completion summary diff --git a/test/test-closeout-reconciliation.js b/test/test-closeout-reconciliation.js index ca322de19..3a1ca84a6 100644 --- a/test/test-closeout-reconciliation.js +++ b/test/test-closeout-reconciliation.js @@ -45,16 +45,32 @@ assert( assert( codeReview.includes( - 'If the story file does not match `{new_status}`, or if `{sprint_status}` was verified and `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion.', + 'If the story file does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion.', ), - 'code-review halts when closeout reconciliation fails', + 'code-review halts when story markdown reconciliation fails', ); assert(codeReview.includes('development_status[{story_key}]'), 'code-review verifies the sprint tracker entry during reconciliation'); assert( - codeReview.includes('Set `{reconciliation_result}` = `story file verified; sprint tracker verification skipped`.'), - 'code-review records when sprint tracker verification is skipped', + codeReview.includes( + 'If `development_status[{story_key}]` is missing, unreadable, or cannot be verified, HALT with a closeout reconciliation failure instead of reporting completion.', + ), + 'code-review halts when the sprint tracker entry cannot be verified', +); + +assert( + codeReview.includes( + 'If `development_status[{story_key}]` does not match `{new_status}`, HALT with a closeout reconciliation failure instead of reporting completion.', + ), + 'code-review halts when sprint tracker reconciliation fails', +); + +assert( + codeReview.includes( + 'If `{sprint_status}` file does not exist, set `{reconciliation_result}` = `story file verified; sprint tracker not applicable`.', + ), + 'code-review treats skipped reconciliation as not applicable only when no sprint tracker exists', ); assert(