Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 52 additions & 15 deletions .github/prompts/pr-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,58 @@ Review against each of these, in this order:

## Output discipline

- Post inline comments on specific lines where the issue is
concrete. Use top-level comments for architectural
concerns or praise.
- In the summary, separate findings by severity. End with
**exactly one of**: `Ship it`, `Iterate`, or `Block`,
followed by the single most important thing the author
must address.
- Use `Block` only when a CRITICAL/HIGH security finding,
data-loss risk, or broken core invariant is present. Use
`Iterate` for everything else that needs changes.
- Don't repeat what the linter already catches (gofumpt,
govet, staticcheck, gosec, gocritic, errcheck, etc. — see
`.golangci.yml`). CI enforces those.
- Don't suggest comments on self-explanatory code — this
project prefers well-named identifiers.
**Use inline review comments for specific line-level
Comment thread
EricAndrechek marked this conversation as resolved.
findings.** Call `mcp__github_inline_comment__create_inline_comment`
with `confirmed: true` for each concrete issue. These become
Comment thread
EricAndrechek marked this conversation as resolved.
real PR review threads that show next to the line in the diff,
must be resolved before merge (the repo's ruleset has
`required_review_thread_resolution: true`), and are the same
mechanism Gemini Code Assist uses. *Do not* dump every
finding into one giant prose blob — that pattern caused the
sticky comment to bloat.

**Tag every inline comment with exactly one severity** at the
start of the body: `[MUST]`, `[SHOULD]`, or `[MAY]`. This
matches `.gemini/styleguide.md` so both reviewers speak the
same language and the author can filter on tag.

- `[MUST]` — correctness bug, security issue, broken
invariant, missing required documentation sync. The PR
can't merge until this is addressed.
- `[SHOULD]` — quality / maintainability issue the author
should fix, but isn't a release blocker if they push back
with reasoning.
- `[MAY]` — minor suggestion, style nit, alternative
approach. Take or leave.

**Use the sticky summary comment for the verdict only.** One
short top-level comment with:
- A one-line headline grouping findings by severity (e.g.
"2 [MUST], 1 [SHOULD], 0 [MAY]").
- A pointer to read the inline threads for detail.
- The verdict line, **exactly one of**: `Ship it`,
`Iterate`, or `Block`, followed by the single most
important thing the author must address.

Verdict rules (matched to the styleguide):
- `Block` — a `[MUST]` finding that's a CRITICAL/HIGH
security issue, data-loss risk, or broken core
invariant. Cannot proceed without addressing.
- `Iterate` — one or more `[MUST]` findings that aren't
`Block`-level, or multiple `[SHOULD]` findings that
collectively need a pass.
- `Ship it` — no `[MUST]`s and few or no `[SHOULD]`s.
`[MAY]` findings alone don't preclude `Ship it`.

What not to comment on:
- Anything the linter already catches (gofumpt, govet,
staticcheck, gosec, gocritic, errcheck, etc. — see
`.golangci.yml`). CI enforces those.
- Self-explanatory code — this project prefers well-named
identifiers over explanatory comments.
- Don't post `gh pr comment` floors of prose for findings
that belong on a specific line — use an inline comment
there instead.

## Noise filter (important)

Expand Down
17 changes: 16 additions & 1 deletion .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,21 @@ jobs:
if: steps.gate.outputs.skip != 'true'
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
# One summary comment per PR — sticky so iteration history
# stays in one place rather than accreting "Claude reviewed
# again" comments per push. Specific findings go in inline
# review comments (see `claude_args` below + the prompt
# template) which become real review threads that count
# against the ruleset's `required_review_thread_resolution`
# — same blocking mechanism Gemini and Copilot's review
# comments use.
use_sticky_comment: true
track_progress: true
# `claude_args` allows the inline-comment MCP tool plus the
# narrow `gh pr` reads the prompt instructs Claude to use.
# Without an explicit allow-list, Claude Code's default
# tool set doesn't include the inline-comment MCP tool, so
# the model falls back to writing everything into the
# sticky comment.
claude_args: |
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh api repos/*/pulls/*/comments:*),Bash(gh api repos/*/issues/*/comments:*),Read,Glob,Grep"
prompt: ${{ steps.prompt.outputs.content }}
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ If you hand work to a subagent or another Claude session, tell them explicitly:

