|
| 1 | +--- |
| 2 | +name: fix |
| 3 | +description: Verify and fix BLOCKING/HIGH code review findings from a prior review session, then run project verification. |
| 4 | +argument-hint: "<cr-dir>" |
| 5 | +--- |
| 6 | + |
| 7 | +# Fix Code Review Findings |
| 8 | + |
| 9 | +Single verify-and-fix pass for BLOCKING/HIGH findings from a prior code review. The caller controls re-review cycles. |
| 10 | + |
| 11 | +## Arguments |
| 12 | + |
| 13 | +$ARGUMENTS |
| 14 | + |
| 15 | +--- |
| 16 | + |
| 17 | +## Step 1: Parse Arguments and Load Findings |
| 18 | + |
| 19 | +```json |
| 20 | +TodoWrite([ |
| 21 | + {"content": "Parse arguments and load findings", "status": "in_progress", "activeForm": "Parsing arguments"}, |
| 22 | + {"content": "Verify findings", "status": "pending", "activeForm": "Verifying findings"}, |
| 23 | + {"content": "Fix confirmed findings", "status": "pending", "activeForm": "Fixing findings"}, |
| 24 | + {"content": "Run project verification", "status": "pending", "activeForm": "Running verification"}, |
| 25 | + {"content": "Print summary", "status": "pending", "activeForm": "Printing summary"} |
| 26 | +]) |
| 27 | +``` |
| 28 | + |
| 29 | +Extract **CR_DIR** (positional) from `$ARGUMENTS`. |
| 30 | + |
| 31 | +<constraints> |
| 32 | +- CR_DIR missing: auto-discover via `ls -td .closedloop-ai/code-review/cr-* | head -1`. No directories found → error "No code review session found. Run a code review first." → exit. |
| 33 | +- `CR_DIR/validate_output.json` missing → error "No validate_output.json found in CR_DIR. Run a code review first." → exit. |
| 34 | +</constraints> |
| 35 | + |
| 36 | +### Load Findings |
| 37 | + |
| 38 | +Read `CR_DIR/validate_output.json`. Each entry in the `validated` array: |
| 39 | +```json |
| 40 | +{"file": "path/to/file.ts", "line": 42, "severity": "HIGH", "category": "Correctness", "issue": "[P1] Brief title", "explanation": "...", "recommendation": "...", "code_snippet": "...", "priority": 1, "confidence": 0.9} |
| 41 | +``` |
| 42 | + |
| 43 | +Filter to `severity` = `"BLOCKING"` or `"HIGH"`. Log skipped MEDIUM count. |
| 44 | +No BLOCKING/HIGH findings → print "No actionable findings. Review complete." → mark all todos completed → exit. |
| 45 | + |
| 46 | +--- |
| 47 | + |
| 48 | +## Step 2: Verify Each Finding |
| 49 | + |
| 50 | +For each BLOCKING/HIGH finding, launch an `Agent` tool call with `subagent_type: "general-purpose"`, `model: "sonnet"`. Inline all finding values — no shell variables. |
| 51 | + |
| 52 | +Launch all verification agents **in parallel** (multiple Agent tool calls in a single message). |
| 53 | + |
| 54 | +**Prompt template:** |
| 55 | + |
| 56 | +``` |
| 57 | +<context> |
| 58 | +Verify whether this code review finding is a real bug or a false positive. |
| 59 | +File: {file} | Line: {line} | Severity: {severity} | Category: {category} |
| 60 | +Issue: {issue} |
| 61 | +Explanation: {explanation} |
| 62 | +Code snippet: {code_snippet} |
| 63 | +Recommendation: {recommendation} |
| 64 | +</context> |
| 65 | +
|
| 66 | +<instructions> |
| 67 | +Read the cited file and 50 lines of surrounding context. Reason through: |
| 68 | +1. PREMISE: What is this code supposed to do? (cite function/context) |
| 69 | +2. EVIDENCE: What concrete evidence proves or disproves the issue? (trace execution path, cite file:line) |
| 70 | +3. GUARD CHECK: Is there error handling or upstream logic that prevents this? (cite search result or "verified none exists at file:line") |
| 71 | +4. VERDICT: Real bug or false positive? |
| 72 | +
|
| 73 | +If analysis concludes the issue is NOT a problem, verdict MUST be REJECTED. |
| 74 | +</instructions> |
| 75 | +
|
| 76 | +<examples> |
| 77 | +CONFIRMED — proven bug: |
| 78 | +Input flows from req.body (line 12) → processData (line 30) → SQL query (line 47) with no escaping. Searched for sanitize/escape calls — none exist. |
| 79 | +{"verdict": "CONFIRMED", "reasoning": "Unsanitized user input flows directly into SQL query at line 47 with no upstream validation."} |
| 80 | +
|
| 81 | +REJECTED — false positive: |
| 82 | +Flagged null dereference at line 85, but variable is guarded by type check at line 78 (if (x !== null)) covering all paths to line 85. |
| 83 | +{"verdict": "REJECTED", "reasoning": "Null dereference prevented by type guard at line 78 covering all paths to line 85."} |
| 84 | +</examples> |
| 85 | +
|
| 86 | +<output_format> |
| 87 | +Output ONLY: {"verdict": "CONFIRMED"|"REJECTED", "reasoning": "...citing specific evidence..."} |
| 88 | +</output_format> |
| 89 | +``` |
| 90 | + |
| 91 | +### Collect Results |
| 92 | + |
| 93 | +Parse each response for `verdict` field. Keep only `"CONFIRMED"` findings. |
| 94 | +No confirmed findings → print "All findings were false positives. No fixes needed." → mark todos completed → exit. |
| 95 | + |
| 96 | +--- |
| 97 | + |
| 98 | +## Step 3: Fix Confirmed Findings |
| 99 | + |
| 100 | +Group confirmed findings by file. Fix **sequentially** (one agent at a time) — findings may share source files, so each fix must see prior results. |
| 101 | + |
| 102 | +For each finding/group, launch `Agent` with `subagent_type: "general-purpose"`, `model: "sonnet"`: |
| 103 | + |
| 104 | +``` |
| 105 | +Fix this code review finding. Minimal change only — no refactoring, no new features, no unnecessary error handling. |
| 106 | +File: {file} | Line: {line} |
| 107 | +Issue: {issue} |
| 108 | +Explanation: {explanation} |
| 109 | +Recommendation: {recommendation} |
| 110 | +
|
| 111 | +Read the file, apply the fix, confirm what changed. |
| 112 | +``` |
| 113 | + |
| 114 | +Track modified files for Step 5 summary. |
| 115 | + |
| 116 | +--- |
| 117 | + |
| 118 | +## Step 4: Run Project Verification |
| 119 | + |
| 120 | +Launch `Agent` with `subagent_type: "code:build-validator"`: |
| 121 | + |
| 122 | +``` |
| 123 | +Run all validation commands (test, lint, typecheck, build). Report VALIDATION_PASSED, VALIDATION_FAILED, or NO_VALIDATION. |
| 124 | +``` |
| 125 | + |
| 126 | +Do NOT run validation commands directly — the `build-validator` discovers and runs them. |
| 127 | + |
| 128 | +- **PASSED** or **NO_VALIDATION** → proceed to Step 5 |
| 129 | +- **FAILED** → launch `general-purpose` subagent (`model: "sonnet"`) to fix, re-run `build-validator`. Max 5 attempts. Warn and proceed on persistent failure. |
| 130 | + |
| 131 | +--- |
| 132 | + |
| 133 | +## Step 5: Summary |
| 134 | + |
| 135 | +Print a structured summary of the fix session: |
| 136 | + |
| 137 | +```markdown |
| 138 | +## Fix Summary |
| 139 | + |
| 140 | +| Metric | Value | |
| 141 | +|--------|-------| |
| 142 | +| Findings received | N | |
| 143 | +| Confirmed (not false positive) | N | |
| 144 | +| Fixed | N | |
| 145 | +| Remaining (warnings) | N | |
| 146 | +| Verification | PASSED/FAILED/NO_VALIDATION | |
| 147 | +| Modified files | file1, file2, ... | |
| 148 | +``` |
| 149 | + |
| 150 | +Mark all todos as `completed`. |
0 commit comments