Skip to content

Commit b341488

Browse files
committed
chore: sync review-loop skill (rollback structured findings)
1 parent ce5d7b6 commit b341488

1 file changed

Lines changed: 23 additions & 128 deletions

File tree

.claude/skills/review-loop/SKILL.md

Lines changed: 23 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,8 @@ You are an autonomous coding agent running a review loop. You trigger Command Ce
2424
- If neither exists, tell the user to set `CC_API_SECRET` in their shell profile and stop.
2525

2626
```bash
27-
# Read CC_API_SECRET from env, fall back to .env.local (handles quoted values)
28-
echo "${CC_API_SECRET:-$(python3 -c "
29-
for line in open('.env.local'):
30-
if line.strip().startswith('INTERNAL_API_SECRET='):
31-
print(line.strip().split('=',1)[1].strip('\"').strip(\"'\"))
32-
" 2>/dev/null)}"
33-
```
34-
35-
```bash
36-
# Read APP_URL from env, fall back to .env.local (handles quoted values)
37-
echo "${CC_APP_URL:-$(python3 -c "
38-
for line in open('.env.local'):
39-
if line.strip().startswith('APP_URL='):
40-
print(line.strip().split('=',1)[1].strip('\"').strip(\"'\"))
41-
" 2>/dev/null)}"
27+
# Read CC_API_SECRET from env, fall back to .env.local
28+
echo "${CC_API_SECRET:-$(grep INTERNAL_API_SECRET .env.local 2>/dev/null | cut -d= -f2-)}"
4229
```
4330

4431
2. Detect repo and branch from git:
@@ -58,83 +45,41 @@ for line in open('.env.local'):
5845
4. Build the prompt text. Check these sources in order:
5946
- If the user provided `$ARGUMENTS`, use that as the prompt.
6047
- Otherwise, look for a plan file in `docs/plans/` or `.ai-docs/plans/` that mentions the branch name.
61-
- Otherwise, extract a prompt from the PR title and body:
62-
```bash
63-
gh pr view --json title,body -q '"Review: " + .title + "\n\nPR Description:\n" + .body'
64-
```
65-
- If `gh` fails or returns empty, use a generic prompt: "Review the code changes on this branch for quality and merge readiness."
48+
- Otherwise, read the PR body: `gh pr view --json body -q .body`
49+
- If nothing useful is found, use a generic prompt: "Review the code changes on this branch for quality and merge readiness."
6650

6751
## Phase 2: Trigger the Review
6852

69-
Call the trigger API using `curl`, saving the response to a file. Use `python3` to parse the saved JSON (never pipe curl to python3 — connection hiccups cause empty stdin and `JSONDecodeError`).
53+
Call the trigger API using `curl`. Use `python3 -c "import sys,json; ..."` for JSON parsing (jq may not be available).
7054

7155
```bash
7256
curl -s -X POST "${APP_URL}/api/orchestration/trigger" \
7357
-H "Content-Type: application/json" \
7458
-H "x-internal-api-secret: ${API_SECRET}" \
75-
-d '{"action":"start","repo":"REPO","branch":"BRANCH","prNumber":PR_NUM,"prompt":"PROMPT_TEXT"}' \
76-
-o /tmp/cc-trigger.json
59+
-d '{"action":"start","repo":"REPO","branch":"BRANCH","prNumber":PR_NUM,"prompt":"PROMPT_TEXT"}'
7760
```
7861

79-
```bash
80-
python3 -c "import json; data=json.load(open('/tmp/cc-trigger.json')); print(data.get('sessionId', data.get('error', 'Unknown response')))"
81-
```
82-
83-
Extract `sessionId` from the parsed response. If the response contains an error, show it and stop.
62+
Extract `sessionId` from the JSON response using python3. If the response contains an error, show it and stop.
8463

8564
Tell the user: "Review triggered (session: SESSION_ID). Waiting for results..."
8665

8766
## Phase 3: Poll for Results
8867

8968
Poll the status endpoint. **Wait 30 seconds between polls.** Use `sleep 30` between each curl call.
9069