| Reviewer | How it runs | Re-runs on new commits | Blocks merge |
| -------- | ----------- | ---------------------- | ------------ |
| Claude (`.github/workflows/claude-review.yml`) | Our workflow, `workflow_run: [CI completed]` trigger — fires after every successful CI on a PR. Manual re-trigger via `@claude` or `/review` in a PR comment, or `gh workflow run "Claude PR review" -f pr_number=<N>` | Yes, auto — **updates the same sticky comment** rather than posting new ones | No (advisory) unless added to required checks |
| Claude (`.github/workflows/claude-review.yml`) | Our workflow, `pull_request: [opened, synchronize, reopened, ready_for_review]` trigger — fires on every PR push directly (not chained off CI). Manual re-trigger via `@claude` or `/review` in a PR comment, or `gh workflow run "Claude PR review" -f pr_number=<N>` | Yes, auto — **posts inline review comments** at specific lines (resolution required by the ruleset) plus a sticky verdict-summary top-level comment that edits in place across pushes | Yes for inline comments — the ruleset's `required_review_thread_resolution: true` blocks merge until each `claude[bot]` thread is resolved. The workflow's check itself is advisory |
| Gemini Code Assist | Marketplace App at repo level | Yes on synchronize, **but silently skips `.github/workflows/**`** (built-in exclusion, can't be overridden) — so Gemini reviews rarely see infra PRs | No (advisory) |
| Copilot | GitHub-native when reviewer has Copilot Pro enabled | Yes if enabled in Copilot settings | No (advisory) |
| Human admins (Eric / Taite) | Auto-assigned to the **PR** by `.github/workflows/project-orchestrator.yml` **only once** the PR is bot-clean: required checks (CI / PR housekeeping) green AND all review threads resolved. Both the `assignees` field AND a GitHub review-request are set together — the board's single-reviewer assignee is the queue signal, the review-request drives GitHub's native notification + dismiss-stale-reviews-on-push interaction (the two must stay in sync; an assignee without a review request would leave the reviewer uninformed mid-iteration). Draft PRs flip to ready at the same moment, PR card goes on Task Board (project #7) with Status=Ready, linked issues move to Status=In review. Reviewer selection (single admin): PR author == Eric → Taite; author == Taite → Eric; any other author → picked by PR-number parity (even → Eric, odd → Taite) to spread load. | Workflow re-checks on every push (`pull_request_target: synchronize`), review (`pull_request_review: submitted` — including `COMMENTED` reviews from bots like Gemini), and CI completion (`workflow_run: [CI completed]`). `check_suite` is **not** used: GitHub suppresses `check_suite` events when the suite was created by a `GITHUB_TOKEN`-triggered workflow (our CI), so the `workflow_run` trigger is the reliable chain-off signal. Idempotent across re-fires. `pull_request_review_thread` would be the natural trigger for thread-resolution events but GitHub Actions' parser rejects it; re-evaluation after thread resolution happens implicitly via the next push, bot review, or CI completion. | Yes — `.github/workflows/admin-approval.yml` is a required status check that fails unless Eric or Taite has an `APPROVED` review (Dependabot PRs bypass). |
Expand Down Expand Up @@ -333,7 +333,7 @@ docs/ → Project documentation
1. **Tier 1 — Issue triage** (`.github/workflows/triage.yml`): GitHub Models (`gpt-4o-mini` via `actions/ai-inference`) classifies new/edited issues and applies `area/*` + `security` + `breaking-change` labels. Optionally writes the `Priority` custom field on the Task Board (Project #7) when a `PROJECT_BOARD_TOKEN` secret with project scope is configured.
2. **Tier 2 — Code review** (two reviewers, both advisory; the `Admin approval` required status check + the ruleset are the actual merge-gate):
- **Gemini Code Assist App** configured via `.gemini/styleguide.md` — Marketplace App attached at the repo/org level, no workflow file.
- **Claude PR review** (`.github/workflows/claude-review.yml`) — `anthropics/claude-code-action` runs automatically after every successful CI on a PR, but only when the PR author is already OWNER/MEMBER/COLLABORATOR/CONTRIBUTOR to bound token cost. Dependabot and draft PRs are skipped. Manual re-trigger via `@claude` or `/review` in a PR comment from a trusted actor, or via `gh workflow run "Claude PR review" -f pr_number=<N>`. The workflow is review-only — Claude can comment but cannot push commits. Requires the `CLAUDE_CODE_OAUTH_TOKEN` secret (generated via `claude setup-token`).
- **Claude PR review** (`.github/workflows/claude-review.yml`) — `anthropics/claude-code-action` runs on every PR open / push, gated on the HEAD commit's author or committer having at least read permission on the repo (catches "admin pushed a fixup onto an external author's PR"). Dependabot is filtered at the workflow level. Claude posts findings as **inline review comments** (tagged `[MUST]` / `[SHOULD]` / `[MAY]` per the prompt template) plus a short sticky verdict summary; inline threads count against the ruleset's `required_review_thread_resolution`, so they block merge until resolved — same mechanism Gemini uses. Manual re-trigger via `@claude` or `/review` in a PR comment from a trusted actor, or via `gh workflow run "Claude PR review" -f pr_number=<N>`. The workflow is review-only — Claude can comment but cannot push commits. Requires the `CLAUDE_CODE_OAUTH_TOKEN` secret (generated via `claude setup-token`).

### Dependabot automation

Expand Down
Loading