|
| 1 | +--- |
| 2 | +description: Address open PR review findings with enforced commit isolation between code fixes and review-file updates |
| 3 | +handoffs: |
| 4 | + - label: Re-Review Updated PR |
| 5 | + agent: devspark.pr-review |
| 6 | + prompt: Run /devspark.pr-review UPDATE for this PR after fixes are committed |
| 7 | +scripts: |
| 8 | + sh: pwsh -File .devspark/scripts/powershell/address-pr-review.ps1 -PrId $ARGUMENTS -Json |
| 9 | + ps: .devspark/scripts/powershell/address-pr-review.ps1 -PrId $ARGUMENTS -Json |
| 10 | +--- |
| 11 | + |
| 12 | +## User Input |
| 13 | + |
| 14 | +```text |
| 15 | +$ARGUMENTS |
| 16 | +``` |
| 17 | + |
| 18 | +You **MUST** consider the user input before proceeding (if not empty). |
| 19 | + |
| 20 | +## Overview |
| 21 | + |
| 22 | +This command is the **author-side companion** to `/devspark.pr-review`. It helps you address open findings in `/.documentation/specs/pr-review/pr-{PR_ID}.md` while enforcing commit isolation: |
| 23 | + |
| 24 | +1. Commit code fixes first. |
| 25 | +2. Commit review-file updates second. |
| 26 | + |
| 27 | +**IMPORTANT**: The staging gates are mechanical and mandatory. Do not bypass them. |
| 28 | + |
| 29 | +## Prerequisites |
| 30 | + |
| 31 | +- Existing PR review file at `/.documentation/specs/pr-review/pr-{PR_ID}.md` |
| 32 | +- Git repository with the PR source branch checked out |
| 33 | +- PowerShell 7+ (`pwsh`) available for the gate helper script |
| 34 | + |
| 35 | +## Outline |
| 36 | + |
| 37 | +### Phase 0 — Load context |
| 38 | + |
| 39 | +> **Script Resolution**: Before running `{SCRIPT}`, apply the 2-tier override check for PowerShell only — if `.documentation/scripts/powershell/address-pr-review.ps1` exists on disk, run that file instead, preserving all arguments. Team override in `.documentation/scripts/powershell/` takes priority over `.devspark/scripts/powershell/`. |
| 40 | +
|
| 41 | +1. Run `{SCRIPT}` with `-PrId {PR_ID} -Json`. |
| 42 | +2. Fail fast if `/.documentation/specs/pr-review/pr-{PR_ID}.md` is missing. |
| 43 | +3. Parse open findings from checklist lines matching: |
| 44 | + - `- [ ] **C-##**` |
| 45 | + - `- [ ] **H-##**` |
| 46 | + - `- [ ] **M-##**` |
| 47 | + - `- [ ] **L-##**` |
| 48 | + - `- [ ] **CON-##**` |
| 49 | +4. Confirm current branch equals the PR source branch. Refuse if mismatched. |
| 50 | +5. Capture `git status --short`. |
| 51 | +6. **Refuse to proceed** if any staged path matches `.documentation/specs/pr-review/pr-*.md`. |
| 52 | + |
| 53 | +If no open findings remain, print: `Nothing to address.` and stop. |
| 54 | + |
| 55 | +### Phase 1 — Plan |
| 56 | + |
| 57 | +1. Render open findings as a checklist with severity badges. |
| 58 | +2. Ask which findings to address this iteration (`all` allowed). |
| 59 | +3. Build an internal todo list with one item per selected finding. |
| 60 | + |
| 61 | +### Phase 2 — Fix loop (per finding) |
| 62 | + |
| 63 | +For each selected finding: |
| 64 | + |
| 65 | +1. Read the cited file/lines and confirm the issue. |
| 66 | +2. Apply the recommended fix, or propose an alternative and show the diff. |
| 67 | +3. Stage **only** code paths touched by that fix. |
| 68 | +4. Never run `git add .`. |
| 69 | +5. Never stage `/.documentation/specs/pr-review/pr-{PR_ID}.md` during this phase. |
| 70 | + |
| 71 | +### Phase 3 — Validate |
| 72 | + |
| 73 | +1. Re-run the **locked pytest scope** from the review file `Stats` table (reuse the same command; do not pick a new scope). |
| 74 | +2. Re-run project-specific validators explicitly recorded in the review file. |
| 75 | +3. If any validation fails, return to Phase 2. |
| 76 | +4. Do not continue until validations pass. |
| 77 | + |
| 78 | +### Phase 4 — Commit code fixes (isolation gate #1) |
| 79 | + |
| 80 | +1. Run gate script with `-Gate code-only` before commit. |
| 81 | +2. If the gate fails, **abort** and print offending staged paths. |
| 82 | +3. Review staged diff and commit with: |
| 83 | + |
| 84 | +```text |
| 85 | +fix(pr-{PR_ID}): address {M-02,M-04,M-05} |
| 86 | +``` |
| 87 | + |
| 88 | +1. Capture the resulting short hash as `{FIX_SHA}`. |
| 89 | + |
| 90 | +### Phase 5 — Update the review file |
| 91 | + |
| 92 | +For each fixed finding: |
| 93 | + |
| 94 | +1. Flip `- [ ]` to `- [x]`. |
| 95 | +2. Append `— *Fixed in {FIX_SHA}: {one-line how}*` to the finding heading line. |
| 96 | +3. Do **not** change finding IDs, descriptions, or broken/fix code blocks. |
| 97 | + |
| 98 | +Then update metadata: |
| 99 | + |
| 100 | +1. Bump revision in the header table (`Rev N -> Rev N+1`). |
| 101 | +2. Update `Stats` with current churn/test counts/commit snapshot. |
| 102 | +3. Append a new row to `Revision Log` for this iteration. |
| 103 | +4. Stage only `/.documentation/specs/pr-review/pr-{PR_ID}.md`. |
| 104 | + |
| 105 | +### Phase 6 — Commit review file (isolation gate #2) |
| 106 | + |
| 107 | +1. Run gate script with `-Gate review-only` before commit. |
| 108 | +2. If the gate fails, **abort** and print offending staged paths. |
| 109 | +3. Commit with: |
| 110 | + |
| 111 | +```text |
| 112 | +review(pr-{PR_ID}): rev {N} — {X} fixed, {Y} remaining |
| 113 | +``` |
| 114 | + |
| 115 | +1. Verify commit disjointness: |
| 116 | + |
| 117 | +```bash |
| 118 | +git log HEAD~2..HEAD --name-only |
| 119 | +``` |
| 120 | + |
| 121 | +Parse the two commits and assert they share zero file paths. |
| 122 | + |
| 123 | +### Phase 7 — Handoff |
| 124 | + |
| 125 | +1. Print both new commit hashes (`fix` and `review`). |
| 126 | +2. Suggest focused re-review: |
| 127 | + |
| 128 | +```text |
| 129 | +Run `/devspark.pr-review UPDATE {PR_URL}` to trigger a focused re-review. |
| 130 | +``` |
| 131 | + |
| 132 | +## Guidelines |
| 133 | + |
| 134 | +### Commit Discipline (MUST) |
| 135 | + |
| 136 | +- A commit touching `/.documentation/specs/pr-review/pr-{PR_ID}.md` MUST NOT include any other path. |
| 137 | +- Code fixes and review-file updates MUST be separate commits. |
| 138 | +- Do not amend/squash these two commits together. |
| 139 | + |
| 140 | +### Gate Execution (MUST) |
| 141 | + |
| 142 | +- Use the helper script as the source of truth for staging gates. |
| 143 | +- If a gate exits non-zero, stop and resolve staging before retrying. |
| 144 | + |
| 145 | +### Edit Scope (MUST) |
| 146 | + |
| 147 | +In review files, limit edits to: |
| 148 | + |
| 149 | +- finding checkbox state |
| 150 | +- heading-line fixed-in suffix |
| 151 | +- revision metadata (`Revision`, `Stats`, `Revision Log`) |
| 152 | + |
| 153 | +Everything else is immutable during addressing. |
| 154 | + |
| 155 | +## Context |
| 156 | + |
| 157 | +$ARGUMENTS |
| 158 | + |
| 159 | +## Shared Review Resolution Contract Output |
| 160 | + |
| 161 | +When emitting findings (review observations, issues, recommendations), structure each entry to include the shared resolution contract fields so downstream tools (/devspark.address-pr-review, telemetry, harvest) can act on them deterministically: |
| 162 | + |
| 163 | +```yaml |
| 164 | +findings: |
| 165 | + - finding_id: <stable-id-unique-within-this-command-output> # e.g., analyze-001, clarify-002 |
| 166 | + severity: critical | high | medium | low |
| 167 | + description: <1-3 sentence problem statement> |
| 168 | + recommended_action: <machine-actionable next step> |
| 169 | + execution_mode: auto | selective | manual |
| 170 | + status: open # set to `resolved` after remediation |
| 171 | + outcome: "" # populated post-resolution by address-pr-review |
| 172 | +``` |
| 173 | +
|
| 174 | +inding_id MUST be stable across re-runs when the underlying issue is unchanged. xecution_mode MUST be one of: `auto` (safe to apply automatically), `selective` (apply with reviewer approval), `manual` (requires human implementation). The `status` and `outcome` fields are written by `/devspark.address-pr-review` (FR-028). |
0 commit comments