Compare commits

...

5 Commits

Author SHA1 Message Date
Alex Verkhovsky 74cea7401a
Merge branch 'main' into fix/docs-zh-cn-anchors 2026-03-20 11:08:30 -06:00
Alex Verkhovsky 182550407c
fix(code-review): update sprint-status to done after review completes (#2074)
* fix(code-review): update sprint-status to done after review completes

The code-review workflow ended without updating sprint-status.yaml from
"review" to "done", leaving stories stuck in review status. The dev-story
workflow implies code-review handles this transition but it was dropped
during the v6.2.0 step-file architecture refactor.

- Add sprint_status path to workflow initialization
- Track story_key in step-01 when discovered from sprint status
- Add step-04 section 6 to update sprint-status.yaml and story file
- Add step-04 section 7 with next-step options

Closes #2043

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(code-review): address PR review findings — split gating, story_key guard, HALT

- Split section 6 guard: story file status gated on spec_file only,
  sprint-status sync sub-gated on story_key separately
- Add conditional branch for manual choice in multi-story path so
  story_key is cleared when user declines a story selection
- Add HALT directive after Next steps menu to prevent LLM runaway

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 10:56:08 -06:00
dependabot[bot] 4b2389231f
chore(deps): bump h3 from 1.15.5 to 1.15.8 (#2064)
Bumps [h3](https://github.com/h3js/h3) from 1.15.5 to 1.15.8.
- [Release notes](https://github.com/h3js/h3/releases)
- [Changelog](https://github.com/h3js/h3/blob/main/CHANGELOG.md)
- [Commits](https://github.com/h3js/h3/compare/v1.15.5...v1.15.8)

---
updated-dependencies:
- dependency-name: h3
  dependency-version: 1.15.8
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alex Verkhovsky <alexey.verkhovsky@gmail.com>
2026-03-20 01:13:25 -06:00
Alex Verkhovsky 9088d4958b
fix(code-review): restore actionable review output with interactive choices (#2055)
* fix(code-review): restore actionable review output with interactive choices

The March 15 rewrite (PR #2007) removed the ability to auto-fix patches,
create action items in story files, and handle deferred/spec findings.
This restores interactive post-review actions:

- Deferred findings: auto-written to deferred-work.md and checked off in story
- Intent gap/bad spec: conversation with downgrade-to-patch, patch-spec,
  reset-to-ready-for-dev, or dismiss options
- Patch findings: fix automatically, create action items, or show details

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(code-review): simplify triage to decision-needed/patch/defer/dismiss

Replace 5-bucket classification (intent_gap, bad_spec, patch, defer, reject)
with 4 pragmatic buckets. Findings always written to story file first.
Decision-needed findings gate patch handling — resolve ambiguity before fixing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(code-review): address PR review findings in step-04-present

Replace undefined curly-brace placeholders with angle-bracket syntax,
add HALT guard before patch menu, guard spec_file references for
no-spec mode, and backtick category names for consistency.

* feat(code-review): add HALT guards, batch option, defer reason, final summary

Add strong HALT guards after decision-needed and patch menus to prevent
auto-progression. Add batch-apply option 0 for >3 patch findings. Prompt
for defer reason and append to story file and deferred-work.md. Show
boxed final summary with counts. Polish clean-review shortcut in triage.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 01:07:04 -06:00
Alex Verkhovsky 0d2863f77f
fix: separate subagent launch from skill invocation in code review (#2069)
* fix: separate subagent launch from skill invocation in code review

The step-02-review prompt fused "invoke skill X" with "in a subagent"
into one instruction, causing LLMs to search for a named agent instead
of launching a generic subagent that uses the skill. Aligns with the
working pattern in quick-dev step-04: upfront gate with inline fallback,
and "Invoke via the skill" as a separate concern from subagent setup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(code-review): address PR review findings on subagent fallback wording

Capitalize "Markdown" (proper noun) in Acceptance Auditor prompt and
simplify fallback trigger from "context-free subagents" to "subagents"
to eliminate ambiguity about when the fallback activates.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 01:04:51 -06:00
6 changed files with 132 additions and 47 deletions

6
package-lock.json generated
View File

@ -7230,9 +7230,9 @@
"license": "ISC"
},
"node_modules/h3": {
"version": "1.15.5",
"resolved": "https://registry.npmjs.org/h3/-/h3-1.15.5.tgz",
"integrity": "sha512-xEyq3rSl+dhGX2Lm0+eFQIAzlDN6Fs0EcC4f7BNUmzaRX/PTzeuM+Tr2lHB8FoXggsQIeXLj8EDVgs5ywxyxmg==",
"version": "1.15.8",
"resolved": "https://registry.npmjs.org/h3/-/h3-1.15.8.tgz",
"integrity": "sha512-iOH6Vl8mGd9nNfu9C0IZ+GuOAfJHcyf3VriQxWaSWIB76Fg4BnFuk4cxBxjmQSSxJS664+pgjP6e7VBnUzFfcg==",
"dev": true,
"license": "MIT",
"dependencies": {

View File

@ -2,6 +2,7 @@
diff_output: '' # set at runtime
spec_file: '' # set at runtime (path or empty)
review_mode: '' # set at runtime: "full" or "no-spec"
story_key: '' # set at runtime when discovered from sprint status
---
# Step 1: Gather Context
@ -23,8 +24,8 @@ review_mode: '' # set at runtime: "full" or "no-spec"
- When multiple phrases match, prefer the most specific match (e.g., "branch diff" over bare "diff").
- **If a clear match is found:** Announce the detected mode (e.g., "Detected intent: review staged changes only") and proceed directly to constructing `{diff_output}` using the corresponding sub-case from instruction 3. Skip to instruction 4 (spec question).
- **If no match from invocation text, check sprint tracking.** Look for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for any story with status `review`. Handle as follows:
- **Exactly one `review` story:** Suggest it: "I found story {{story-id}} in `review` status. Would you like to review its changes? [Y] Yes / [N] No, let me choose". If confirmed, use the story context to determine the diff source (branch name derived from story slug, or uncommitted changes). If declined, fall through to instruction 2.
- **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. Then use the selected story's context to determine the diff source as in the single-story case above, and proceed to instruction 3.
- **Exactly one `review` story:** Set `{story_key}` to the story's key (e.g., `1-2-user-auth`). Suggest it: "I found story {{story-id}} in `review` status. Would you like to review its changes? [Y] Yes / [N] No, let me choose". If confirmed, use the story context to determine the diff source (branch name derived from story slug, or uncommitted changes). If declined, clear `{story_key}` and fall through to instruction 2.
- **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. If the user selects a story, set `{story_key}` to the selected story's key and use the selected story's context to determine the diff source as in the single-story case above, and proceed to instruction 3. If the user selects the manual choice, clear `{story_key}` and fall through to instruction 2.
- **If no match and no sprint tracking:** Fall through to instruction 2.
2. HALT. Ask the user: **What do you want to review?** Present these options:

View File

@ -13,27 +13,20 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
## INSTRUCTIONS
1. Launch parallel subagents. Each subagent gets NO conversation history from this session:
1. If `{review_mode}` = `"no-spec"`, note to the user: "Acceptance Auditor skipped — no spec file provided."
- **Blind Hunter** -- Invoke the `bmad-review-adversarial-general` skill in a subagent. Pass `content` = `{diff_output}` only. No spec, no project access.
2. Launch parallel subagents without conversation context. If subagents are not available, generate prompt files in `{implementation_artifacts}` — one per reviewer role below — and HALT. Ask the user to run each in a separate session (ideally a different LLM) and paste back the findings. When findings are pasted, resume from this point and proceed to step 3.
- **Edge Case Hunter** -- Invoke the `bmad-review-edge-case-hunter` skill in a subagent. Pass `content` = `{diff_output}`. This subagent has read access to the project.
- **Blind Hunter** — receives `{diff_output}` only. No spec, no context docs, no project access. Invoke via the `bmad-review-adversarial-general` skill.
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) -- A subagent that receives `{diff_output}`, the content of the file at `{spec_file}`, and any loaded context docs. Its prompt:
> You are an Acceptance Auditor. Review this diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, contradictions between spec constraints and actual code. Output findings as a markdown list. Each finding: one-line title, which AC/constraint it violates, and evidence from the diff.
- **Edge Case Hunter** — receives `{diff_output}` and read access to the project. Invoke via the `bmad-review-edge-case-hunter` skill.
2. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) — receives `{diff_output}`, the content of the file at `{spec_file}`, and any loaded context docs. Its prompt:
> You are an Acceptance Auditor. Review this diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, contradictions between spec constraints and actual code. Output findings as a Markdown list. Each finding: one-line title, which AC/constraint it violates, and evidence from the diff.
3. If `{review_mode}` = `"no-spec"`, note to the user: "Acceptance Auditor skipped — no spec file provided."
3. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
4. **Fallback** (if subagents are not available): Generate prompt files in `{implementation_artifacts}` -- one per active reviewer:
- `review-blind-hunter.md` (always)
- `review-edge-case-hunter.md` (always)
- `review-acceptance-auditor.md` (only if `{review_mode}` = `"full"`)
HALT. Tell the user to run each prompt in a separate session and paste back findings. When findings are pasted, resume from this point and proceed to step 3.
5. Collect all findings from the completed layers.
4. Collect all findings from the completed layers.
## NEXT

View File

@ -30,19 +30,18 @@
- Set `source` to the merged sources (e.g., `blind+edge`).
3. **Classify** each finding into exactly one bucket:
- **intent_gap** -- The spec/intent is incomplete; cannot resolve from existing information. Only possible if `{review_mode}` = `"full"`.
- **bad_spec** -- The spec should have prevented this; spec is wrong or ambiguous. Only possible if `{review_mode}` = `"full"`.
- **patch** -- Code issue that is trivially fixable without human input. Just needs a code change.
- **decision_needed** -- There is an ambiguous choice that requires human input. The code cannot be correctly patched without knowing the user's intent. Only possible if `{review_mode}` = `"full"`.
- **patch** -- Code issue that is fixable without human input. The correct fix is unambiguous.
- **defer** -- Pre-existing issue not caused by the current change. Real but not actionable now.
- **reject** -- Noise, false positive, or handled elsewhere.
- **dismiss** -- Noise, false positive, or handled elsewhere.
If `{review_mode}` = `"no-spec"` and a finding would otherwise be `intent_gap` or `bad_spec`, reclassify it as `patch` (if code-fixable) or `defer` (if not).
If `{review_mode}` = `"no-spec"` and a finding would otherwise be `decision_needed`, reclassify it as `patch` (if the fix is unambiguous) or `defer` (if not).
4. **Drop** all `reject` findings. Record the reject count for the summary.
4. **Drop** all `dismiss` findings. Record the dismiss count for the summary.
5. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping rejects AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review.
5. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping dismissed AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review.
6. If zero findings remain after dropping rejects and no layers failed, note clean review.
6. If zero findings remain after triage (all rejected or none raised): state "✅ Clean review — all layers passed." (Step 3 already warned if any review layers failed via `{failed_layers}`.)
## NEXT

View File

@ -1,38 +1,129 @@
---
deferred_work_file: '{implementation_artifacts}/deferred-work.md'
---
# Step 4: Present
# Step 4: Present and Act
## RULES
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
- Do NOT auto-fix anything. Present findings and let the user decide next steps.
- When `{spec_file}` is set, always write findings to the story file before offering action choices.
- `decision-needed` findings must be resolved before handling `patch` findings.
## INSTRUCTIONS
1. Group remaining findings by category.
### 1. Clean review shortcut
2. Present to the user in this order (include a section only if findings exist in that category):
If zero findings remain after triage (all dismissed or none raised): state that and proceed to section 6 (Sprint Status Update).
- **Intent Gaps**: "These findings suggest the captured intent is incomplete. Consider clarifying intent before proceeding."
- List each with title + detail.
### 2. Write findings to the story file
- **Bad Spec**: "These findings suggest the spec should be amended. Consider regenerating or amending the spec with this context:"
- List each with title + detail + suggested spec amendment.
If `{spec_file}` exists and contains a Tasks/Subtasks section, append a `### Review Findings` subsection. Write all findings in this order:
- **Patch**: "These are fixable code issues:"
- List each with title + detail + location (if available).
1. **`decision-needed`** findings (unchecked):
`- [ ] [Review][Decision] <Title> — <Detail>`
- **Defer**: "Pre-existing issues surfaced by this review (not caused by current changes):"
- List each with title + detail.
2. **`patch`** findings (unchecked):
`- [ ] [Review][Patch] <Title> [<file>:<line>]`
3. Summary line: **X** intent_gap, **Y** bad_spec, **Z** patch, **W** defer findings. **R** findings rejected as noise.
3. **`defer`** findings (checked off, marked deferred):
`- [x] [Review][Defer] <Title> [<file>:<line>] — deferred, pre-existing`
4. If clean review (zero findings across all layers after triage): state that N findings were raised but all were classified as noise, or that no findings were raised at all (as applicable).
Also append each `defer` finding to `{deferred_work_file}` under a heading `## Deferred from: code review ({date})`. If `{spec_file}` is set, include its basename in the heading (e.g., `code review of story-3.3 (2026-03-18)`). One bullet per finding with description.
5. Offer the user next steps (recommendations, not automated actions):
- If `patch` findings exist: "These can be addressed in a follow-up implementation pass or manually."
- If `intent_gap` or `bad_spec` findings exist: "Consider running the planning workflow to clarify intent or amend the spec before continuing."
- If only `defer` findings remain: "No action needed for this change. Deferred items are noted for future attention."
### 3. Present summary
Workflow complete.
Announce what was written:
> **Code review complete.** <D> `decision-needed`, <P> `patch`, <W> `defer`, <R> dismissed as noise.
If `{spec_file}` is set, add: `Findings written to the review findings section in {spec_file}.`
Otherwise add: `Findings are listed above. No story file was provided, so nothing was persisted.`
### 4. Resolve decision-needed findings
If `decision_needed` findings exist, present each one with its detail and the options available. The user must decide — the correct fix is ambiguous without their input. Walk through each finding (or batch related ones) and get the user's call. Once resolved, each becomes a `patch`, `defer`, or is dismissed.
If the user chooses to defer, ask: Quick one-line reason for deferring this item? (helps future reviews): — then append that reason to both the story file bullet and the `{deferred_work_file}` entry.
**HALT** — I am waiting for your numbered choice. Reply with only the number (or "0" for batch). Do not proceed until you select an option.
### 5. Handle `patch` findings
If `patch` findings exist (including any resolved from step 4), HALT. Ask the user:
If `{spec_file}` is set, present all three options (if >3 `patch` findings exist, also show option 0):
> **How would you like to handle the <Z> `patch` findings?**
> 0. **Batch-apply all** — automatically fix every non-controversial patch (recommended when there are many)
> 1. **Fix them automatically** — I will apply fixes now
> 2. **Leave as action items** — they are already in the story file
> 3. **Walk through each** — let me show details before deciding
If `{spec_file}` is **not** set, present only options 1 and 3 (omit option 2 — findings were not written to a file). If >3 `patch` findings exist, also show option 0:
> **How would you like to handle the <Z> `patch` findings?**
> 0. **Batch-apply all** — automatically fix every non-controversial patch (recommended when there are many)
> 1. **Fix them automatically** — I will apply fixes now
> 2. **Walk through each** — let me show details before deciding
**HALT** — I am waiting for your numbered choice. Reply with only the number (or "0" for batch). Do not proceed until you select an option.
- **Option 0** (only when >3 findings): Apply all non-controversial patches without per-finding confirmation. Skip any finding that requires judgment. Present a summary of changes made and any skipped findings.
- **Option 1**: Apply each fix. After all patches are applied, present a summary of changes made. If `{spec_file}` is set, check off the items in the story file.
- **Option 2** (only when `{spec_file}` is set): Done — findings are already written to the story.
- **Walk through each**: Present each finding with full detail, diff context, and suggested fix. After walkthrough, re-offer the applicable options above.
**HALT** — I am waiting for your numbered choice. Reply with only the number (or "0" for batch). Do not proceed until you select an option.
**✅ Code review actions complete**
- Decision-needed resolved: <D>
- Patches handled: <P>
- Deferred: <W>
- Dismissed: <R>
### 6. Update story status and sync sprint tracking
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 `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.
#### Sync sprint-status.yaml
If `{story_key}` is not set, skip this subsection and note that sprint status was not synced because no story key was available.
If `{sprint_status}` file exists:
1. Load the FULL `{sprint_status}` file.
2. Find the `development_status` entry matching `{story_key}`.
3. If found: update `development_status[{story_key}]` to `{new_status}`. Update `last_updated` to current date. Save the file, preserving ALL comments and structure including STATUS DEFINITIONS.
4. If `{story_key}` not found in sprint status: warn the user that the story file was updated but sprint-status sync failed.
If `{sprint_status}` file does not exist, note that story status was updated in the story file only.
#### Completion summary
> **Review Complete!**
>
> **Story Status:** `{new_status}`
> **Issues Fixed:** <fixed_count>
> **Action Items Created:** <action_count>
> **Deferred:** <W>
> **Dismissed:** <R>
### 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
**HALT** — I am waiting for your choice. Do not proceed until the user selects an option.

View File

@ -44,6 +44,7 @@ Load and read full config from `{main_config}` and resolve:
- `project_name`, `planning_artifacts`, `implementation_artifacts`, `user_name`
- `communication_language`, `document_output_language`, `user_skill_level`
- `date` as system-generated current datetime
- `sprint_status` = `{implementation_artifacts}/sprint-status.yaml`
- `project_context` = `**/project-context.md` (load if exists)
- CLAUDE.md / memory files (load if exist)