91-
Save the response to a file, then parse it:
92-
9370
```bash
9471
curl -s "${APP_URL}/api/orchestration/status?repo=REPO&sessionId=SESSION_ID&orgId=legacy-default" \
95-
-H "x-internal-api-secret: ${API_SECRET}" \
96-
-o /tmp/cc-review-status.json
97-
```
98-
99-
```bash
100-
python3 -c "
101-
import json
102-
data = json.load(open('/tmp/cc-review-status.json'))
103-
state = data.get('state', {})
104-
phase = state.get('phase', 'unknown')
105-
print(f'Phase: {phase}')
106-
if state.get('error'):
107-
print(f'Error: {state[\"error\"]}')
108-
if state.get('lastLogMessage'):
109-
print(f'Log: {state[\"lastLogMessage\"]}')
110-
"
72+
-H "x-internal-api-secret: ${API_SECRET}"
11173
```
11274

113-
Check `state.phase` against this table:
114-
115-
| Phase | Action |
116-
|----------------------|-----------------------------------------------|
117-
| `completed` | Go to Phase 4 (success) |
118-
| `failed` | Go to Phase 4 (failure) |
119-
| `waiting_for_nudge` | Go to Phase 5 (fix blocking issues) |
120-
| `pending` | Just started — poll again after 30s |
121-
| `routing` | Auto-detecting Vercel project — poll again |
122-
| `dispatching` | Starting checks — poll again |
123-
| `executing` | Running analysis — poll again |
124-
| `verifying_build` | Checking Vercel build — poll again |
125-
| `quality_check` | Running quality review — poll again |
126-
| `reviewing` | Running merge readiness — poll again |
127-
| Anything else | Show phase + latest log message, poll again |
75+
Parse the JSON response with python3. Check `state.phase`:
12876

129-
After parsing poll results, print a progress summary:
77+
- `completed` → Go to Phase 4 (success)
78+
- `failed` → Go to Phase 4 (failure)
79+
- `executing` AND the logs mention "waiting for fix" → Go to Phase 5 (fix issues)
80+
- Anything else → Show the current phase and latest log message, then poll again after 30s
13081

131-
```
132-
Iteration {N} — Phase: {phase}
133-
Blocking: {count} | Resolved: {delta.resolvedCount} | New: {delta.newCount} | Regressed: {delta.regressedCount}
134-
Stale: {findingsStale} | Commit: {analyzedCommitSha}
135-
```
136-
137-
**Keep polling until you reach one of the terminal/actionable states.** Do NOT give up after a few polls.
82+
**Keep polling until you reach one of the above terminal/actionable states.** Do NOT give up after a few polls.
13883

13984
## Phase 4: Terminal States
14085

@@ -155,38 +100,10 @@ Tell the user:
155100

156101
**This is the most important phase. You are an autonomous agent — fix the issues yourself.**
157102

158-
Print a progress summary at the start of each iteration:
159-
160-
```
161-
Iteration {N} — Phase: {phase}
162-
Blocking: {count} | Resolved: {delta.resolvedCount} | New: {delta.newCount} | Regressed: {delta.regressedCount}
163-
Stale: {findingsStale} | Commit: {analyzedCommitSha}
164-
```
165-
166-
1. **Extract findings** from the status response. Prefer structured findings when available:
167-
- `state.structuredFindings` — array of structured findings with `findingId`, `title`, `severity`, `evidence`, `file`, `line`, `acceptanceCriteria`, and `status`
168-
- `state.findingDelta` — shows `newCount`, `resolvedCount`, `regressedCount`, `remainingCount` since last iteration
169-
- `state.findingsStale` — if `true`, analysis is in progress; wait and re-poll before acting
170-
- **Fallback** (if structuredFindings is null): `state.lastMergeResult.blockingIssues`, `state.lastMergeResult.summary`, `state.lastQualityResult.findings`
171-
172-
### Triage Findings Before Fixing
173-
174-
Not all blocking findings are your responsibility. Triage each finding before acting:
175-
176-
- **Check `firstSeenIteration`**: If a finding's `firstSeenIteration` is > 1, it pre-dates your changes and is likely a pre-existing issue.
177-
- **Check if the file was modified by you**: Run `git diff --name-only HEAD~1` to get your modified files. If a finding's `file` is NOT in that list, it's likely pre-existing.
178-
- **Priority order**: Fix findings in YOUR modified files first. Findings in untouched files are likely pre-existing — skip them with a justification commit (see false-positive escape below).
179-
- **High `newCount` but all in unmodified files?** That's pre-existing noise, not your fault. Skip with justification.
180-
181-
### Interpret `findingDelta` to Guide Your Actions
182-
183-
Use the delta to understand what happened since your last push:
184-
185-
- `resolvedCount > 0` → Your last push fixed things. Good progress — keep going.
186-
- `newCount > 0` in YOUR files → Your fix introduced new issues. Address these.
187-
- `newCount > 0` in unmodified files → Pre-existing issues surfaced by deeper analysis. Consider skipping.
188-
- `regressedCount > 0` → Previously resolved findings came back. Check if you accidentally reverted something.
189-
- `remainingCount == 0 && newCount == 0` → All issues addressed. This iteration will likely pass.
103+
1. **Extract findings** from the status response:
104+
- `state.lastMergeResult.blockingIssues` — array of blocking issue descriptions
105+
- `state.lastMergeResult.summary` — detailed markdown summary with findings
106+
- `state.lastQualityResult.findings` — quality check findings text
190107

