feat(ci): give the review bot a severity scale and a tally

REVIEW.md said what blocks and what does not, but never how to mark a finding,
so every review invented its own shape and none carried a severity. It now
names the three markers the hosted Code Review service uses - Important, Nit,
Pre-existing - and keys them to what the pull request did rather than to how
alarming a defect looks alone: a defect it introduces or worsens is Important,
one it merely brought into view is Pre-existing and cannot be a reason to hold
it. Pre-existing was missing entirely, and checking what this panel emits means
reading far outside the diff, so those findings had nowhere to go except a
wrong Important or silence.

The volume cap said how many and never which. It now collapses a nit repeated
across files into one finding, prefers a nit in code the pull request wrote
over one in code it only moved, caps pre-existing findings at three, and states
that Important findings are never capped - a section listing two caps otherwise
reads as licence to trim what matters. The review opens with a tally so the
author sees the shape before the detail.

Two contradictions went with it. The file told the reviewer to skip what CI
enforces and then to check that a new i18n key reaches all 13 locales, which
i18n-dead-keys.test.ts pins in both directions - the rule moves to "Do not
report" with the reason. "Anything CI already enforces: npm audit" overstated
what runs; CI audits production dependencies at high and above, so a
dev-dependency advisory is out of scope by design.

The reviewer could not read its own CI. Only postgres-durable-first runs
against PostgreSQL, and XRAY_E2E_BINARY and XUI_SCALE_TEST are set by no job,
so a dialect or migration change can carry a wall of green while the paths it
touches never executed. That belongs to the verification bar, next to the rule
that a behaviour claim needs a file:line citation, and "CI passed" now needs a
run actually read. Also names the two house choices no linter defends: neither
golangci-lint nor oxlint rejects a testify or Tailwind import.

Both kinds of claim rot on a rename, so a test pins them the way
repo-context.md's claims are already pinned - the CI jobs REVIEW.md names must
exist in ci.yml, the skip gates it calls unset must stay unset, and the locale
count must match the directory.

The review itself moves from high to max effort, and the prompt records why it
names REVIEW.md at all: the code-review skill reads CLAUDE.md on its own but
not REVIEW.md, so dropping that clause would silently stop the file applying.
Drops a CLAUDE.md reference to tools/seedperf/, which no longer exists - the
review reads that file as project context, so a stale path there misleads it.

.github/workflows/claude-bot.yml

@@ -494,6 +494,16 @@ jobs:
       - uses: actions/checkout@v7
         with:
           persist-credentials: false
+      # Read-only: this job holds a write-scoped token, so building or running
+      # anything out of pr-head/ would turn the review into a pwn-request.
+      # checkout v7 refuses a fork PR ref outright unless that risk is accepted
+      # here, and nearly every pull request to this repository is from a fork.
+      - uses: actions/checkout@v7
+        with:
+          ref: refs/pull/${{ github.event.pull_request.number || github.event.issue.number }}/head
+          path: pr-head
+          persist-credentials: false
+          allow-unsafe-pr-checkout: true
       - uses: anthropics/claude-code-action@v1
         with:
           github_token: ${{ secrets.GITHUB_TOKEN }}
@@ -501,13 +511,18 @@ jobs:
           allowed_non_write_users: "*"
           plugin_marketplaces: "https://github.com/anthropics/claude-code.git"
           plugins: "code-review@claude-code-plugins"
-          prompt: "/code-review:code-review --comment ${{ github.repository }}/pull/${{ github.event.pull_request.number || github.event.issue.number }}"
+          # The skill reads CLAUDE.md on its own but NOT REVIEW.md - that file
+          # reaches a review only through the append-system-prompt below.
+          prompt: "/code-review:code-review max --comment ${{ github.repository }}/pull/${{ github.event.pull_request.number || github.event.issue.number }}"
+          # allowedTools only pre-approves; it denies nothing. Only the deny
+          # list stops the review executing what it just checked out.
           claude_args: |
             --model claude-opus-5
             --effort xhigh
             --max-turns 100
