Skip to content

Sentinel: stop waiting on reviews the review lane never runs - #22445

Merged
CamSoper merged 1 commit into
masterfrom
camsoper/hopeful-keller-2hxctw
Oct 22, 2026
Merged

CamSoper merged 1 commit into
masterfrom
camsoper/hopeful-keller-2hxctw

Conversation

@CamSoper

Copy link
Copy Markdown
Contributor

Proposed changes

G1 (review-ran) was demanding a review on PRs the review lane skips on purpose. Those PRs sat red until someone waived them or forced a full review of a one-line change. #22417 is what started this: Chris waived a date: bump because review:frontmatter-only had no G1 branch at all, so waiving was the only sane move. I audited every skip reason in claude-code-review.yml against the G1 ladder and found three dead ends plus one fail-open.

Lane Before After
review:frontmatter-only (#22417) G1 red forever. No branch, and date: is outside the mechanical bar's {updated, tags} G1 ok when the diff really is frontmatter-only. G3 still needs a human
Clean review:trivial that misses the mechanical bar (#22429, open now: "two ways" → "three ways") G1 red. The stand-in required triage's prose comment, which triage only posts when it finds something. The claims signal and deletions cap make plenty of trivial PRs non-mechanical G1 ok when the diff really is trivial. G3 still needs a human
pulumi-bot PRs outside content-review/, fix/broken-links-, vale/brand-sync- (e.g., the marketing-web editions sync, #22204) G1 red forever. Triage and the review lane both skip these G1 ok. G3 still needs a human
Zero changed files Internal: red with misleading advice. Fork: every gate skipped → success with nobody involved (and on a transiently empty files read, over a real diff) Red in every lane, with an honest message

Guardrails:

  • The label isn't trusted on its own. Anyone with triage access can apply review:trivial / review:frontmatter-only, so the Sentinel re-runs triage's own classify_pr on the diff and only passes G1 when the shape really fits. If it doesn't, G1 stays red and points at /new-review. A file GitHub sends without a patch reads as binary, so it never fits.
  • A review card at any head keeps the PR on the review track, same rule the trivial stand-in already had. A stale card with an open finding can't pass on a skip label.
  • review:prose-flagged with the triage comment missing stays red. The label promises nits that only the comment can show.
  • G3 is unchanged. A skipped review moves the reading to the approver; it never removes it.

Plumbing:

  • review-sentinel.yml: the label-event filter wakes on review:frontmatter-only.
  • claude-triage.yml: triage re-dispatches the Sentinel when review:frontmatter-only or review:prose-flagged moves, not just review:trivial. Its GITHUB_TOKEN label writes fire no events.
  • REVIEW_LANE_SKIPPED_BOTS / REVIEW_LANE_REVIEWED_BOT_BRANCHES mirror the review lane's bot-author rule. A test pins them to both claude-code-review.yml and triage's if:, so a branch added in one place and not the others fails the suite instead of creating a new dead end.
  • Docs: the sentinel.py docstring, scripts/review-v3/README.md gate table, and CONTRIBUTING §short-circuits.

Testing

Worth a look

The "approver is the review" call means a trivial PR carrying a factual claim (like #22429's count fix) now clears G1 without a model review. Before, it needed /new-review or a waive. That matches what the review lane already decided by skipping it, and content-review PRs like #22429 were fact-checked by their own pipeline. If you'd rather claims get a model pass, the place to change it is the lane's skip bar (or swapping the v2 shortcuts for the mechanical bar, which replay-mechanical.py was built to evaluate), not a G1 that can never pass.

Related issues (optional)

#22417 (waived over this), #22429 (stuck on it right now), #22204 (bot-author shape).


Generated by Claude Code

G1 asked for a review on PRs the review lane skips by policy, so they
sat red until someone waived them or forced a full review:

- review:frontmatter-only had no G1 branch at all (#22417, a one-line
  `date:` bump, was waived).
- A clean review:trivial PR needed triage's prose comment, which triage
  only posts when it finds something. Trivial isn't mechanical (the claims
  signal, the deletions cap), so #22429, a "two" -> "three" fix, sat red.
- pulumi-bot PRs outside content-review/, fix/broken-links-, and
  vale/brand-sync- (the editions sync) never get a review.

G1 now passes those lanes on a PR that never got a review, and G3 still
needs the human approver. The skip labels can be applied by hand, so the
Sentinel re-derives the shape from the diff with triage's own classify_pr
and stays red, pointing at /new-review, when the diff doesn't fit. A
prose-flagged PR whose triage comment is gone stays red, and a review card
at any head keeps a PR on the review track.

Also:
- A PR with no changed files is red in every lane. A fork PR with an
  empty file list used to skip G1, G2, and G3 and conclude success.
- The Sentinel's label filter wakes on review:frontmatter-only, and
  triage re-dispatches the Sentinel when frontmatter-only or prose-flagged
  moves, since its GITHUB_TOKEN label writes fire no events.
- Tests pin the bot-skip lists to both claude-code-review.yml and
  claude-triage.yml.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JFCv9URLXVVabm6YieHZG
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Sentinel — merge gate status

2 of 5 gates need attention before this can merge.

Gate What it needs
🔴 G1 review-ran No current review for faccb35ba — comment /update-review <what changed> to refresh it, or /new-review for a fresh one.
➖ G2 findings-answered Doesn't apply to this PR: no review present — see G1.
🔴 G3 right-approver Needs an approval from anyone with write access to this repo — no qualifying human approval at this head (bot approvals count only on the automated lanes bot_approvers lists).
➖ G4 infra-evidence Doesn't apply to this PR: no changed path affects the deploy.
➖ G5 oversized-ack Doesn't apply to this PR: not oversized.

Break glass: a member of a routing team (.github/review-routing.yml teams:) can apply review:waived, which is logged.

Evaluated at head faccb35ba.


Need help, or think a gate got it wrong? Ask in #docs on Slack.

@pulumi-bot

Copy link
Copy Markdown
Collaborator

@CamSoper
CamSoper marked this pull request as ready for review October 22, 2026 22:31
@CamSoper
CamSoper merged commit 380ad3d into master Oct 22, 2026
16 checks passed
@CamSoper
CamSoper deleted the camsoper/hopeful-keller-2hxctw branch October 22, 2026 22:31
@CamSoper
CamSoper deployed to production October 22, 2026 22:31 — with GitHub Actions Active
@github-actions github-actions Bot added review:triaging Claude Triage is currently classifying the PR domain:infra PR touches workflows, scripts, infra, Makefile, or build config labels Oct 22, 2026
@pulumi-bot
pulumi-bot requested a review from a team October 22, 2026 22:32
@github-actions github-actions Bot removed the review:triaging Claude Triage is currently classifying the PR label Oct 22, 2026

This branch was successfully deployed

2 active deployments
production — faccb35b Deployed Oct 22, 2026 by CamSoper via Record review outcome #14897
testing — faccb35b Deployed Oct 22, 2026 by CamSoper via Do cleanup #14897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain:infra PR touches workflows, scripts, infra, Makefile, or build config

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants