pr-review

PR Review — AI Agent Review for one pull request

Repo profile — read .claude/repo-profile.md first. This skill is repo-agnostic; arc is the reference implementation. Use the profile's values wherever this doc shows an arc default: repo_slug (the gh -R target), gate_mode (arc = scripts: no CI on PRs, so gh pr checks is empty and the verification scripts are the only gate; ci/both repos ALSO fold in gh pr checks), verification_entry / pre_merge_entry. Arc's own provenance for the lessons below is not inlined here (fuller case narratives, where they exist, are under .claude/case-law/).

Takea PR all the way to done: read the PR diff + linked issue + existing review/comments → verify item by item against the landed code/tests → run the verification gate (pre-merge) and diagnose the root cause(there is no CI on the PR anymore) → detect conflicts/duplication with sibling PRs → produce aevidence-backedmerge-readiness verdict, posted back on the PR (comment), rather than buried in some conversation.

This is the issue-review PR version: the same engine (read → check against reality → post evidence-backed comment), with the target switched to PRs, plus three things issue-review doesn't have—running the verification gate and diagnosing it, cross-PR conflict detection, merge-readiness verdict。

Output language and writing conventions = profile comment_language。 verdict comment, "needs human confirmation blocks", and all team-facing output must be written in this language (the team's reading language); code identifiers, paths, commands,path:line、gh output, test outputstay as-is(not translated).No piling on: too much content is itself a reading burden—lead with a one-sentence conclusion, then the minimum but sufficient evidence (docs / code path:line / real test output,screenshots required for UI-related items—screenshots/screen recordings produced by ui-verify are this kind of evidence); collapse long logs into <details>, and the verdict only cites the conclusion without repeating the full logs.

Usage

/agentloop:pr-review <pr-number-or-url> [--post]

• <pr-number-or-url> — the PR to review.gh use it directly when the CLI is available; when there is no gh(cloud routine), use the mcp__github__* tool instead (loaded via ToolSearch).
• --post — post the verdict comment to the PR (default read-only: only produce output for the user, do not post comments, do not touch labels,never merge)

For a single PR use this skill;batch + dedupe-close + gated auto-mergeuse pr-sweep。

When to Use

• Want to get an independent judgment, checked against real code, ona PR before merging (not just looking at whether the gate is red or green).
• Suspect that a PR duplicates/conflicts with/contradictsanother PR, and need to assign responsibility for "which to keep, which to close".
• Building "a machine that automatically reviews+merges PRs on a schedule", and need a reusable per-PR action with traceable artifacts.

Not applicable: review of a purely local diff (use /code-review); design review in an issue (use /agentloop:issue-review)。

Verdict vocabulary (controlled, 5 categories)

recommendation	Meaning	Next step
MERGE	Claims verified, no real blockers in verification, no unresolved conflicts	Can merge (merged automatically by pr-sweep per the risk gate, or merged by a human)
COMMENT	In principle mergeable but has concerns worth raising (partial fix, missing tests, minor issues),oris thekeeperin a duplicate pair (note the peer to be closed)	Post a comment; usually can still merge
SUPERSEDE	In a duplicate/contradictory pair, theredundant/inferior side	Post an explanatory comment + point to the keeper → close this PR(closing is pr-sweep's action)
BLOCK	Has a real defect / verification failure is this PR's fault / unresolved conflict	Post a comment pointing out,cannot merge
CLOSE	Outdated/superseded by a merged PR/no longer needed	Post a comment explaining → close

verdict requiring human intervention (BLOCK escalate、COMMENT with blocking concerns, and on the pr-sweep side 🔴/security/awaiting-direction|judgment|caution)→ the comment must include a "needs human confirmation block" (awaiting-glance not included—it only asks a human to take a look, with no question requiring human judgment) (Step 5.5): what you need them to decide + how to verify (runnable steps, security listed item by item) + recommendation. Don't stop at "please confirm manually."

★ Fix on discovery (fix-now)—the default action of review output is to fix, not to relay

Discovered during reviewdeterministic defects(especially UI defects visible at a glance in screenshots: overlap/bare styles/broken interaction/Unknown boxes), the default action isfix it on the spot this round, not leave a comment for the next round. The criterion is "does it need a human to make the call," not "is it this PR's fault":

Where the defect is	Four gates (fix only if all pass)	On-the-spot action
Introduced by this PR's diff	Evidence is solid (screenshot/test/path:line) · the fix is unambiguous · non-security · no need for a directional call	--post mode: directly on the PR branchfix (fix commit + push + comment explaining what was changed); read-only mode: the comment gives a fix that can be applied directly.BLOCK is left only for what can't be fixed/needs direction
Already existing on main(encountered incidentally during review, e.g. a layout bug exposed in a screenshot)	Same four gates as above + bounded (single-point CSS/renderer level, not architectural)	Open a tracking issue (with screenshot) + branch from origin/<default_branch> to fix + before/after screenshots + verification + open a separate fix PR link both ways; point to it in one sentence in the verdict comment. Close the loop in one round, don't write "recommend re-checking" or "leave to another agent"
If any gate fails(security / direction A-B undecided / large change / semantic dispute)	—	comment + needs-human-confirm, write out "what the human needs to decide + how to verify + recommendation" in full (Step 5.5)

Anti-pattern: A screenshot reveals a deterministic, CSS-level, non-security defect → only write "an observation unrelated to this PR, recommend re-checking (does not affect merging)" → nobody follows up, the bug keeps lying there. Correct action: handle it on the spot per the second row of the table above (fix + issue + fix PR); what the human sees is "Already fixed, PR #N" rather than a relayed sentence.A discovered defect that "could have been fixed in passing but wasn't" = this review is incomplete.

Concurrency discipline for fix-now (the same bug spawning duplicate issues): Multiple actors may simultaneously target the same bug—"I searched when I started and there was no issue" ≠ "there still isn't one when I submit" (during the investigation window someone else may have created one, TOCTOU). Three rules: ① Search once more right before opening a tracking issue(title keywords + --state all sorted by created descending to see recent ones), if an issue for the same bug already exists → don't open a new one, put the root cause/evidence comment into it; if you accidentally opened a duplicate → close it yourself and comment noting which one it was merged into. ② The fix branchmustuse the deterministic name claude/issue-<N>(the repo-wide hard dedup key, same as issue-sweep/pr-sweep), before pushing git ls-remote origin refs/heads/claude/issue-<N> Do the claim check — if someone has already pushed → read their progress before deciding whether to join or yield. ③ The target issue has a fresh agent:processing lock butno branch and no PR(the other party is still investigating), while you already have a verified fix → push directly claude/issue-<N> to claim (first to push wins; the other party's claim check will short-circuit) and explain in an issue comment; if you only have an idea and no fix, don't grab the lock, just leave evidence in a comment.

How It Works (cold start)

┌────────────────────────────────────────────────────────────┐
│ 0. 读 PR 全貌 + 关联 issue + 已有 review/comments            │
│ 0.6 ★ 复审去重(sha 机器键):fresh→跳过;stale→增量,不从零   │
│ 1. 读 diff + 受影响文件在「当前 main」里的真实样子            │
│ 2. ★ 逐条核验声明 vs 已落地代码/测试(path:line 或 NOT FOUND)│
│ 2.5 ★ 横切:反向引用/parity/端到端/性能/测试/清理           │
│ 3. ★ 运行 verification (pre-merge) —— PR 上已无 CI,这是唯一 │
│    门控信号;每次必跑(read-only 安全),--comment 落 PR      │
│    简单失败可自修;复杂失败 → BLOCK + 完整日志 context       │
│ 4. ★ 检测与兄弟 PR 的冲突/重复/矛盾(同 issue + 同文件)       │
│ 5. 出判定:5 类之一 + 证据;MERGE 必须以 Step3 通过为前提     │
│ 6. (--post)落 verdict comment;never merge、never close    │
└────────────────────────────────────────────────────────────┘

Step 0 — Read the full PR picture (cheap, do it first)

gh pr view <n> --json title,body,author,headRefName,baseRefName,mergeable,additions,deletions,files,labels
gh pr view <n> --comments          # 会话区意见(AI 评论以 "> 🤖 AI Agent" 开头)
# ⚠️ 人类意见共有三个面,--comments 只显示第一个面,后两个必须补读
# (遗漏 = 人类的 inline 修改要求被忽略):
gh api repos/{owner}/{repo}/pulls/<n>/comments --paginate \
  --jq '.[]|{user:.user.login, t:.created_at, path, line, body}'   # ② 代码行 inline review comment(带 path+行号,直接喂核验)
gh api repos/{owner}/{repo}/pulls/<n>/reviews --paginate \
  --jq '.[]|select(.body!="")|{user:.user.login, state, t:.submitted_at, body}'  # ③ review 总评(approve/request-changes 正文)

★ The three-face check is tool-agnostic —gh api is only one implementation; when switching to other tools such as MCP, the following three calls must all have corresponding output this round (even if "no result"); missing any one = Step 0 not complete:

face	gh	MCP equivalent (gh when unavailable)
①session comments	gh pr view <n> --comments	pull_request_read(method="get_comments")
②inline code line comments	gh api pulls/<n>/comments	pull_request_read(method="get_review_comments")
③review summary (including state)	gh api pulls/<n>/reviews	pull_request_read(method="get_reviews")

③'s state field (APPROVED/CHANGES_REQUESTED/COMMENTED) must be explicitly recorded, not just the body text — a CHANGES_REQUESTED is a stronger signal than an ordinary comment, and there is another independent hard gate before merge (pr-sweep Step 5)。

The PR body usually contains Fixes #N / Part of #N → note down thelinked issue number(the primary key for conflict detection). Read the linked issue (gh issue view <N>) to get "what exactly this PR is trying to solve".

agent:hold(human hold = terminal freeze, not processing freeze): Step 0 has already retrieved labels; if the PR has agent:hold——explicit manual /agentloop:pr-review <n> only warn, don't block(a human named it precisely because they want to look; and this skill is read-only by default and never merges, so risk is low). hold means "don't merge/close before someone responds",not "don't process": review as usual,new human comments on a hold PR must be read and responded to(they are often change requests or decision conditions). Reflection in the verdict: even if all green, also write MERGE (held) and note "wait for someone to remove agent:hold before merging"; if a human comment gives an explicit change request → handle per COMMENT/BLOCK and make "respond to human feedback" the next step (--post mode can directly implement the human's explicit requested changes on the PR branch). What actually enforces the merge gate (never merge with a hold) is pr-sweep. Only humans add and only humans remove; the agent never removes automatically.

