Skip to content

Commit 15dcfeb

Browse files
ryzizubclaude
andcommitted
feat(review): add option to file findings on the PR as comments
Add a post-review option to /review Step 4 that posts selected findings to the branch's pull request. The user chooses which findings (all, critical+important, or specific FINDING-NN ids); each is posted as an inline comment anchored to its file:line via gh/glab, with a single summary comment as fallback when a line is not on the diff or no PR/CLI is available. Mechanism lives in a new shared reference (file-findings-on-pr.md), symlinked into the review skill; allowed-tools now permits gh/glab. Also fix the Step 4 pointer to the renamed "Act (by id or rule)" consolidation heading. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 47774b2 commit 15dcfeb

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

skills/review/SKILL.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ user-invocable: true
44
description: Runs quality review agents on demand — reviews code against VGV standards for architecture, tests, and simplicity, then writes one consolidated, numbered report.
55
when_to_use: Use when user says "review this code", "review my code", "code review", "review", "check this code", or "review before merging".
66
argument-hint: "[path/to/files/or/directories (optional)]"
7-
allowed-tools: Bash(*/scripts/detect-review-scope.sh)
7+
allowed-tools: Bash(*/scripts/detect-review-scope.sh) Bash(gh *) Bash(glab *)
88
effort: high
99
compatibility: Designed for Claude Code (or similar products with agent support)
1010
---
@@ -103,6 +103,9 @@ present post-review options (the ids and rules follow the consolidation procedur
103103
- **Fix critical + important**: same, plus Important findings.
104104
- **Fix specific findings**: accept ids from the user (e.g. "FINDING-01, FINDING-04"), or a
105105
rule id to act on a whole class (e.g. "fix every `tests/missing-test-file`").
106+
- **File findings on the PR as comments**: post selected findings to the branch's pull
107+
request, following [file findings on the PR](references/file-findings-on-pr.md) — it asks
108+
which findings to include and posts them as inline comments (with a summary fallback).
106109
- **Keep report and exit**: the report stays at `docs/code-review/<slug>/` for manual review.
107110

108111
**After fixing (if chosen):** re-run linter + test runner (no agent re-run), then present a
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../shared/references/file-findings-on-pr.md
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# File Findings on the PR
2+
3+
Post selected review findings to the branch's pull request as comments. Findings keep the
4+
`FINDING-NN` ids and `<category>/<rule>` from the [consolidation procedure](review-consolidation.md),
5+
so a PR comment maps back to the consolidated report unambiguously.
6+
7+
Outward-facing action: only post after the user has selected findings in Step 1 below. Never
8+
post without an explicit selection.
9+
10+
## Step 1 — Choose which findings
11+
12+
Use **AskUserQuestion**: "Which findings should I file on the PR?"
13+
14+
- **All** — every finding in the report.
15+
- **Critical + Important** — skip Suggestions.
16+
- **Pick specific** — present the findings as one or more multi-select lists (batches of up to
17+
four, by `FINDING-NN` id + title) and post only the checked ones.
18+
19+
If the user picks none, stop without posting.
20+
21+
## Step 2 — Detect the platform and PR
22+
23+
Run in parallel:
24+
25+
```bash
26+
gh --version 2>/dev/null
27+
glab --version 2>/dev/null
28+
```
29+
30+
Store the first available as `PR_CLI` (`gh`, `glab`, or `none`). Then find the PR for the
31+
current branch:
32+
33+
| `PR_CLI` | Command |
34+
| -------- | ------- |
35+
| `gh` | `gh pr view --json number,url,headRefOid` |
36+
| `glab` | `glab mr view` |
37+
| `none` | No CLI — go to the fallback in Step 4. |
38+
39+
If no PR exists for the branch, tell the user and offer the fallback (print the comments as
40+
markdown to paste manually). Do not create a PR from here.
41+
42+
## Step 3 — Post the comments
43+
44+
Prefer **inline** comments anchored to each finding's `file:line`. Comment out of these fields:
45+
46+
```markdown
47+
**FINDING-03 · 🟡 Important · `vgv/force-unwrap`**
48+
49+
Why: <why line>
50+
Fix: <fix line>
51+
```
52+
53+
### gh (GitHub)
54+
55+
Resolve `OWNER_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)`,
56+
`PR=$(gh pr view --json number -q .number)`, `SHA=$(gh pr view --json headRefOid -q .headRefOid)`.
57+
For each finding whose `location` is `file:line` and that line is part of the PR diff:
58+
59+
```bash
60+
gh api "repos/$OWNER_REPO/pulls/$PR/comments" \
61+
-f body="$COMMENT" -f commit_id="$SHA" -f path="$FILE" -F line=$LINE -f side=RIGHT
62+
```
63+
64+
### glab (GitLab)
65+
66+
Post each finding as a note on the MR: `glab mr note <id> -m "$COMMENT"` (include the
67+
`file:line` in the comment text, since inline positions need the diff SHAs).
68+
69+
### Fallback (no line on the diff, `PR_CLI=none`, or no PR)
70+
71+
Collect the leftover findings into **one** summary comment and post it once
72+
(`gh pr comment --body-file <file>` / `glab mr note -m ...`), or print it for manual paste
73+
when there is no CLI/PR. Group by severity; keep the ids.
74+
75+
## Step 4 — Report
76+
77+
Tell the user how many comments were posted, how many fell back to the summary, and the PR
78+
URL. If any `gh api` call failed (e.g. the line was not in the diff), report which findings
79+
and include them in the summary comment rather than dropping them.

0 commit comments

Comments
 (0)