diff --git a/review/SKILL.md b/review/SKILL.md index ea6f7b75..35075d16 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -36,6 +36,16 @@ Read `.claude/skills/review/checklist.md`. --- +## Step 2.5: Check for Greptile review comments + +Read `.claude/skills/review/greptile-triage.md` and follow the fetch, filter, and classify steps. + +**If no PR exists, `gh` fails, API returns an error, or there are zero Greptile comments:** Skip this step silently. Greptile integration is additive — the review works without it. + +**If Greptile comments are found:** Store the classifications (VALID & ACTIONABLE, VALID BUT ALREADY FIXED, FALSE POSITIVE, SUPPRESSED) — you will need them in Step 5. + +--- + ## Step 3: Get the diff Fetch the latest main to avoid false positives from a stale local main: @@ -68,6 +78,30 @@ Follow the output format specified in the checklist. Respect the suppressions - If only non-critical issues found: output findings. No further action needed. - If no issues found: output `Pre-Landing Review: No issues found.` +### Greptile comment resolution + +After outputting your own findings, if Greptile comments were classified in Step 2.5: + +**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)` + +1. **VALID & ACTIONABLE comments:** These are already included in your CRITICAL findings — they follow the same AskUserQuestion flow (A: Fix it now, B: Acknowledge, C: False positive). If the user chooses C (false positive), post a reply using the appropriate API from the triage doc and save the pattern to `~/.gstack/greptile-history.md` (type: fp). + +2. **FALSE POSITIVE comments:** Present each one via AskUserQuestion: + - Show the Greptile comment: file:line (or [top-level]) + body summary + permalink URL + - Explain concisely why it's a false positive + - Options: + - A) Reply to Greptile explaining why this is incorrect (recommended if clearly wrong) + - B) Fix it anyway (if low-effort and harmless) + - C) Ignore — don't reply, don't fix + + If the user chooses A, post a reply using the appropriate API from the triage doc and save the pattern to `~/.gstack/greptile-history.md` (type: fp). + +3. **VALID BUT ALREADY FIXED comments:** Reply acknowledging the catch — no AskUserQuestion needed: + - Post reply: `"Good catch — already fixed in ."` + - Save to `~/.gstack/greptile-history.md` (type: already-fixed) + +4. **SUPPRESSED comments:** Skip silently — these are known false positives from previous triage. + --- ## Important Rules diff --git a/ship/SKILL.md b/ship/SKILL.md index bbf72c81..ff6a2ca6 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -23,6 +23,7 @@ You are running the `/ship` workflow. This is a **non-interactive, fully automat - Test failures (stop, show failures) - Pre-landing review finds CRITICAL issues and user chooses to fix (not acknowledge or skip) - MINOR or MAJOR version bump needed (ask — see Step 4) +- Greptile review comments that need user decision (complex fixes, false positives) **Never stop for:** - Uncommitted changes (always include them) @@ -171,6 +172,43 @@ Save the review output — it goes into the PR body in Step 8. --- +## Step 3.75: Address Greptile review comments (if PR exists) + +Read `.claude/skills/review/greptile-triage.md` and follow the fetch, filter, and classify steps. + +**If no PR exists, `gh` fails, API returns an error, or there are zero Greptile comments:** Skip this step silently. Continue to Step 4. + +**If Greptile comments are found:** + +Include a Greptile summary in your output: `+ N Greptile comments (X valid, Y fixed, Z FP)` + +For each classified comment: + +**VALID & ACTIONABLE:** Use AskUserQuestion with: +- The comment (file:line or [top-level] + body summary + permalink URL) +- Your recommended fix +- Options: A) Fix now (recommended), B) Acknowledge and ship anyway, C) It's a false positive +- If user chooses A: apply the fix, commit the fixed files (`git add && git commit -m "fix: address Greptile review — "`), reply to the comment (`"Fixed in ."`), and save to `~/.gstack/greptile-history.md` (type: fix). +- If user chooses C: reply explaining the false positive, save to history (type: fp). + +**VALID BUT ALREADY FIXED:** Reply acknowledging the catch — no AskUserQuestion needed: +- Post reply: `"Good catch — already fixed in ."` +- Save to `~/.gstack/greptile-history.md` (type: already-fixed) + +**FALSE POSITIVE:** Use AskUserQuestion: +- Show the comment and why you think it's wrong (file:line or [top-level] + body summary + permalink URL) +- Options: + - A) Reply to Greptile explaining the false positive (recommended if clearly wrong) + - B) Fix it anyway (if trivial) + - C) Ignore silently +- If user chooses A: post reply using the appropriate API from the triage doc, save to history (type: fp) + +**SUPPRESSED:** Skip silently — these are known false positives from previous triage. + +**After all comments are resolved:** If any fixes were applied, the tests from Step 3 are now stale. **Re-run tests** (Step 3) before continuing to Step 4. If no fixes were applied, continue to Step 4. + +--- + ## Step 4: Version bump (auto-decide) 1. Read the current `VERSION` file (4-digit format: `MAJOR.MINOR.PATCH.MICRO`) @@ -275,6 +313,11 @@ gh pr create --title ": " --body "$(cat <<'EOF' ## Eval Results +## Greptile Review + + + + ## Test plan - [x] All Rails tests pass (N runs, 0 failures) - [x] All Vitest tests pass (N tests)