docs(contributor): require Frontier-tier models; encode review-loop detection#6579
Conversation
…etection Restrict AI-CONTRIBUTOR to Frontier tier only. The Standard row (claude-sonnet-*, claude-haiku-*, gpt-5-4 and below, codex-5-4 and below) is removed rather than trimmed, and every passage that depended on a two-tier world moves with it: the "assume Standard" fallback becomes an abort, "Pre-PR gates (all tiers)" drops the qualifier, the Tier: block loses its Standard alternative, and the Appendix B copy-paste prompt now tells a sub-Frontier runtime to stop instead of "proceed even if your runtime is below that". That prompt is the one contributors actually paste, so leaving it stale would have kept inviting the PRs the gate is meant to prevent. Model tier is a hard gate; the agentic harness is not. A Co-authored-by: Cursor trailer raises scrutiny to the full evidence bar and forfeits light-touch, but does not by itself close a PR -- the failure mode is CI-as-correction-loop, not the tool. The behavioural tells are what earn a standing change: reverting a fix to push a diagnostic commit so CI prints a value, or deleting a passing assertion to turn a job green. pr-review-loop gains the matching detection recipe (check both the canonical Model: line and commit trailers, and read what matched -- a bare `rg cursor` hits WordCursor), and its skip route now closes on sight, dequeues first, and follows alias_of across an account's alts. Also fixes a defect in the compile-error attribution recipe shipped in 6576: `git cat-file -e "$sha:<path>"` is corrupted before git sees it here, swallowing the ":" plus the path's first character, so paired with `2>/dev/null || echo "file absent"` it silently fabricates evidence of maintainer-caused staleness for any crates/ path. Uses git ls-tree, which takes rev and path as separate arguments.
📝 WalkthroughWalkthroughThe changes make Frontier-tier model usage mandatory across contributor guidance and review-loop policy. They also refine skip handling, cursor-signal detection, and maintainer-caused staleness checks. ChangesPolicy and Review Protocol
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/pr-review-loop/SKILL.md:
- Around line 77-79: The skip route in the review loop must not directly dequeue
or close pull requests because those mutations belong to
pr-contribution-handler. Update the contributor-standing skip handling to emit
the appropriate handler action instead, and require live verification that the
handler executed the dequeue and closure before recording completion; preserve
alias enumeration, hard-stop escalation, and the no-review behavior.
- Around line 77-79: Update the skip handling instructions around the “skip”
decision to require a live recheck immediately before every mutating action,
including closure and dequeue operations. Revalidate each PR’s current head,
body, state, auto-merge status, and applicable standing override before acting,
and apply the same checks to every alias PR; do not rely solely on the generated
dashboard packet.
- Around line 61-70: Update the model-enforcement guidance around the PR body
and commit inspection commands to implement a positive Frontier-only allowlist:
parse canonical Model/Tier fields, abort on missing, malformed, or unknown
values, and reject all documented non-Frontier model families. Inspect
structured commit model and Co-authored-by trailer evidence rather than
arbitrary commit prose, covering the complete documented set consistently.
In `@docs/AI-CONTRIBUTOR.md`:
- Around line 347-349: Update the ## LLM canonical PR template section to
include the required Tier: Frontier line alongside Model and Thinking, ensuring
generated PR bodies satisfy the documented tier requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 539ff0ae-e93c-45d7-bcb9-3502f2dc3551
📒 Files selected for processing (2)
.claude/skills/pr-review-loop/SKILL.mddocs/AI-CONTRIBUTOR.md
| **Model tier is a hard gate; the agentic harness is a scrutiny signal.** `docs/AI-CONTRIBUTOR.md` §0.1.1 accepts **Frontier tier only**. A PR whose canonical `Model:` line names a non-Frontier model (`claude-sonnet-*`, `claude-haiku-*`, `composer-*`, `gpt-5-4` or below, `codex-5-4` or below), or whose commits show one, is closed as out-of-policy without an implementation review. Separately, a `Co-authored-by: Cursor <cursoragent@cursor.com>` trailer is **not** grounds to close on its own — it raises scrutiny to the full evidence bar and forfeits light-touch, because the observed failure mode is CI-as-correction-loop rather than the harness itself. Judge the PR on its merits; good work under Cursor is still good work. | ||
|
|
||
| Check both surfaces, and read what actually matched before acting — a bare `rg cursor` hits `WordCursor` and other identifiers: | ||
|
|
||
| ```bash | ||
| gh pr view <PR> --repo <owner>/<repo> --json body --jq .body | rg -i '^\s*\**Model:' | ||
| gh api repos/<owner>/<repo>/pulls/<PR>/commits --paginate \ | ||
| --jq '.[] | "\(.commit.author.name) <\(.commit.author.email)> :: \(.commit.message)"' \ | ||
| | rg -i 'cursoragent@cursor\.com|composer|haiku' | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make model enforcement a positive allowlist and cover the full commit evidence surface.
The body command only finds a Model: line; it does not reject unknown or malformed values. The commit scan also misses claude-sonnet-*, gpt-5-4 and below, and codex-5-4 and below, while matching arbitrary commit prose can create false positives. This does not implement the documented “Frontier only; unknown means abort” policy. Parse the canonical Model:/Tier: fields and inspect structured model/trailer evidence against the same allowlist.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 250: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/pr-review-loop/SKILL.md around lines 61 - 70, Update the
model-enforcement guidance around the PR body and commit inspection commands to
implement a positive Frontier-only allowlist: parse canonical Model/Tier fields,
abort on missing, malformed, or unknown values, and reject all documented
non-Frontier model families. Inspect structured commit model and Co-authored-by
trailer evidence rather than arbitrary commit prose, covering the complete
documented set consistently.
| - `skip` — disambiguate by `reason`: `closed` / `self_authored` need no action. `contributor_standing_skip` is an explicit maintainer standing override and **means close on sight, not merely decline to review**: post the standard closure comment, then `gh pr close <PR>`, then record the event. Do not review, approve, or enqueue. If the PR is in the merge queue, **dequeue it first** (GraphQL `dequeuePullRequest` with input field `id` — `gh pr merge --disable-auto` does not dequeue) so it cannot land while you work through the rest of the account's PRs. Check `skip_standing_policy` in `private-overrides.json` for the current action; a standing entry may carry `close_on_sight` and an `alias_of` list of the same person's other accounts — enumerate every alias's open PRs, not just the one that surfaced. A skip-listed contributor touching hard-stop paths still surfaces as `request_changes` (safety outranks the skip). | ||
|
|
||
| A prior approval on a skip-listed account's PR is **not** evidence it should still land. Where the standing was set because the account farmed CI and review feedback as its correction loop, "approved and green" is the very metric that was being optimized, so it carries no independent signal. Re-enqueue only on evidence you gathered yourself after the standing was set. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the mutation-ownership conflict in the skip route.
This section instructs the review loop to close PRs and dequeue merge-queue entries directly, but Authorized Mode states that dequeue and other GitHub mutations belong to pr-contribution-handler and must not be performed here. Choose one owner—preferably emit a handler action—and define the required live verification so the close-on-sight path cannot be skipped or executed through the wrong boundary.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 250: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/pr-review-loop/SKILL.md around lines 77 - 79, The skip route
in the review loop must not directly dequeue or close pull requests because
those mutations belong to pr-contribution-handler. Update the
contributor-standing skip handling to emit the appropriate handler action
instead, and require live verification that the handler executed the dequeue and
closure before recording completion; preserve alias enumeration, hard-stop
escalation, and the no-review behavior.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Live-recheck skip decisions before mutating PR state.
Unlike the decline route, skip has no mandatory recheck immediately before closing or dequeuing. A new head, changed body, queue state, or standing override can arrive after the dashboard packet is generated, causing the loop to close or dequeue a PR that no longer matches the recommendation. Recheck the current head, body, state, auto-merge, and standing before each skip action, including alias PRs.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 250: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/pr-review-loop/SKILL.md around lines 77 - 79, Update the skip
handling instructions around the “skip” decision to require a live recheck
immediately before every mutating action, including closure and dequeue
operations. Revalidate each PR’s current head, body, state, auto-merge status,
and applicable standing override before acting, and apply the same checks to
every alias PR; do not rely solely on the generated dashboard packet.
| ## LLM | ||
| Model: <claude-opus-4-7 | claude-sonnet-4-6 | codex-5.4 | …> | ||
| Model: <claude-opus-4-8 | gpt-5-5 | codex-5-5 | …> # Frontier tier only — see §0.1.1 | ||
| Thinking: <high | max> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the required Tier: line to the canonical PR template.
§0.1.4 requires every PR body to contain Tier: Frontier, but this template only emits Model: and Thinking:. Contributors following it will produce otherwise-valid PRs that the handler closes as malformed. Add the canonical Tier line.
Proposed fix
## LLM
Model: <claude-opus-4-8 | gpt-5-5 | codex-5-5 | …> # Frontier tier only — see §0.1.1
+Tier: Frontier
Thinking: <high | max>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## LLM | |
| Model: <claude-opus-4-7 | claude-sonnet-4-6 | codex-5.4 | …> | |
| Model: <claude-opus-4-8 | gpt-5-5 | codex-5-5 | …> # Frontier tier only — see §0.1.1 | |
| Thinking: <high | max> | |
| ## LLM | |
| Model: <claude-opus-4-8 | gpt-5-5 | codex-5-5 | …> # Frontier tier only — see §0.1.1 | |
| Tier: Frontier | |
| Thinking: <high | max> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/AI-CONTRIBUTOR.md` around lines 347 - 349, Update the ## LLM canonical
PR template section to include the required Tier: Frontier line alongside Model
and Thinking, ensuring generated PR bodies satisfy the documented tier
requirement.
Resolves the deterministic crates/engine/tests/integration/main.rs mod-line conflict by keeping both test modules in rustfmt-sorted order. Maintainer-side port: main advanced under this PR tonight (phase-rs#6538 phase-rs#6543 phase-rs#6545 phase-rs#6554 phase-rs#6563 phase-rs#6579). The phase-tauri 0.35.1->0.35.2 client/src-tauri/Cargo.lock bump is now a no-op, since main landed the identical bump in phase-rs#6568.
…issolved merge groups (phase-rs#6581) The Frontier-only tier rule landed in phase-rs#6579 with no cutoff clause, so read literally it closes PRs opened weeks earlier that declared Sonnet or Composer accurately under the then-current Standard tier. That inverts the incentive the disclosure rule exists to create: the only PRs the rule can find are the ones whose authors filled the field in honestly. Scope it to PRs opened on or after 2026-07-24 and cross-reference the cutoff from the Tier-line section. Add a janitor for dissolved merge groups. Every queue reformation deletes the gh-readonly-queue refs and mints new ones, and ci.yml's concurrency group is keyed on github.ref -- unique per synthesized ref -- so cancel-in-progress can never match the superseded runs. One evening accumulated 22 such runs (~198 queued jobs) against a 20-job concurrency cap. This cannot be fixed in the concurrency key: GitHub's expression language has no regex or split, so a concurrency.group cannot extract the PR number from the ref, and keying on the base branch would cancel live speculative groups -- which the queue reads as a failed check and evicts the entry for. Cancelling runs whose ref no longer resolves is the safe equivalent, and only queued/in_progress runs are considered so a completed run whose ref was reaped is never touched. Paired with a ruleset change (applied out-of-band): max_entries_to_build 5 -> 2, since 5 speculative groups x 9 jobs requests 45 jobs against a 20-job cap; and check_response_timeout_minutes 60 -> 120, which evicted an approved PR ten seconds past deadline during a hosted-runner stall whose CI then passed. Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…etection (phase-rs#6579) Restrict AI-CONTRIBUTOR to Frontier tier only. The Standard row (claude-sonnet-*, claude-haiku-*, gpt-5-4 and below, codex-5-4 and below) is removed rather than trimmed, and every passage that depended on a two-tier world moves with it: the "assume Standard" fallback becomes an abort, "Pre-PR gates (all tiers)" drops the qualifier, the Tier: block loses its Standard alternative, and the Appendix B copy-paste prompt now tells a sub-Frontier runtime to stop instead of "proceed even if your runtime is below that". That prompt is the one contributors actually paste, so leaving it stale would have kept inviting the PRs the gate is meant to prevent. Model tier is a hard gate; the agentic harness is not. A Co-authored-by: Cursor trailer raises scrutiny to the full evidence bar and forfeits light-touch, but does not by itself close a PR -- the failure mode is CI-as-correction-loop, not the tool. The behavioural tells are what earn a standing change: reverting a fix to push a diagnostic commit so CI prints a value, or deleting a passing assertion to turn a job green. pr-review-loop gains the matching detection recipe (check both the canonical Model: line and commit trailers, and read what matched -- a bare `rg cursor` hits WordCursor), and its skip route now closes on sight, dequeues first, and follows alias_of across an account's alts. Also fixes a defect in the compile-error attribution recipe shipped in 6576: `git cat-file -e "$sha:<path>"` is corrupted before git sees it here, swallowing the ":" plus the path's first character, so paired with `2>/dev/null || echo "file absent"` it silently fabricates evidence of maintainer-caused staleness for any crates/ path. Uses git ls-tree, which takes rev and path as separate arguments. Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
Restrict AI-CONTRIBUTOR to Frontier tier only. The Standard row
(claude-sonnet-, claude-haiku-, gpt-5-4 and below, codex-5-4 and below)
is removed rather than trimmed, and every passage that depended on a
two-tier world moves with it: the "assume Standard" fallback becomes an
abort, "Pre-PR gates (all tiers)" drops the qualifier, the Tier: block
loses its Standard alternative, and the Appendix B copy-paste prompt now
tells a sub-Frontier runtime to stop instead of "proceed even if your
runtime is below that". That prompt is the one contributors actually
paste, so leaving it stale would have kept inviting the PRs the gate is
meant to prevent.
Model tier is a hard gate; the agentic harness is not. A
Co-authored-by: Cursor trailer raises scrutiny to the full evidence bar
and forfeits light-touch, but does not by itself close a PR -- the
failure mode is CI-as-correction-loop, not the tool. The behavioural
tells are what earn a standing change: reverting a fix to push a
diagnostic commit so CI prints a value, or deleting a passing assertion
to turn a job green.
pr-review-loop gains the matching detection recipe (check both the
canonical Model: line and commit trailers, and read what matched --
a bare
rg cursorhits WordCursor), and its skip route now closes onsight, dequeues first, and follows alias_of across an account's alts.
Also fixes a defect in the compile-error attribution recipe shipped in
6576:
git cat-file -e "$sha:<path>"is corrupted before git sees ithere, swallowing the ":" plus the path's first character, so paired with
2>/dev/null || echo "file absent"it silently fabricates evidence ofmaintainer-caused staleness for any crates/ path. Uses git ls-tree,
which takes rev and path as separate arguments.
Summary by CodeRabbit