Step 0.5 — PR branch sync <default_branch>(cannot be skipped)

Must be done yourself when running pr-review standalone; when called by pr-sweep it is already handled after its own Step 0. <default_branch> = profile field (some repos use master— below and throughout, "main" refers to this field, not annotated again each time).

Step 0 has already obtained the mergeable field. If it is CONFLICTING, or the PR branch is known to be behind <default_branch>, rebase before continuing — otherwise pre-merge will produce false failures due to a stale base (<default_branch> errors already fixed will be misjudged as introduced by this PR):

git fetch origin <default_branch>
gh pr update-branch <n> --rebase   # 把 origin/<default_branch> 带进 PR 分支并 push；已是最新则无操作

After a successful rebase, retrieve mergeableagain (it should be MERGEABLE), then proceed to the following steps.

Step 0.6 — ★ Re-review deduplication (cross-runner: check existing verdicts first, don't duplicate work)

Real incident (PR #1812): the agents of three runners full-reviewed the same PR one after another; each round posted a new comment, and none of the 6 comments contained a valid verdict for the current HEAD—people had to piece the state together from the pile of comments themselves, and as a result no one could make a decision. Two root causes: ① freshness relied on timestamp comparison, which cannot be machine-deduplicated across runners; ② conclusions were persisted in anappend-onlymanner—old verdicts were not refreshed, and subsequent rounds only posted incremental FYIs. Fix: make verdicts use the HEAD sha as a machine key(sweep-trace's sha field), carried bya per-PR unique canonical comment(Step 6)。

Step 0 has already fetched all comments; take the HEAD oid and compare it with the most recent verdict trace (one posted by any runner counts):

head=$(gh pr view <n> --json headRefOid --jq .headRefOid)
# 从已取回的会话 comments 里抓最近一条 gate:"verdict" 的 sweep-trace,读其 sha + val 字段
# (旧格式 trace 无 sha → 一律视为 stale)

★ The freshness key is the pair (sha, val), not the sha alone. sha answers "did the input change",val(the previous round's verdict conclusion) answers "will the conclusion change". The latter is what really determines whether one should speak again: for a PR that has already been escalated to a human and whose conclusion is constant, whether the input changes every day or not at all,one should not post a second identical message. Looking only at the sha degenerates into a spam machine on the following two kinds of PRs (real data: an automatic release PR accumulated 29 comments, of which 22 were verbatim identical in conclusion MERGE (held), across 26 skills versions—showing this is not a bug in any one version, but the wrong dimension chosen for the key).

Determination	Action
fresh(sha == HEAD, and no new human comments after it)	Do not repeat the review. Act only when it can "advance" things: rerun gates that the previous round's environment could not run (daemon-type ui-verify / e2e-gate), merge new evidence into the canonical verdict (Step 6 upsert). If nothing can be advanced → report "HEAD <sha7> already has a fresh verdict (runner:<x>), skip" andend; zero output is correct output。
fresh + new human comments	Enter "respond to human feedback": read the human opinions on the three surfaces (Step 0), handle them in a targeted way, andrefresh the same canonical verdict(do not open a new one).
★ sha changed, but it is a mechanical regeneration(see "mechanically regenerated PR" below)	Treat as fresh. sha churn is not new work. Only upsert the canonical verdict's sha field (so the next round can recognize it),do not rerun verification, do not change the conclusion, and absolutely do not post a new comment. Report "mechanical regeneration, conclusion unchanged, sha silently refreshed".
★ stale, but after incremental re-review the conclusion is the same as the previous round's val , and the PR is already in an escalated, waiting-for-human state(disposition is awaiting-* / agent:hold)	Silent: upsert the canonical verdict's sha (+ one or two necessary sentences saying "the commits added this round do not change the conclusion"),do not restate the reasons, do not post a new comment, and do not @ anyone again. Before the human replies, repeated escalation is just noise—escalating once is enough。
stale(sha != HEAD or no sha)	Incremental re-review, not redoing from scratch:git diff <旧sha>..<HEAD> Only verify the delta (what the new commits changed, which of the old conclusions are invalidated); still-valid verification results are inherited directly (noting "inherited from <sha7> rounds]),upsert the same canonical verdict and refresh the sha to the current HEAD.Never open a new FYI comment that only writes a delta—that is exactly the stacked form of #1812.
no verdict	first round, run the full flow.

Mechanically-regenerated PR: a PR whose branch is rewritten by a tool according to a fixed template and whose contents roll automatically with upstream —typically an automated release PR such as release-please. Criteria (any one is sufficient,judge by signals, don't guess from the PR title):

• the branch name carries the fixed prefix of an automated release tool (such as release-please--*);
• carries an automated release label (such as autorelease: pending);
• or git diff <旧sha>..<HEAD> onlytouches mechanical artifacts such as version numbers / CHANGELOG / lockfiles, with no source code changes.

For these PRs the sha changes every time upstream merges, butthe reviewable content shape has not changed. For them: produce a verdict normally on the first round; afterward, unless "the diff shape changed" (the set of touched files goes beyond mechanical artifacts) ora human speaks, always follow the "treated as fresh" row in the table above.

sha only exemptsconclusive labor, not theobligation to respond—new human comments must always be read and answered (same as the Step 0 hold section). read-only mode (no --post) has the same decision logic, except the artifact is presented to the user rather than upserted to the PR.

val must faithfully reflect this round's conclusion, as it is the machine key for the table above. See the end of the document for the gate vocabulary—the verdict comment's trace mustbe gate:"verdict"; marking it as some other gate (in practice, mis-marking as needsReview) will cause the next round's verdict lookup to miss this one, deduplication willsilently fail, and it will fall back to spamming.

Step 1 — Read the diff + the current state of the affected code

gh pr diff <n>

The local checkout must be atlatest <default_branch>(pr-sweep has already synced; when running alone, first git fetch origin <default_branch>). Read the files touched by the diff as they actually are in the current tree—do not judge only for self-consistency within the diff; check whether it is correct when it lands in the real code.

When you need to check out the PR branch itself for deeper investigation (running a package's tests, exploring related files outside the diff), you must never directly switch branches / modify the shared main checkout above—it is pinned at <default_branch>, and when pr-sweep runs multiple PRs in a batch it will be reused by other PR reviews.You must create a separate worktree, and it must be created under $AGENTLOOP_WORKTREE_BASE , hardcoding /tmp/...is forbidden (the fleet driver has already injected this variable into the worker environment; it is the fixed directory dedicated to agentloop):

git worktree add --detach "$AGENTLOOP_WORKTREE_BASE/pr-<n>.$$" "origin/pr/<n>/head" 2>/dev/null \
  || git worktree add --detach "$AGENTLOOP_WORKTREE_BASE/pr-<n>.$$" "$(gh pr view <n> --json headRefName -q .headRefName)"
# 本 PR review 结束时(无论 verdict 是什么)务必清理:
git worktree remove --force "$AGENTLOOP_WORKTREE_BASE/pr-<n>.$$" 2>/dev/null || true

Hardcoding /tmp/... bypasses the deployer's checkoutBase configuration, accumulating on the system disk and never being cleaned up after use. The driver will, each round, sweep away leftovers under $AGENTLOOP_WORKTREE_BASE that are older than 15 minutes and have no active process, but that is a safety net, not a replacement—clean up what you create yourself.Most PRs can be verified with gh pr diff + reading the current state of the code on the current default_branch; only when you really need to run the PR branch's own code (tests/builds) is this worktree worth creating.

Step 2 — ★ Verify each claim one by one against the landed code/tests (most critical)

Separate "reads correctly" from "is actually correct". Set verification depth by PR type:

PR type	How to verify
bug fix / behavior change	Does this diff actually fix the problem described in the title? Is the logic correct? Is therecorresponding test(newly added or existing coverage)?grep/read the tests, and if you can run them, run that one:<package_manager> --filter <pkg> test / <test_runner> <path>, note downexact command + pass/fail。
test-only	Is the test meaningful (real assertions, not an empty run)? Does it fit runProviderTests / the existing paradigm?actually run it onceto confirm green.
docs-drift	every documentation changedoes it match shipped reality? Go through each one, grep back it with code (path:line), or is it just "sounds reasonable" prose? (issue-review discipline:status frontmatter is generally untrustworthy, must be checked against code.)
release(release-please automated PR)	Mechanical part: confirm it is the current release branch, version numbers are consecutive, CHANGELOG was generated by the tool.There can be only one of the same kind(see conflict detection).
feature	Scope, correctness, test coverage, whether it touches shared config (lockfile / wrangler.toml / package.json) introducing a conflict surface. Leave strong semantic surfaces to a strong model.

Each claim → locate path:line or mark NOT FOUND. Without evidence, do not write.

Step 2.5 — ★ Cross-cutting impact verification (must check beyond the diff, regardless of PR type)

Step 2 verifies "what this PR claimsto change"; this step verifies its impact onother parts of the system—something that self-consistency within the diff cannot reveal, so you must step outside the diff and actively search for it.This is the layer agent review most often misses: staring only at the change itself, never asking "who depends on it, does anything else need to be changed in sync, is it actually wired into a usage scenario, will it get slower, was it tested, was the old thing cleaned up". Go through each dimension, and for each dimensioneither provide evidence or explicitly judge it "not applicable"—failing to check and checking then judging "not applicable" are two different things:

Dimension	When to check	How to check	Handling on hit
Reverse references	diff deletes a file / deletes or renames an export / changes a public signature	Run impact-check: search the whole repo in reverse by basename (including multiline imports,readSource/dynamic references), don't just search single-line import	dangling reference = deterministic break check-types/test = real defect in this PR → assign to Step 3 root cause (a), can BLOCK, list each occurrence path:line。run before Step 3, can predict where verification will go red, and pin the root cause to this PR rather than a stale base
Cross-package parity (mirror + companion)	①modify runtime layer (node↔cloudflare)/dual-end shared provider;②give enum, union type, registry, factory add a new item(new widget/component/message type/provider kind, etc.)	①check against the other runtime with the same semantics, are both sides changed;②grep for the switch/dispatch/renderer consuming that enum (often inanother package)Is the corresponding case/handler	Missing counterpart → BLOCK(deterministic bug) or COMMENT(one-sided intentional).②Most insidious: when the consumer side has a default/fallback branch (rendering "Unknown"/silent no-op),check-types/build is all green, only degrades at runtime—the opposite of "reverse references" (deleting something breaks the build loudly, adding something doesn't break it but silently breaks)
End-to-end delivery (use case closed loop)	diff adds new export / function / capability / provider / API / abstraction layer	①grep all repo call sites of that symbol (comes out in the same impact-check pass); ②one level up: does this new capability havea connection into a real user-visible flow / actual use case, or is it only half-landed infra	Zero callers and not an external API/SDK export → COMMENT;Has callers but not connected end-to-end into a use case(built the capability but not the feature that uses it, only wired up half) → COMMENT, require the author to explain where the use case closed loop is / which follow-up PR completes it.Foundational capability building must not be divorced from actual use cases—a foundation with no real consumption scenario should not land independently
Performance regression	Change torequest handling chainany link (routing / middleware / auth / provider mount / serialization), cold start / init path, DB query, in-loop I/O, cache	Read the changes: ①whether it introduces N+1, full table scans, lost index hits or cache, synchronous blocking, large payloads; ②whether it pushes scale-work (fleet / tenant-level / O(N)) into per-request / cold start / init paths—there it must be O(1) or moved to the background	Suspicious → COMMENT, require the author to provide before/after measured data (performance changes must have data; "should be faster" is not accepted);per-request·per-tenant overhead on the request chain / init must be measured or it doesn't pass
Test coverage	Any behavior change / new feature / bug fix	Whether there are corresponding newly added or existing automated tests covering it, and they truly assert, not just run empty	Core logic has no tests → BLOCK; edges have no tests → COMMENT(missing tests)
Cleanup / wrap-up	PR replaces / migrates / renames some implementation, adds an alternative path or temporary flag	Does the diffdelete the superseded old code / old code path / dead feature flag / outdated tests / commented-out code; does a newly added file correspond to an old file that should be deleted; did the migration leave two implementations	Only adding new without deleting old (dead code accumulation / dual implementations coexisting) → COMMENT, list what should be cleaned up path:line;deprecated old paths still have live callers(leaving them will cause misuse) → BLOCK

Same-name redefinition ≠ reverse-reference miss: a symbol hit by grep because a new/ported module redefines it (common in rename/rewrite) is noise; only dangling references still pointing to a deleted/changed definition count (criteria see impact-check)。

Step 3 — ★ Run the verification gate (pre-merge) and diagnose the root cause

Gate form = profile gate_mode(arc = scripts: deleted ci.yml/pr-title.yml,gh pr checks is always empty, the verification script isthe only gating signal;ci/both repos that also need to factor in gh pr checks into the determination). Before issuing a determination,run every time <pre_merge_entry>(profile field; read-only, does not write to remote, does not conflict with the default read-only; --comment will post the report to the PR):

# <pre_merge_entry> from repo-profile.md
<pre_merge_entry>                       # read-only:报告呈现给用户
<pre_merge_entry> --comment <n>         # --post 模式:一步跑 + 贴到 PR

pre-merge use the latest origin/<default_branch> as the affected base (after sibling PRs merge, the environment has moved forward, so it can catch breakage that pre-pr cannot see). renderReport has already generated markdown (status/duration table + rawTail + collapsed rawFull), reference it directly, do not hand-write the numbers.

The verification report can only be delivered via --comment , hand-writing is forbidden. Do not hand-copy verification results into a review verdict comment you wrote yourself (even if it carries the <!-- verification-report ... --> marker)—— postComment() 's upsert generation logic (sha/result encoding, PATCH-vs-POST determination) exists only inside the script; hand-writing the marker easily becomes inconsistent with the real sha (a hand-written marker with a short sha is judged a mismatch by the full comparison). Send the verdict comment and verification report separately: write the verdict as a standalone regular PR comment (gh pr comment/mcp__github__add_issue_comment), and always deliver the verification result separately with pre-merge.ts --comment <n> , do not merge them into one hand-written comment.

Never paste any upsert marker verbatim into the body of a verdict/discussion comment (<!-- verification-report ... / <!-- pr-review-verdict ... , etc.)——even if it is only quoting/explaining what it is.postOnce 's upsert targeting only recognizes the marker on the commentfirst non-empty line(tightened since #3576, narrows #1246); by this rule a purely discussion comment will not be mistakenly overwritten; but the safer approach isdo not paste it verbatim at all——if you need to mention it, escape it with a code block, remove the angle brackets, or add a space to break it up, so it does not appear in the comment body in the form of a valid marker.

Core principle: gating is a signal, not a judge. failure must have its root cause determined:

Root cause	Signal	Handling
(a) Real defect in this PR	The failing test/type/architecture check was introduced by the diff	→ BLOCK, the comment points out path:line + the failing check name + rawTail
(b) flaky / infrastructure	Occasional failure unrelated to the change (network/timeout/sandbox jitter)	→ do not block; the comment notes "failure is unrelated to this PR (flaky)", rerun to confirm
(c) base is stale and needs rebase	mergeable=CONFLICTING, or the failure is because main has moved forward (pre-merge base has been fetched)	→ Step 0.5 has already auto-rebased; if it was not executed (exceptional case),COMMENT requires a manual rebase
(d) self-fixable noise	format is warn-only (the script is already non-blocking); mechanical items like single-line lint/import	→ do not block;--post mode can self-fix on the PR branch → push → rerun

Results go into the verdict: all blocking ✅ PASS → allow MERGE; non-PASS → verdict must not be MERGE。

• Simple failures(format/import/single-line lint):--post mode fixes on the PR branch → rerun; in read-only mode point it out in the verdict.
• Complex failures(test/type/architecture violation): send BLOCK, verdict carries the failed check name + rawTail + collapsed rawFull, to hand off context to the next agent.

In pr-review this step is advisory (verdict input), not a non-skippable merge gate. The truly non-skippable hard gate——"<merge_gate_entry> <pr#> exit 0 (SHA match + result=PASS/NA; profile field, arc = <merge_gate_entry>)"——is in the pr-sweep merge gate(mechanism point: the SHA comparison is performed by that script, not left to good faith). pr-review only judges, it does not merge; the gating is enforced over there.

Exception: if there is already a verification report for the same HEAD with a timestamp after the last diff → reuse it, do not rerun.

Runtime / CF-parity PR (changes this repo's multi-runtime parity surface <Backend Face Paths>, or blocklet render/mount/serve semantics):/agentloop:verification 's static gate cannot see "whether it works when run"——run an extra /e2e-verify(this repo's companion, see repo-profile's Companion Skills; if none, stub or skip this step). It locally boots both runtimes to verify anonymous + authenticated roundtrip: Node = <dev_server_node>,CF = <dev_server_edge> local miniflare (comes with D1 + migration, no CF account needed, does not touch staging). So the CF sidecan be verified locally——never judge it as "needs a cloud CF/wrangler environment" and defer; don't treat "don't hammer online staging" as "CF can't be verified locally".<cli_binary> missing/stale → first run <cli_setup_command>(arc cli_binary = arc)。

Step 3.5 — UI changes: conditionally triggered ui-verify

verification(Step 3) is a daemon-less, browser-lessstaticgate——it cannot see "whether the page still renders, whether the user flow still goes through". So when the PR diff hits a UI surface, run ui-verify as a review step, and merge the UI report into the verdict evidence (do not put it into pre-pr.ts/pre-merge.ts the deterministic script ——that would break its hermetic/deterministic nature; the connection happens at this layer of orchestration).

# diff 命中 UI Face Paths(regex 见 repo-profile.md「UI Face Paths」;下方为 arc 的值)→ 触发
gh pr diff "$n" --name-only | grep -Eq '^(blocklets/|providers/runtime/ui/|providers/runtime/web-device/|providers/.+/aup/|packages/aup/|packages/core/)' \
  && echo "UI 面命中 → 跑 /ui-verify --pr $n"

• First check whether the proposing agent submitted images (screenshot shift-left contract, issue-sweep side): the PR body already embeds runtime screenshots → reuse as the evidence baseline, do not regenerate images; only spot-check and retake when "the diff changed again after the screenshot" or "the screenshot is suspicious (does not match the diff)".Screenshot evidence and the pre-merge report share the same SHA matching semantics: valid only for the HEAD at capture time——any subsequent push (including this review agent's own fix commit, even if it only changes one line of CSS) invalidates it; if the UI surface changed, retake(the counterexample of rerunning only static verification and merging, with nobody having looked at the new styles). The hard interception on the merge side is in pr-sweep the merge gate's "UI evidence gate"。PR hits a UI surface but carries no screenshot = one COMMENT-level concern (remind the proposing side to add it, or generate it on their behalf this round). Before reusing, first do abroken-image check: for every embedded image in the body, URL without credentials curl -s -o /dev/null -w '%{http_code}' must be 200 (camo anonymous view)—— non-200 = all readers see a broken image, treat as "no image submitted" and leave a COMMENT.
• Hits <UI Face Paths>（.claude/repo-profile.md）→ first judge renderer/widget level vs page level (the criterion takes precedence over whether a daemon exists):
  • renderer/widget level(what is changed is <UI Face Paths> renderer/primitive/widget/css code inside; no standalone blocklet page to run) → must use <ui_shot_script>(real shipped bundle rendering fixture,no daemon needed, parameter matrix/before-after comparison/interaction sequence)."no daemon" on this path is not a reason to skip——<ui_shot_script> does not need a daemon in the first place; hitting this path must produce a screenshot, and it must not be skipped on the grounds of "no daemon".
  • page-level(changing blocklets/** page logic, requiring a running blocklet to render the whole page) → requires a running daemon (cloud routine / local), run /ui-verify --pr <n>; it automatically infers scenarios from the diff, takes screenshots + records video, and posts them back to the PR;daemon unavailable → add ui-verify:pending label（gh pr edit <n> --add-label ui-verify:pending, if the label does not exist first gh label create) + note "ui-verify requires daemon, not run in this environment" —— the label lets a routine with a daemon deterministically scan for it (rather than guessing "which PRs still owe ui-verify" by parsing verdict text, see #1205); after the routine with a daemon successfully re-runs, remove that label (see pr-sweep the pending scan step). But if it also contains renderer changes, the renderer part must still <ui_shot_script>(unaffected by whether a daemon is present).
  • Both paths must ultimately land the same sticky evidence comment (issue #3010)：merge-gate.ts the UI evidence gate only recognizes .claude/skills/ui-verify/scripts/gate-comment.ts generated <!-- ui-verify-report marker comment （sha=+result=PASS|FAIL|BLOCKED|NA), and does not parse screenshot text in the PR body./ui-verify The page-level flow has this step built in; for renderer/widget level, after using <ui_shot_script> to produce the screenshot, the review agent must additionally assemble an evidence JSON (8 fields: headSha/route/viewport/theme/scenario/assertion/consoleStatus/screenshotUrl,scenario write renderer:<组件名>) and feed it to the same gate-comment.ts script; otherwise a pure renderer change PR will get stuck at Gate 3 —— even if the PR body already embeds a ui-shot screenshot. In the verdict, cite the screenshot pass/fail (on par with pre-merge as gating input).Obvious defects hit by the screenshot/self-check checklist (overlap/bare styles/broken interaction/Unknown box) are handled per "★ fix on discovery"—— including exposed pre-existing bugs in main; fix on the spot + issue + fix PR, do not just write "observation unrelated to this PR, recommend review".
• no hit(pure backend/docs/scripts/tests) → skip, not FAIL (note "no UI surface, skip ui-verify" in the verdict).

Step 3.6 — backend/data-plane changes: conditionally trigger profile additional_merge_gates(arc: e2e-gate)

verification(Step 3) cannot see "whether the provider can be mounted,list/read/write RPC returns 500 as soon as it runs, /user whether the data-plane round trip is broken" —— these kinds ofonly visible once the service is startedruntime problems.This step applies only when the profile's additional_merge_gates is non-empty(arc declared [e2e-gate]; repos where this field is [] skip this step entirely, and note "repo has no additional_merge_gates" in the verdict). When applicable, when the PR diff hits the backend surface, run this gate as a review step (symmetric with the Step 3.5 UI face, not entering the deterministic script — the orchestration happens at this layer), and merge the backend smoke report into the verdict evidence.

# diff → scope 推断(命中 Backend Face Paths → RUN,纯文档/UI/前端/测试 → N/A)
bun .claude/skills/e2e-gate/scripts/scope.ts --pr "$n" --json   # backendHit + 要起的 blocklets(arc 的 e2e-gate 脚本)
# backendHit:true → /e2e-gate --pr $n(起受影响 blocklet 跑 Tier A+C,贴 sticky comment)

• Hit <Backend Face Paths>(.claude/repo-profile.md) → run additional_merge_gates declared gates (arc: e2e-gate): only start the blocklets hit by the diff (not the full 7×2), Tier A structural probe + Tier C auth round-trip, and post the results back to the PR as a sticky comment.requires a running daemon (CF side = local miniflare <dev_server_edge>, do not touch staging; <cli_binary> missing/stale, run <cli_setup_command>)first; if the daemon cannot start → record "additional_merge_gates gate requires daemon, skipped in this environment" and note it; the gating is supplemented by the routine / pr-sweep with a daemon (same daemon-absent handling as ui-verify).Runtime defects hit by smoke (500s/round-trip breakage introduced by this PR) are handled per "★ fix on discovery".
• Not hit(pure docs/UI/frontend/tests, or only non-fleet blocklet backend changes) → skip, not FAIL (note in the verdict "no backend face, skipping additional_merge_gates"). Screenshot/static gates and this gate do not substitute for each other: the UI face goes through Step 3.5, the backend face goes through this step.
• The hard block on the merge side is in pr-sweep merge gate's "backend data-plane gate"— this step is advisory input to the decision, the truly non-skippable gating is performed by <merge_gate_entry> dual gate (see the mechanism note at the end of Step 3).

Step 4 — ★ Cross-PR conflict / duplicate / contradiction detection

Two primary keys:same issue and same file。

1. Same issue duplicate(most common, root cause see "source governance" below): multiple machines each run issue-sweep and open a separate PR forthe same issue , with branch names of the form claude/<verb>-<N>-<slug>——issue number <N> the same, verb/slug different. Criteria:

   # 找所有 head branch 含同一 issue 号、或 body 同指一个 issue 的开放 PR
   gh pr list --state open --json number,headRefName,body \
     --jq '.[] | {number, headRefName, fixes: (.body|capture("(?<k>(Fixes|Part of) #\\d+)")?.k)}'
   

   two PRs point to the same #N → compare diffs, classify:exact duplicate(one is redundant)/ contradiction(assertions differ, e.g. the same license spot one writes BUSL-1.1 and another writes BSL-1.1). Givewhich to keep(the more complete/correct/better-evidenced one), judge the other as SUPERSEDE。
2. Same file conflict: the two PRs change the same file. Classify:line-level merge conflict(same region)/ independent(same file, different regions, can coexist)/ semantic contradiction(both change the same config to different values). Shared config files (pnpm-lock.yaml、wrangler.toml、package.json) overlap → flag merge order/second conflict risk.

Criteria for who to keep (for the SUPERSEDE decision): more complete diff > more correct facts (against authoritative sources, e.g. repo LICENSE)> has tests > newer base > first-come (under equal conditions keep the earlier one, close the later one). Write the criteria and evidence into the comment, don't just state the conclusion.

Step 5 — Issue the verdict

• item-by-item verification table:claim → path:line or NOT FOUND.
• cross-cutting impact conclusions(Step 2.5 six dimensions): for each dimension give evidence or explicitly judge "not applicable" — reverse-reference dangling list (path:line)/ parity gaps / end-to-end use-case closure (not just whether a caller exists) / suspicious performance points (including request chain and init) / missing tests / dead code that should be cleaned up.
• verification results:pre-merge PASS/FAIL + root-cause classification for failures (a/b/c/d).
• conflicting conclusions: with which peer, what relationship, which to keep and which to close, and why.
• recommendation: one of 5 categories.MERGE must be premised on Step 3 verification passing; not run or not passed = must not issue MERGE.
• "The PR type is feature" is not a reason to escalate/downgrade. all green + checked item by item + a non-breaking feature, the verdict is exactly MERGE, do not write "feat requires human approval to merge." The only things that require human intervention are: security surface, breaking change (incompatible protocol/wire, schema without migration, data corruption), architecture direction/design A/B undecided, and humans have explicitly objected — the criteria and risk tiers are fully detailed in pr-sweep Step 5(including the "default allow, tighten only when problems arise" ratchet). When escalating, write clearlywhat exactly you are unsure about, not the commit type.
• The value lies inindependent discovery + attribution, not in restating an existing review.

Step 5.5 — human-confirmation block (required for human-escalation verdicts)

Hard precondition (must be answered before escalating, issue #1860): before creating the decision block, first ask "Can I state a safe default action?" — if yes → escalation is forbidden; execute on the spot according to the ratchet (default allow, tighten only when problems arise, see pr-sweep Step 5) and leave a trace; if not, only then is it a true judgment question, and only then does it qualify for the structured decision block below. This hard precondition applies before the structured decision blockbeforego through it once — first ask whether you can decide it yourself, and only if you cannot answer does it fall through to requiring human confirmation.

When the verdict requireshuman intervention——BLOCK(escalate rather than something the agent can self-fix),COMMENT(with blocking concerns), and on the pr-sweep side 🔴 high risk / awaiting-direction|judgment|caution / security——comment cannot stop at "please confirm manually"(#660 is the counterexample: the verification table was very complete, but it broke down at "please confirm manually before merging," without saying what the human should judge or how to verify it). It must include a structured "human-confirmation block":

> 🛑 需人确认 — <一句话: 要你定的那一个决定>
>
> **为什么停在这:** <agent 不能自决的精确原因: 设计 A/B 未定 / 安全边界 / 不可逆 / 缺写权限>
> **agent 已核验(不必重做):** <已坐实的部分, path:line + 测试 pass/fail —— 省去人重复劳动>
> **请你验证(可照跑):**
>   1. `<确切命令>` → 预期 `<X>`;现状 `<Y>`
>   2. 看 `<path:line>` 确认 `<具体属性>`
> **要定的:** <选项 A / 选项 B,各自后果> · **我的推荐:** <X,因为…>
> **定了之后:** A → `<解锁动作/命令>` / B → `<解锁动作>`

Hard check for the two elements (missing either one = the decision block is unqualified and must not be issued) — based on the #1812 retrospective:

1. question + suggested answer: the question to be answered by the human must bespecific and closed-ended(can be answered in one sentence with "yes/no/choose A"), and the agent must attach its own suggested answer — the human only confirms/rejects, and does not do open-ended thinking. An open-ended phrasing like "please confirm manually before merging" is unqualified.
2. choice + difference + recommendation: for each option, write clearlywhat will happen if it is chosen, and how it differs from the other options(not just listing names), and give a recommendation + reason.

If neither can be provided → this is not normal output, it is the discovery of a real problem: explicitly write "⚠️ unable to form a recommendation" + precisely what is missing (which piece of information / which permission / which undecided prerequisite decision). This itself is the highest-priority escalation — more worth a human's attention first than any BLOCK. It is absolutely not allowed to use a vague phrase like "needs manual review" to disguise "I haven't thought it through" as "review completed."

timeliness: the decision block must be pinned tothe current HEAD sha (marked at the start of the block 针对 HEAD <sha7>); once PR receives another push it is invalidated; on re-review refresh the same canonical comment per Step 0.6, do not append a new block.

Iron rule — if you can give steps, give steps; if you can't, give criteria; never fabricate commands:

• Can be reduced to runnable steps(verification failure needs human judgment / code correctness / security properties / dedup conflict / mechanical operation lacking permissions) → fill in "Please verify"Exact command + which path:line + expected vs current, so that a person (or a person's agent) can run it and confirm.
• Cannot be reduced(design A/B with no precedent, architecture direction, taste) → Never invent a command that pretends to be verifiable; replace "Please verify" withoptions + criteria + recommendation, and state explicitly that this is a judgment question.
• What the agent has already verified, don't make people redo: put up the conclusions of mechanical correctness (sanitization, test pass/fail, verification root cause), and let people focus only on the point that truly requires their judgment.

What to fill in for per-type "Please verify":

escalation type	"Please verify" fill in
verification failure needs human judgment	Exact failing check name (check-*)+ pre-merge 's rawTail excerpt + root cause (a/b/c/d) + "reproduce: run <pre_merge_entry>, expected <PASS/FAIL>"
code/logic correctness	trigger point path:line + reproduction test command (<package_manager> --filter <pkg> test / <test_runner> <path>) + expected vs actual behavior
security (must be listed item by item)	eachsecurity property on its own line:<属性: const-time compare / path-traversal guard / authz / 注入过滤> @ path:line + the command or test that verifies it + failure appearance(how it counts as bypassed). The decision still rests with the person, but give a complete checklist,neverjust say "security involved, please have a person look"
dedup/conflict unsure	both PR#s + conflicting line path:line + authoritative source path (repo LICENSE / canonical doc) + comparison command (gh pr diff / git show)
design A/B (judgment question)	do not fabricate commands: crisp description of options A/B + their respective tradeoffs + the intent/issue anchor that triggered it + your recommendation + what evidence can settle it

scope: only the few categories aboverequire human intervention's verdict carries this block;MERGE(no human needed), mechanical items the agent self-fixesdo not carry— don't pile an empty block onto PRs that don't need a human.

Step 6 — post comment (--post time)

Write in Chinese, mark AI identity and read/run scope at the top.The entire header line is generated by a single-point script(cannot be hand-assembled/placeholders/date substituted; engine/model are automatically brought out by the script according to ARC_AGENT_ENGINE/ARC_AGENT_MODEL , don't hand-write "Claude Code()" again — running under Codex would wrongly call itself Claude), append read/run scope at the end of the line:

hdr=$(bash <agent_identity_script> --header "PR Review" --skill pr-review)   # profile 字段
# → "> 🤖 AI Agent PR Review @ <hostname> · runner:<name> · skills@<hash>[ · engine:<kind>[/<model>]]"
echo "${hdr}。读取:PR diff + 受影响代码/文档/测试 + 关联 issue。运行:/agentloop:verification (pre-merge) + <测试命令>。每条结论附可复现证据。"

See root CLAUDE.md "Agent Comment Format" for details (the prefix is the pr-sweep detection predicate, guaranteed by the script not to drift).

# canonical verdict comment:每 PR 唯一,body 必含 marker <!-- pr-review-verdict -->,复审 = 原地 upsert
# (--edit-last 只能编辑「自己」的上一条;上一轮 verdict 可能是别的 runner 发的,必须 marker 定位 + PATCH)
cid=$(gh api "repos/{owner}/{repo}/issues/<n>/comments" --paginate \
      --jq '[.[]|select(.body|contains("<!-- pr-review-verdict -->"))]|last|.id // empty')
if [[ -n "$cid" ]]; then
  gh api -X PATCH "repos/{owner}/{repo}/issues/comments/$cid" -F body=@draft.md   # 跨 runner 原地刷新
else
  gh pr comment <n> --body-file draft.md                                          # 首轮:新建
fi
# gh 不可用时：先安装（apt install gh -y），确实无法安装才用 mcp__github__add_issue_comment
# ⚠️ MCP add_issue_comment 会吃掉 ![截图](...) 的 `!`（图片变纯链接）——含截图时避免用 MCP

• verdict comment is unique per PR (canonical): body must contain <!-- pr-review-verdict --> marker; any re-review (including cross-runner) = upsert the same one, historical versions remain in the GitHub edit history for lookup. Don't pile up duplicates — the stack of 6 comments on #1812 is the direct consequence of this rule being absent. Supplementary artifacts (verification report, ui-verify screenshot comment) are still delivered independently; the verdict only cites their conclusions.
• Default read-only;--post only then write.
• The Step 3 verification report has already been posted separately with --comment ; the verdict comment only cites its conclusion (PASS/FAIL), without repeating the full log.

Autonomy Boundary (Iron Rule)

This skill isengine, with tighter boundaries than sweep:

• Do automatically (when --post): post/edit one verdict comment, adjust labels. Reversible and traceable.
• Never do:gh pr merge(merging is irreversible; leave it to pr-sweep's gated step or a human),gh pr close(closing is left to sweep's dedup step), push to someone else's branch, or modify the PR author's code.
• In one sentence:pr-review only judges + only comments; merge/close are pr-sweep's gated actions.

Key Principles

1. Checking against already-shipped code is the first priority; the verification scope includes cross-cutting impact. A self-consistent PR diff ≠ correct in reality. The most valuable findings are "this fix didn't actually fix it / this doc change doesn't match the shipped surface / this test is a no-op".And: a self-consistent diff also ≠ harmless to the rest of the system — you must step outside the diff and check the six cross-cutting impacts (Step 2.5): reverse references (dangling references left by deletions/renames), cross-package parity (node/cf mirror + declaration↔dispatch pairing), end-to-end usage scenario closure (not just whether there's a caller), performance regression (including request chain, init, cold start), test coverage, cleanup and wrap-up.This is the layer agent review most often misses.
2. Verification gating is a signal, not a judge (there's no CI on the PR anymore). Failures must be diagnosed to root cause (a/b/c/d), and even a PASS doesn't exempt item-by-item verification. Noise-type failures (format warn-only / single-line lint) don't block and can be self-fixed; only real test/type/architecture violations BLOCK.
3. Conflicts must be attributed to who stays and who closes, with criteria. Same issue / same file are the two primary keys; contradictions (e.g. inconsistent license strings) must be adjudicated against an authoritative source — you can't keep both.
4. Every finding has reproducible evidence. path:line / gh Output / real test output. No evidence = don't write it.
5. Artifacts land in the PR, not in the session. After a run = one more verdict comment in the PR that the next round can pick up.
6. The engine only judges, never merges. The irreversible actions of merge/close are left to pr-sweep's gated ladder + human boundaries.
7. When human intervention is needed, give a verification that can be run as-is, not a vague "please confirm". An escalation verdict must include a "needs human confirmation block" (Step 5.5): what you need to judge + what the agent has already verified (to avoid redoing) + how to verify (if it can be reduced to a command, give the command + path:line + expectation; list security items one by one; for judgment calls that can't be reduced, give options + criteria + recommendation, and never fabricate commands) + the unlock actions for each branch once it's decided. Hard checks on the two elements of waving through:question + suggested answer, options + differences + recommendation — if neither can be given = you've found the real problem; escalate explicitly, don't paper over it with "please confirm manually"。
8. One verdict per PR, bounded by sha. the verdict comment is unique across the whole PR (marker upsert, refreshing the same one even across runners); sweep-trace's sha is the freshness machine key—fresh means skip (zero output is the correct output), stale means incremental review. Three agents each doing a full review, comments piling up to six, none of them valid for the current HEAD (#1812) is the pattern this rule is meant to eliminate.

★ sweep-trace instrumentation (L2 observability layer)

At the end of every AI verdict comment emitted by this skillmustappend a line of sweep-trace HTML comment (invisible to humans, grep-able, reused by L1 eval as the golden baseline data source):

<!-- sweep-trace: {"ver":1,"pr":N,"gate":"verdict","val":"<val>","sha":"<head-oid>","run":"<ISO8601>"} -->

Fields:

• ver: schema version, currently 1
• pr: corresponding PR number (numeric)
• gate: fixed value verdict
• val: decision value, taken from the pr-review controlled vocabulary (5 categories):MERGE / COMMENT / SUPERSEDE / BLOCK / CLOSE
• sha: the PR HEAD this verdict targets (40-character commit oid,gh pr view <n> --json headRefOid)——Step 0.6 cross-runner deduplication/freshness determination'smachine key. Old traces lacking this field → always treated as stale
• run: UTC time,new Date().toISOString() format

trace is only appended at the end of the emitted verdict comment; read-only mode (no --post) does not emit a comment and does not append a trace.