Rework pr-reviewer workflow around suggestions

[aram356]

Summary

• Reworks the pr-reviewer agent around a single review pass that posts GitHub review comments and user-approved suggestion blocks, instead of pushing fixes or opening a stacked fix-up PR.
• Adds explicit PR, branch-remote, and branch-local modes with private review refs, merge-base-aware diff handling, branch-only output, and a head re-check before submission.
• Tightens CI and submission behavior: required checks are classified from gh pr checks --json buckets, failed required CI becomes a blocking body-level finding, pending reviews are preserved unless the user chooses to delete them, and review payloads are built as strict JSON.

Changes

File	Change
.claude/agents/pr-reviewer.md	Replaces the previous fix-up-PR loop with a one-pass review workflow. Documents mode resolution, worktree setup, full changed-file reads, CI classification, finding triage, GitHub suggestion eligibility, scratch verification, verdict precedence, branch-only rendering, PR head drift handling, pending-review handling, and safe gh api review submission.

Closes

Closes #706

Test plan

This is a workflow/documentation-only change under .claude/agents/; no Rust or JS source is affected.

• git diff --check -- .claude/agents/pr-reviewer.md
• Mocked CI capture cases:
  • nonzero exit with JSON -> classify from JSON
  • exit 8 with no JSON -> pending/no-output note
  • nonzero exit with no JSON -> command/API/auth diagnostic
• cargo test --workspace - n/a
• cargo clippy --workspace --all-targets --all-features -- -D warnings - n/a
• cargo fmt --all -- --check - n/a
• JS tests: cd crates/js/lib && npx vitest run - n/a
• JS format: cd crates/js/lib && npm run format - n/a
• Docs format: cd docs && npm run format - n/a, file is outside docs/

Checklist

• Changes follow CLAUDE.md conventions
• No production code changed
• No secrets or credentials committed

[ChristianPavilonis]

Automated review by Yesman.

I found one blocking correctness issue in the updated pr-reviewer instructions. The PR adds a stale-head guardrail in Step 1, but Step 2 still says to enumerate changed files with git diff main...HEAD --name-only (.claude/agents/pr-reviewer.md, current line 59). Because that line is not part of the submitted diff, I could not attach this as an inline comment.

This should use the just-fetched PR head, not the reviewer worktree's local HEAD. Otherwise later passes can enumerate files from an old checkout, a fix-up branch, or another branch entirely, causing the agent to miss current PR changes before submitting a review. Please make it consistent with Step 1, for example git diff origin/<baseRefName>...origin/<headRefName> --name-only after fetching the base, or git diff main...origin/<headRefName> --name-only if main is intentionally fixed.

CI is mostly passing with some checks pending at review time.

[ChristianPavilonis]

Automated review: I reviewed PR #707 against origin/main at head 5ea9211305c040fe8ddce01da70b392aa63db554. The workflow rewrite is generally careful around stale heads, suggestion safety, pending reviews, and JSON review submission, but I found one CI-coverage gap that can cause the reviewer to miss failing Trusted Server gates.

Findings

P1 / High

1. Do not limit CI classification to branch-protection-required checks — .claude/agents/pr-reviewer.md:370
   • Issue: The new CI collection uses gh pr checks --required and the body template later says to render only checks reported by that required-only query. In this repository, that currently returns only cargo test, format-typescript, cargo fmt, and format-docs; it omits checks such as Analyze (rust), vitest, CodeQL, browser integration tests, and integration tests.
   • Why it matters: CLAUDE.md treats those broader workflows as PR gates. If one of the omitted checks fails, the reviewer can still report clean CI and potentially approve/comment with no failed-CI finding, hiding a real regression in Trusted Server's Rust/JS/integration validation.
   • Suggested fix: Query and report all PR checks (for example, gh pr checks "$NUMBER" --json name,bucket,state,link), then separately mark which are required if branch-protection status matters. At minimum, classify failures/cancellations from the full check set as review findings, while preserving the existing required-check handling for merge-blocking branch protection.

CI / Existing Reviews

CI is currently passing. Existing review feedback included an older stale-HEAD concern that appears addressed by the current $DIFF_RANGE / private-ref workflow; I did not duplicate that finding.

[ChristianPavilonis]

Automated review: this required-only query drops several Trusted Server CI gates from the reviewer's view. On this PR, gh pr checks --required returns only four checks (cargo test, format-typescript, cargo fmt, format-docs), while the full check set also includes Analyze (rust), vitest, CodeQL, browser integration tests, integration tests, etc. Since CLAUDE.md treats those workflows as PR gates, a failure in one of the omitted checks could be missed and the reviewer could still report clean CI.

Suggested fix: query the full check set for reporting/classification, and separately annotate which checks are branch-protection-required if needed.