-            --allowedTools "mcp__github_inline_comment__create_inline_comment"
-            --append-system-prompt "Before reviewing, read REVIEW.md at the repository root and follow it: it defines what counts as a blocking finding in this repository, what not to report, and the repo-specific checks. Two overrides apply here. First, the skip gate for already-reviewed PRs: an existing Claude review comment justifies skipping ONLY when its 'Reviewed head:' SHA equals the PR's current head SHA; when the head has moved on, or this run was triggered by an explicit '@claude review' comment, run the full review, focusing on the commits since the previously reviewed head. Second, this is a headless run that terminates the moment you end your turn: launch every subagent with run_in_background set to false and wait for its result inside the same turn - never end your turn while a subagent is still running, and never end it before the review comment is posted. A run that ends without posting the review has failed."
+            --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh api:*),Bash(gh pr diff:*),Bash(grep:*),Bash(rg:*),Bash(ls:*),Bash(find:*),Bash(sed:*),Bash(git log:*),Bash(git show:*),Bash(git diff:*),Bash(go doc:*),Bash(go env:*),Read,Glob,Grep,WebFetch,WebSearch"
+            --disallowedTools "Bash(go build:*),Bash(go run:*),Bash(go test:*),Bash(go generate:*),Bash(go install:*),Bash(make:*),Bash(npm:*),Bash(npx:*),Bash(pnpm:*),Bash(yarn:*),Bash(node:*),Bash(bash:*),Bash(sh:*),Bash(docker:*),Bash(chmod:*),Edit,Write,NotebookEdit"
+            --append-system-prompt "Before reviewing, read REVIEW.md at the repository root and follow it: it defines the severity marker every finding carries, what counts as Important in this repository, what not to report, and the repo-specific checks. Five overrides apply here. First, the skip gate for already-reviewed PRs: an existing Claude review comment justifies skipping ONLY when its 'Reviewed head:' SHA equals the PR's current head SHA; when the head has moved on, or this run was triggered by an explicit '@claude review' comment, run the full review, focusing on the commits since the previously reviewed head. Second, this is a headless run that terminates the moment you end your turn: launch every subagent with run_in_background set to false and wait for its result inside the same turn - never end your turn while a subagent is still running, and never end it before the review comment is posted. A run that ends without posting the review has failed. Third, the comment you post is the only part of this run anyone can see: it must open with the tally and end with the coverage list REVIEW.md asks for, whether or not you found anything. Fourth, the default working tree is the BASE branch, and a read-only checkout of the pull request head sits beside it in pr-head/: read and grep the changed files under pr-head/, and treat anything read outside it as the pre-merge baseline rather than as the code under review. Never build, install or execute anything from pr-head/ - this job holds a write-scoped token, so running pull-request code with it is the workflow vulnerability REVIEW.md itself calls blocking. Fifth, you cannot build or test here, but CI already did: read the head commit's checks with 'gh api repos/OWNER/REPO/commits/HEAD_SHA/check-runs' and report what they actually concluded instead of writing that verification was unavailable. A required check that failed, or that never ran on this head, is itself a finding."
       - name: Upload the run transcript
         if: always()
         env:
@@ -656,6 +671,10 @@ jobs:
   resolve-conflicts:
     if: github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, 'resolve pr conflicts') && github.event.comment.user.login == github.repository_owner && github.event.comment.author_association == 'OWNER'
     runs-on: ubuntu-latest
+    # claude-code-action replaces these with the base branch's copies before it
+    # runs, so a change to them is the action's doing, never the agent's.
+    env:
+      RESTORED_PATHS: ".claude .claude-pr .mcp.json .claude.json .gitmodules .ripgreprc CLAUDE.md CLAUDE.local.md .husky"
     concurrency:
       group: claude-conflicts-${{ github.event.issue.number }}
       cancel-in-progress: false
@@ -746,6 +765,18 @@ jobs:
             hand_back "The merge of \`${base}\` conflicts over paths this job refuses to hand to its tooling:
           $(printf '%s\n' "$odd" | sed 's/^/- /')
 