191108
2. **Read and understand each blocking issue.** Common issue types:
192109
- **Code bugs** (e.g., case-sensitivity, missing error handling) → Fix the code
@@ -196,7 +113,7 @@ Iteration {N} — Phase: {phase}
196113

197114
3. **Fix each issue** using your normal code editing tools (Read, Edit, Write). Do thorough fixes — don't just add comments or TODOs.
198115

199-
4. **Commit and push** the fixes. Use descriptive commit messages:
116+
4. **Commit and push** the fixes:
200117
```bash
201118
git add <specific-files-you-changed>
202119
```
@@ -207,48 +124,26 @@ Iteration {N} — Phase: {phase}
207124
git push
208125
```
209126

210-
**False-positive escape**: If you determine a blocking finding is a false positive (the code is correct and the reviewer is wrong), you can skip it by pushing a commit with this message format:
211-
```
212-
review: skip finding <findingId> — <brief technical justification>
213-
```
214-
The reviewer will evaluate your justification. If it's valid, the finding will be resolved automatically. If not, it will be re-flagged at high confidence. Only use this for genuine false positives — do not abuse it to skip real issues.
215-
216-
**Batching skips**: If multiple findings are false positives, batch all skips into a single commit with each on its own line:
217-
```
218-
review: skip findings
219-
220-
skip finding <findingId1> — <justification1>
221-
skip finding <findingId2> — <justification2>
222-
skip finding <findingId3> — <justification3>
223-
```
224-
225-
5. **Verify push and wait for bot reviews.** The quality check and merge readiness analysis read PR review comments — bot reviewers (linters, security scanners) need time to post theirs after a new push.
226-
```bash
227-
PUSHED_SHA=$(git rev-parse --short HEAD)
228-
```
229-
Tell the user: "Pushed $PUSHED_SHA, waiting 3 minutes for bot reviews before nudging re-review..."
127+
5. **Wait 3 minutes for CI and bot reviews** to process the new commit. Bot reviewers on the PR need time to analyze the push before Command Center re-reviews:
230128
```bash
231129
sleep 180
232130
```
233131

234-
6. **Nudge the orchestrator** to re-review. Save the response to a file:
132+
6. **Nudge the orchestrator** to re-review:
235133
```bash
236134
curl -s -X POST "${APP_URL}/api/orchestration/trigger" \
237135
-H "Content-Type: application/json" \
238136
-H "x-internal-api-secret: ${API_SECRET}" \
239-
-d '{"action":"fix_pushed","sessionId":"SESSION_ID"}' \
240-
-o /tmp/cc-nudge.json
137+
-d '{"action":"fix_pushed","sessionId":"SESSION_ID"}'
241138
```
242139

243-
Tell the user: "Pushed $PUSHED_SHA, nudging re-review."
244-
245140
7. **Go back to Phase 3** (poll again). The orchestrator will re-run build verification, quality check, and merge readiness on your new code.
246141

247142
**Repeat Phase 3→5 until the review passes or fails permanently.**
248143

249144
## Important Notes
250145

251-
- **Do NOT use `jq`** — it may not be available. Use `python3` to parse JSON from saved files: `python3 -c "import json; data=json.load(open('/tmp/cc-review-status.json')); print(data['key'])"`. Never pipe curl output directly to python3 — save to a file first to avoid `JSONDecodeError` on connection hiccups.
146+
- **Do NOT use `jq`** — it may not be available. Use `python3 -c "import sys,json; data=json.load(sys.stdin); print(data['key'])"` for JSON parsing.
252147
- **Do NOT run long-lived bash loops** — poll by making individual curl calls with `sleep 30` between them.
253148
- **Always push before nudging** — the orchestrator checks the latest commit on the branch.
254149
- **Be thorough with fixes** — superficial fixes will just fail the next review cycle.

0 commit comments

Comments
 (0)