ai:blocked-on sits with the vetter: typed dependency refs, automated clearance on dep merge, human polling removed - #164
Conversation
…vetter-lane emission Closes #161. - flag-blocked-on gains repeatable --blocked-by <owner/repo#n>, stored as machine-readable blocked-by lines in the flag comment alongside the prose reason; a new flag without at least one typed ref is REFUSED (fail closed on the input that makes the state automatable). Refs parse through parse_pr_ref — the one owner/repo#number parser — into a typed BlockedByRef; clearance is never judged from prose. - The vetter's state-load (unvetted) runs the clearance check every run: all typed deps MERGED/CLOSED => post a 'Blocked-on cleared:' vetter comment (deliberately not a verdict, so vetted_at_head goes false at an unmoved head) + clear the label => the PR re-enters vetting fresh. Any dep OPEN => held, untouched, withheld from the vet queue with open deps named. No typed refs (the 17 legacy prose-only flags) / malformed line / unresolvable ref => blockedOnManualReview — visible, named, never auto-anything. - human-queue emission: ai:blocked-on files under the vet-lifecycle lane (the vetter is its next mover), out of producer-blocked; the leak check recognises the clearance marker as a modeled transition. The rain-org-health STATES-row owner flip follows as a separate PR. - Legacy flags are NOT parsed or migrated in code: migration is an eyes-on re-flag pass; until then they sit safely (and loudly) in manual review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🤖 ai:producer Tests ( Mutation ledger (each applied to the committed baseline 62f9079, full suite per mutant, then restored — 13/13 KILLED):
Mutations 0–3 are the #157-style partition proof: the three clearance outcomes (Clear / Hold / ManualReview) are disjoint, and every comparison swap between them is caught by the one partition test that enumerates the dep-state combinations. |
|
Reviewed c904a98: ready — typed blocked-by refs via the one parser with pre-network refusal, clearance in the vetter state-load on the latency argument, clearance-comment-as-un-vet forcing a genuine re-vet under the full chain, manual-review dominating hold with the 17 legacy flags safe, emission moved to the vetter's grouping, no protocol bump verified from the readers, 13/13 mutants with the four-way partition test, pre-existing flag-strip hole closed. |
Closes #161
The two rulings, honored
The judgement is now automated and it is the vetter's: the dependency is a typed
owner/repo#nref stored machine-readably in the flag comment (parsed byparse_pr_ref, the codebase's one subject-ref parser, into a typedBlockedByRef— never classified out of prose), and the vetter's own state-load resolves those refs every run and clears the flag when every dep is MERGED or CLOSED.flag-blocked-onnow refuses a new flag without at least one--blocked-by <owner/repo#n>— fail closed on the exact input that makes the state automatable; the prose reason stays for the WHY, alongside.Nothing anywhere asks anyone to merge a dependency: the dep flows through vet → ready → merge like every PR. What changed is the presentation side: a held blocked-on PR is now withheld from the vet queue (a verdict on it would strip the modeled blocked state outside any transition and could present a not-truly-ready PR), and a cleared one re-vets fresh — the clearance comment is deliberately not a verdict, so
vetted_at_headgoes false even at an unmoved head. The dep landing may change what correct means; clearance is never a rubber stamp back toai:ready.Mechanism choice: the vetter's state-load, not gc — the latency argument
The check on the ruling is "a dependent PR unblocks within one vetter run of its dependency merging." Only the state-load delivers that:
unvettedcall, so the first vetter run after the dep merges clears the flag and vets the PR in the same run — worst case one inter-run gap (≤4h at the 6×/day cadence), i.e. exactly "within one vetter run of the dep merging". Being inside the tool also means the prompt cannot forget it: the vetter has no Bash, and its state-load is the one call it cannot skip.gh pr/issue view --json statecalls (one per typed ref per blocked PR, ~17 PRs today) inside a call that already pays onegh pr viewper open PR. The tokens it does add are the re-vets of cleared PRs, which are the ruling's point, not overhead.Write ordering is fail-safe: clearance posts the comment first (the un-vet + the durable record), removes the label second; if the removal fails the next state-load recomputes Clear, dedups the identical comment, and retries. The reverse order could strand a label-less PR that still reads vetted-at-head — silently stuck.
The emitted comment shapes
The flag (prose + typed lines, alongside not instead):
The clearance record (a trusted vetter comment, deliberately not a verdict — no
Reviewed <sha>:, novet-protocolstamp, both asserted structurally in tests):On
vetted_at_head/record_gate: this change deliberately touches whatvetted_at_headreads — the clearance comment becoming the newest🤖 ai:vettercomment is the un-vet mechanism. I checked every reader oflast_vetter_comment:cost_from_comment(queue) and thenext_readynote read it only behindvetted_at_head == true, which a clearance comment makes false, so neither can read a clearance as a verdict;should_skip_commentrequiresReviewed <sha>: <verdict>+ a current stamp, which the clearance never spells. NoVET_PROTOCOLbump (stays 4): what vetting means is unchanged — clearance changes one PR's input state, not the vet function; the verdict contract,record_verdict's gates, and the comment grammar are untouched.The state machine of clearance (partitioned, #157 lesson applied)
Every
ai:blocked-onPR lands in exactly one of three disjoint outcomes each state-load:blockedOnarray with the open deps named.blocked-byline, an unresolvable ref, or a failed clearance write →blockedOnManualReview, every reason naming its ref and why. Never auto-cleared, never auto-vetted, never silent. Manual-review dominates Hold: a broken typed record needs eyes now, not when it becomes the last dep standing.clearance_partitions_every_dep_combinationenumerates the combinations (empty / merged / closed / merged+closed / open-among-cleared / unresolvable-among-cleared / unresolvable-among-open / malformed) and asserts exact variants and payloads — the swap-catching partition test the #157 sibling asked for; the mutation pass below shows each comparison swap is killed by it.The 17 legacy prose-only flags
Not parsed, not migrated in code. With no typed refs they land in
blockedOnManualReview— visible in every state-load with the re-flag instruction as the stated reason — and the clearance can never touch them (Clearrequires every dep to have positively resolved, and an empty dep set short-circuits to manual review before any resolution). Migration is the eyes-on re-flag pass the issue describes; until then they stay human-owned and loud.Ownership in the queue emission
classify_lanenow filesai:blocked-onunder vet-lifecycle (the vetter's lane — its act is "clear when deps merge"), out of producer-blocked, at the same precedence (still dominates a staleai:ready; still dominated by a human decision). The human-readablehuman-queueprints it with the vetter's group, titled with the vetter's act. The leak check learned the clearance marker: an unlabelled PR whose newest trusted hand-off isBlocked-on cleared:is a modeled transition into un-vetted, not an FSM leak (a producer note after the clearance is a leak again). Legacystates/counts.blockedOnkeys are unchanged for existing dashboard reads.Follow-up (separate PR, not here): the rain-org-health STATES-row owner flip (file
ai:blocked-onunder vetter action, #148-shaped: owner flip + test updates) follows on rain-org-health once this lands. This PR does not touch that repo.QA
blocked_on_tests::{blocked_by_ref_round_trips_through_the_one_parser, blocked_by_ref_rejects_garbage, a_new_flag_without_a_typed_ref_is_refused, the_flag_comment_carries_prose_and_typed_lines_and_round_trips, blocked_by_lines_is_line_anchored_and_fails_closed, the_live_flag_is_the_most_recent_blocked_on_comment, clearance_partitions_every_dep_combination, dep_state_parses_exactly_the_three_github_states, the_clearance_comment_un_vets_the_pr_at_an_unmoved_head, leak_reason_treats_the_clearance_as_a_modeled_transition, unvetted_doc_reports_held_and_manual_review_blocked_prs}, plus extendedfsm_completeness_tests::{classify_lane_maps_every_state_by_precedence, lanes_doc_emits_every_state_with_the_right_members}and thecli_tests--blocked-byparse — each fails on base (the eleven new tests name functions absent on base and do not compile there; the lane assertions assertvet-lifecycle/producer-blocked=0where base emitsproducer-blocked=1, the exact behavior mutation M7 restores and the suite kills). Full suite: 647 + 7 + 47 + 8 + 7 pass, clippy-D warningsclean,cargo fmtapplied.clearance_partitions_every_dep_combination; Hold-before-ManualReview dominance swap → same; empty-refs manual-review arm dropped → same; unresolvable-ref-silently-held → same;blocked_by_guardempty-refusal dropped →a_new_flag_without_a_typed_ref_is_refused;"OPEN"→Mergedstate-map swap →dep_state_parses_exactly_the_three_github_states+ partition test; lane filed back to producer-blocked →classify_lane_maps_every_state_by_precedence+lanes_doc_emits_every_state_with_the_right_members; leak-check clearance arm dropped →leak_reason_treats_the_clearance_as_a_modeled_transition; clearance comment stamped withvet-protocol→the_clearance_comment_un_vets_the_pr_at_an_unmoved_head; held rows misfiled to manual-review in the doc →unvetted_doc_reports_held_and_manual_review_blocked_prs; typed-dep lines silently unwritten →the_flag_comment_carries_prose_and_typed_lines_and_round_trips; oldest-flag-wins →the_live_flag_is_the_most_recent_blocked_on_comment; malformed-line-silently-dropped →blocked_by_lines_is_line_anchored_and_fails_closed.gh --json state), and the existing FSM invariants (vetted_at_headcache-key semantics, marker-by-author trust, one-modeled-state-per-PR) — expected values written from the issue text and GitHub's API contract, independent of the implementation.--blocked-byrefs stored machine-readably + refuse flag without one, (2) automated clearance with fresh re-vet / hold-on-open / manual-review-on-unresolvable, (3) queue-emission ownership move to the vetter, (4) legacy flags migrated by eyes not code. Covered (1), (2), (3), and (4)-as-specified (mechanism ships, migration deliberately not automated; manual-review is what keeps the legacy set safe meanwhile). The rain-org-health STATES flip is named as the separate follow-up the issue itself scopes out.