+          Nothing was changed. Resolve those by hand."
+          fi
+          clobbered=$(printf '%s\n' "$files" | while IFS= read -r f; do
+            for p in $RESTORED_PATHS; do
+              case "$f" in "$p" | "$p"/*) printf '%s\n' "$f" ;; esac
+            done
+          done)
+          if [ -n "$clobbered" ]; then
+            git merge --abort 2>/dev/null || true
+            hand_back "The merge of \`${base}\` conflicts over paths the bot's own tooling replaces with the \`${base}\` copy before it runs, so a resolution there cannot survive:
+          $(printf '%s\n' "$clobbered" | sed 's/^/- /')
+
           Nothing was changed. Resolve those by hand."
           fi
           rules=""
@@ -856,7 +887,12 @@ jobs:
           stray=""
           while IFS= read -r f; do
             [ -z "$f" ] && continue
-            if ! grep -qxF "$f" <<< "$FILES"; then
+            grep -qxF "$f" <<< "$FILES" && continue
+            restored=false
+            for p in $RESTORED_PATHS; do
+              case "$f" in "$p" | "$p"/*) restored=true ;; esac
+            done
+            if [ "$restored" = false ]; then
               stray="${stray} ${f}"
             fi
           done <<< "$(git diff --name-only)"

CLAUDE.md

@@ -59,8 +59,7 @@ file locations when it can answer in one hop.
 - `tools/openapigen/` — Go generator that emits frontend types + Zod/JSON schemas
   into `frontend/src/generated/` from Go structs. The OpenAPI doc itself
   (`frontend/public/openapi.json`) is assembled from those + `endpoints.ts` by
-  `frontend/scripts/build-openapi.mjs`. (`tools/seedperf/` is a separate seeding
-  /load helper.)
+  `frontend/scripts/build-openapi.mjs`.
 - `docs/` — separate Next.js/Fumadocs site (pnpm, own CI in `docs-ci.yml`,
   outside `make verify`). Holds a THIRD independent implementation of
   link/subscription generation in `docs/lib/xray/` — check it whenever

REVIEW.md

@@ -5,9 +5,32 @@ and subscriptions for other programs — Xray-core, mihomo, sing-box, mtg-multi
 and is deployed by operators who upgrade in place. Judge findings by what
 breaks for those consumers and operators, not by style.
 
-## What a blocking finding means here
+## Severity
 
-Reserve blocking severity for:
+Mark every finding with exactly one of these, at the start of the finding:
+
+| Marker | Severity | Use it for |
+| --- | --- | --- |
+| 🔴 | Important | A defect this pull request introduces or makes worse, in one of the classes under "What Important means here". Worth fixing before it merges. |
+| 🟡 | Nit | Style, naming, refactoring, and an ordinary `CLAUDE.md` violation the change introduces — a source comment block over two lines, a fix larger than the bug it removes, a test `CLAUDE.md` rejects outright. |
+| 🟣 | Pre-existing | A real bug you hit while reading that this pull request neither introduced nor made worse. |
+
+Not every `CLAUDE.md` rule is a nit. The three listed below — the dispatch
+rule, the migration rule, the endpoint chain — are Important, because each one
+passes every local test and breaks a real deployment.
+
+Severity follows what this pull request did, not how alarming the defect looks
+on its own. One the change worsens is 🔴 for the regression it added, not for
+the whole defect; one it merely brought into view is 🟣.
+
+Checking what this panel emits means reading far more code than the diff
+changes, so pre-existing bugs surface on every review. One already on the base
+branch stays 🟣 however bad it is: this pull request did not cause it, so it
+cannot be a reason to hold this pull request. Say in one clause that it
+predates the change. The exception is a live security hole on an exposed
+surface — still 🟣, but open the summary with it.
+
+## What Important means here
 
 - Security on the exposed surfaces: `internal/web/controller/`, session and
   middleware code, the PUBLIC `internal/sub/` subscription server, and Xray
@@ -28,9 +51,7 @@ Reserve blocking severity for:
 - Any edit to `.github/workflows/`: this repository runs workflows with
   secrets against a public fork stream. Untrusted expression interpolation
   into `run:` blocks, broadened permissions, weakened guards, or a job that
-  executes pull-request code is blocking.
-
-Style, naming and refactoring suggestions are nits at most.
+  executes pull-request code.
 
 ## Always check
 
@@ -40,23 +61,43 @@ Style, naming and refactoring suggestions are nits at most.
   in `tools/openapigen/main.go`, and `frontend/public/openapi.json` copied to
   `docs/public/openapi.json` with the docs MDX regenerated
   (`cd docs && pnpm gen:api`). CI checks the first three; the docs copy is
-  checked by nothing — a missed copy is blocking, not a nit.
-- A new i18n key exists in ALL 13 locale files in `internal/web/translation/`
-  and is referenced from `frontend/src` or Go in the same PR.
-- A bug fix carries a test that would fail without the fix. A test that
-  passes either way, asserts only `err != nil` or `len(x) > 0`, or was made
-  green by regenerating golden fixtures or Vitest snapshots is a real finding.
+  checked by nothing — a missed copy is Important, not a nit.
+- A bug fix carries a test that would fail without the fix. A test that cannot
+  tell the broken behaviour from the fixed one passes before and after, so it
+  certifies nothing and is itself the finding — asserting only `err != nil` or
+  `len(x) > 0`, or going green by regenerating golden fixtures or Vitest
+  snapshots.
+- No second way to do a thing already decided: Go tests are stdlib `testing`
+  (never testify), the panel is Ant Design (never Tailwind or shadcn). Neither
+  golangci-lint nor oxlint forbids the import, so it passes CI clean.
 
 ## Do not report
 
 - Anything CI already enforces: golangci-lint and gofumpt, oxlint, format
-  and typecheck, `npm audit`, govulncheck.
+  and typecheck, govulncheck, and `npm audit --omit=dev --audit-level=high`.
+  A dev-dependency advisory is out of scope on purpose: it ships to nobody.
 - The contents of generated files (`frontend/src/generated/`,
   `frontend/public/openapi.json`, `docs/public/openapi.json`) or lock files.
   Those files being STALE after a source change is reportable; their style
   is not.
 - Missing tests for getters, constants, renames or pure map lookups —
   `CLAUDE.md` rejects such tests outright.
+- A missing or unreferenced i18n key.
+  `frontend/src/test/i18n-dead-keys.test.ts` pins the 13 locale files in
+  `internal/web/translation/` in both directions, so the `frontend` job is
+  already red. Report the failing check, not the key.
+
+## A higher bar, not silence
+
+Everything named under "What Important means here" gets full scrutiny. Two
+areas do not — they earn review, but report there only what you are
+near-certain about and that actually breaks something:
+
+- `docs/` — the standalone Fumadocs site, with its own CI and its own
+  dependency tree. `docs/lib/xray/` is the exception and gets full scrutiny:
+  it is the third link implementation.
+- `internal/web/translation/` — the key set is CI's job and the wording of a
+  translation is nobody's here.
 
 ## Verification bar
 
@@ -67,9 +108,40 @@ Style, naming and refactoring suggestions are nits at most.
   or hash choice — must name the upstream symbol that decides it (repository,
   file, identifier). If you cannot verify it, keep the finding but say
   explicitly that it is unverified instead of asserting it.
+- "CI passed" is a claim too, and needs the same evidence: say it only of a
+  run you actually read. A green one proves less here than it looks — only
+  `postgres-durable-first` runs against PostgreSQL, `go-test` and `race` are
+  SQLite, and `XRAY_E2E_BINARY` and `XUI_SCALE_TEST` are set by no job, so
+  those tests have never run in CI at all. Where a change touches dialect,
+  migration or Xray gRPC code that no job exercised, say it is unverified
+  rather than repeating a green tick as proof.
 
-## Cap the nits
+## Cap the volume
 
-Report at most five nits per review and say "plus N similar" in the summary
-for the rest. Lead the summary with "No blocking issues" when everything found
-is a nit. After the first review of a PR, report blocking findings only.
+🔴 findings are never capped. Report every one.
+
+Report at most five 🟡 nits and at most three 🟣 pre-existing bugs. Past that,
+say "plus N similar" in the summary instead of posting them.
+
+A cap decides WHICH ones survive, so choose rather than truncate: the same nit
+repeated across files is ONE finding with a count, not five slots; a nit in
+code this pull request wrote outranks one in code it only moved; and a nit
+nobody would act on does not deserve a slot at all.
+
+After the first review of a pull request, report 🔴 findings only: a one-line
+fix must not reach round seven on style.
+
+## What the comment must show
+
+Open with a one-line tally — `2 🔴 / 4 🟡 / 1 🟣` — so the author sees the
+shape of the review before the detail. When nothing is 🔴, lead with
+`No blocking issues` and put the tally after it.
+
+The posted comment is the only part of a review anyone sees, so a bare "no
+issues found" is a receipt, not a review: nothing in it says whether the diff
+was read or the run died early. Every comment therefore ends with a short
+coverage list — one line per area actually checked, naming what was examined
+and what it turned out to be, plus the head SHA and the size of the diff it
+covers. Say which claims could not be verified and why, including a check
+this environment blocked. Keep that coverage list under ten lines; it is
+evidence, not a retelling of the pull request.

bot_context_test.go

@@ -153,3 +153,46 @@ func TestBotContextSkipGatesExist(t *testing.T) {
 		})
 	}
 }
+
+// REVIEW.md tells the reviewer which CI job proves what, and which skip gates
+// mean a green run proved nothing. Both go stale silently on a rename.
+func TestReviewNamesRealCIJobsAndGates(t *testing.T) {
+	doc := readRepoFile(t, reviewPath)
+	ci := readRepoFile(t, ciWorkflowPath)
+	// Hyphenated only: a single-word job name is indistinguishable from prose.
+	jobs := regexp.MustCompile("`([a-z0-9]+(?:-[a-z0-9]+)+)`").FindAllStringSubmatch(doc, -1)
+	if len(jobs) < 2 {
+		t.Fatalf("expected %s to name at least 2 CI jobs in backticks, found %d", reviewPath, len(jobs))
+	}
+	for _, j := range jobs {
+		t.Run(j[1], func(t *testing.T) {
+			if !strings.Contains(ci, "\n  "+j[1]+":\n") {
+				t.Errorf("%s names a CI job %q that %s does not define", reviewPath, j[1], ciWorkflowPath)
+			}
+		})
+	}
+	for _, g := range regexp.MustCompile("`((?:XUI|XRAY)_[A-Z0-9_]+)`").FindAllStringSubmatch(doc, -1) {
+		t.Run(g[1], func(t *testing.T) {
+			if strings.Contains(ci, g[1]) {
+				t.Errorf("%s claims %s is never set in CI, but %s sets it", reviewPath, g[1], ciWorkflowPath)
+			}
+		})
+	}
+}
+
+// The i18n rule is the one REVIEW.md states as a number, so it is the one that
+// goes wrong silently when a locale is added.
+func TestReviewLocaleFileCount(t *testing.T) {
+	doc := readRepoFile(t, reviewPath)
+	m := regexp.MustCompile(`(\d+) locale files`).FindStringSubmatch(doc)
+	if m == nil {
+		t.Fatalf("%s no longer states the i18n rule as \"N locale files\"", reviewPath)
+	}
+	files, err := filepath.Glob("internal/web/translation/*.json")
+	if err != nil {
+		t.Fatalf("glob locales: %v", err)
+	}
+	if got := len(files); m[1] != itoa(got) {
+		t.Errorf("%s tells the reviewer to expect %s locale files, internal/web/translation/ holds %d", reviewPath, m[1], got)
+	}
+}