Repository navigation
Lower the advisory quorum to 5: drop the three reviewers that produce no countable signal (re-file of #1894 AC #3) #1997
Description
Story
As the fleet operator,
I want the advisory quorum to count only reviewers that actually produce a countable signal,
so that pr-review stops reaching approval through a timeout fallback on every single PR.
This is a re-file of #1894 AC #3 as a standalone, single-purpose change. PR #1902 has carried it for eight days across five dispatch attempts and never once got a running attempt at it — see "Why this is a fresh issue" below. #1902 keeps its diagnostic work (AC #1/#2); this issue owns the registry change alone.
The change
In scripts/lib/reviewer-sources.tsv, set advisory_gate to no for exactly these three rows:
copilot-pull-request-reviewerchatgpt-codex-connectorqodo-code-review
Record in each row's rationale: dropped from the approval-wait set because zero reviews were observed across eight consecutive PRs (#1888, #1887, #1873, #1799, #1796, #1794, #1793, #1770), measured 2026-09-23.
Leave graphite-app and cubic-dev-ai at advisory_gate=yes. Graphite's zero-review count is a measurement artefact — it reports via the Graphite / AI Reviews check run, so a clean pass posts no PR review and is invisible to the reviews API. Its 2026-08-21 silence was a lapsed 30-day trial; the org moved to the paid plan on 2026-09-22. Record that in its rationale so the next reader does not "re-drop" it.
Resulting count: 5 (gemini-code-assist, sonarqubecloud, codeant-ai, graphite-app, cubic-dev-ai). It was 4 when this was first decided; cubic-dev-ai has since been registered by #1903.
Four copies must move together
The advisory set is duplicated. All four must agree or the denominator silently diverges from the recorded policy:
scripts/lib/reviewer-sources.tsv— the source of truth_advisory_gate_load_fallback_botsinscripts/lib/advisory-review-gate.sh- the
json='[…]'fallback inscripts/lib/approval-diagnostic.sh(only if that file is onmainby then — it currently lives on PR feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent) #1902's branch; skip it if absent and say so) - the
_adv_botsliteral inscripts/review-one-pr.sh
Acceptance Criteria
- The three rows are flipped;
graphite-appandcubic-dev-aiuntouched; every rationale records the reason. - Every present copy above agrees with the registry, with a consistency test for each that is not already covered (
tests/test_reviewer_sources.batsalready covers copies 1–2). tests/dev-lead/unit/test_advisory_review_gate.batsassertsBOTS=7for the unreadable-registry path and must be updated in the same change. It runs in CI viatest-dev-lead.yml, so the PR goes red otherwise. Verified previously withREVIEWER_SOURCES_MANIFEST=/nonexistent/reviewer-sources.tsv.- Nothing hardcodes the count. Derive from
reviewer_sources_advisory_gate_logins; a literal breaks the next time a reviewer is registered, exactly as it just did whencubic-dev-aitook the count from 4 to 5. creates_threadsanddev_lead_trustedare unchanged on every row. The [Phase 4] A review bot that can create blocking threads but is not in dev-lead's TRUSTED_BOTS creates an unclearable merge block (graphite-app; root cause of the 2026-08-02 deadlock) #1425 invariant is unrelated toadvisory_gate; all three dropped bots stay dev-lead-trusted and on the 🔎 Third-Party Reviewer Scorecard — Weekly Report #1220 scorecard. They leave the approval-wait set only.- Do not add a closing keyword for 10 of 11 open PRs have never received an approving review: the advisory quorum is unmeetable, the disposition gate revokes what does issue, and nothing reports why #1894 — that issue stays open for its remaining ACs. Note also that a conventional-commit scope like
fix(#1894):is parsed by GitHub as a closing reference; use a non-issue scope.
Known limitation to record, not fix here
graphite-app stays in the quorum but the gate counts only reviews and comments, so its check-run signal remains uncountable and the quorum still cannot be satisfied on its own terms. That is not a regression (the denominator shrinks from 8 to 5 against the same countable sources) and is out of scope. Note it in the rationale; gate-side check-run counting needs its own issue.
Why this is a fresh issue rather than another nudge on PR #1902
Five attempts, none of which reached a running agent:
| # | Outcome | Cause |
|---|---|---|
| 1–2 | never seen | instruction posted as a PR review body, which creates no review thread for fix-reviews to walk |
| 3 | failure |
reviews-retry dispatch mis-routed to fix-bot-comment |
| 4 | cancelled |
evicted by the per-PR concurrency lane (dev-lead ran at a 45% cancel rate that day, cf. #1741) |
| 5 | targeted other PRs | the successful reviews-retry runs in that window were for #1920 and #1794 |
So there is still no evidence about whether dev-lead can make this change — it has never been given a running attempt. A fresh issue gets a clean branch, a clean concurrency lane, and the instruction in the issue body, which dev-lead reads reliably, rather than in threads on a contested eight-day-old PR that also carries two unresolved bot findings.
References
- 10 of 11 open PRs have never received an approving review: the advisory quorum is unmeetable, the disposition gate revokes what does issue, and nothing reports why #1894 — parent; AC Add @claude delegation, auto-merge, and rebase handling #3 and the "MAINTAINER DECISION 2026-09-23 (revised)" that set the policy
- Register cubic (cubic-dev-ai) as an advisory reviewer — add it to the reviewer council and let dev-lead act on its findings #1903 — registered
cubic-dev-ai, which is why the target count is 5 not 4 - [bug] 42% of dev-lead runs are cancelled before doing any work — review findings sit unaddressed while the PR looks active #1741 — the dev-lead cancellation rate that evicted attempt 4
- PR feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent) #1902 — keeps AC test issue from agent #1/Go-live improvements for PR review agent #2 (the approval diagnostic)
Activity
github-actions commentedon Oct 1, 2026
Thanks for filing this as a focused, well-scoped re-file, and for laying out the four registry copies and the acceptance criteria so clearly. One clarifying question: should
scripts/lib/approval-diagnostic.shbe skipped, with that noted in the PR, if it still lives only on PR #1902's branch when this change is made? Or do you want this change to wait until that file lands onmain?don-petry commentedon Oct 1, 2026
Dev-Lead Implementation Plan
Issue: #1997 — Lower the advisory quorum to 5: drop the three reviewers that produce no countable signal (re-file of #1894 AC #3)
Scope
Flip
advisory_gatetonoforcopilot-pull-request-reviewer,chatgpt-codex-connector, andqodo-code-reviewin the reviewer-source registry, keepgraphite-app/cubic-dev-aiatyes(quorum → 5), and bring the two present advisory-set copies and all affected tests into agreement — deriving fromreviewer_sources_advisory_gate_logins, never a hardcoded count.Implementation Checklist
scripts/lib/reviewer-sources.tsv— flip the three rows toadvisory_gate=no; record the zero-review-over-eight-PRs reason (measured 2026-09-23) in each rationale; add the paid-plan note tographite-app's rationale so it is not re-dropped;cubic-dev-aiuntouched;creates_threads/dev_lead_trustedunchanged on every row ([Phase 4] A review bot that can create blocking threads but is not in dev-lead's TRUSTED_BOTS creates an unclearable merge block (graphite-app; root cause of the 2026-08-02 deadlock) #1425 invariant intact).scripts/lib/advisory-review-gate.sh— update_advisory_gate_load_fallback_bots(copy Go-live improvements for PR review agent #2) to the 5-bot set;RATE_LIMIT_NOTICE_BOTSfallback (all 9 sources) unchanged.scripts/review-one-pr.sh— update the_adv_botsliteral fallback (copy Optimize review: small-PR and incremental fast paths #4) to the 5-bot advisory set (it was already stale at 4).scripts/lib/approval-diagnostic.sh(copy Add @claude delegation, auto-merge, and rebase handling #3) — absent onmain(lives on PR feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent) #1902's branch); skipped, noted here per the issue.Tests to Write / Update
tests/test_reviewer_sources.bats— add a consistency test asserting thereview-one-pr.sh_adv_botsliteral (copy Optimize review: small-PR and incremental fast paths #4) equals the registry advisory-gate projection (not previously covered).tests/dev-lead/unit/test_advisory_review_gate.bats— update the#1538unreadable-registry assertion fromBOTS=8toBOTS=5(AC Add @claude delegation, auto-merge, and rebase handling #3;NOTICE=9unchanged); update runtime tests coupled to the now-dropped bots (codex/qodo) to use a still-advisory bot or to assert the dropped bots are no longer gated.ADVISORY_BOTS == registry projectiontest auto-adapts (AC Optimize review: small-PR and incremental fast paths #4).Test Command
bats tests/test_reviewer_sources.bats tests/dev-lead/unit/test_advisory_review_gate.bats(full suite +bash .dev-lead/scripts/dev-lead-lint.shin Phase 4)don-petry commentedon Oct 1, 2026
dev-lead progress note — in progress (not final)
Implementation for #1997 is drafted and passing local verification. Posting this as an interim status; the change still needs CI + reviewer sign-off before it should be considered done.
What the change does
Lowers the advisory review quorum to 5 by dropping three reviewer bots that produced no countable signal, while recording why in each row's rationale.
scripts/lib/reviewer-sources.tsv(source of truth):advisory_gateflippedyes → noforcopilot-pull-request-reviewer,chatgpt-codex-connector,qodo-code-review. Each rationale now records: dropped from the approval-wait set because zero reviews were observed across eight consecutive PRs (feat: implement issue #1565 — Auto-pause companion: usage-telemetry adapter + scheduled poller that sets/clears AGENTS_PAUSED from the Claude budget #1888, feat: implement issue #1881 — auto-rebase still merges main into every open PR on every push, though the ruleset no longer requires up-to-date branches — the treadmill's driver outlived the strict fix #1887, feat: implement issue #1871 — Merge-queue prerequisite: no workflow triggers on merge_group, and CodeQL default setup cannot — all five required checks would wedge the queue #1873, feat: implement issue #1743 — Harness cannot resolve a bot review thread whose correct response is 'no change needed' — false positives block merge permanently #1799, feat: implement issue #1733 — feature-ideation.yml: every run fails at 'Install Python jsonschema' — requirements file missing from the pinned tooling checkout #1796, feat: implement issue #1728 — [Phase 2] Teach the interaction-model section 4 table and validate-interaction-model about a multi-role ingress file #1794, feat: implement issue #1754 — pr-review counts a human-escalation as a posted review — run reports "1 reviews posted" with nothing on the PR #1793, feat: implement issue #1766 — [bug] pr-review approves PRs with unresolved threads (violates its own decision gate 4) and double-reviews the same SHA #1770), measured 2026-09-23. All three staydev_lead_trusted=yesand on the scorecard — they leave only the approval-wait set.graphite-appandcubic-dev-aileft atadvisory_gate=yes. graphite's rationale now explains its zero-review count is a measurement artefact (clean passes post only theGraphite / AI Reviewscheck run, invisible to the reviews API), that its 2026-08-21 silence was a lapsed trial, and that the org moved to the paid plan on 2026-09-22 — so it must not be re-dropped. The known gate-side limitation (the advisory gate counts only reviews/comments, so graphite's check-run signal stays uncountable by the gate) is noted as out-of-scope, needing its own issue.gemini-code-assist,sonarqubecloud,codeant-ai,graphite-app,cubic-dev-ai.The copies of the set are kept in agreement
_advisory_gate_load_fallback_botsinscripts/lib/advisory-review-gate.sh: reduced to the same 5 (its unreadable-registry fallback).approval-diagnostic.sh: absent onmain, so there was nothing to update (as the issue anticipated)._adv_botsliteral inscripts/review-one-pr.sh: updated to the same 5. (The prior literal had already drifted to a stale 4-bot list; a new consistency test now guards it.)RATE_LIMIT_NOTICE_BOTSsuperset (all registered sources) is unchanged.reviewer_sources_advisory_gate_logins.Tests (written before the implementation, red → green)
tests/test_reviewer_sources.bats: added a copy-Optimize review: small-PR and incremental fast paths #4 consistency test (review-one-pr.sh_adv_botsliteral == registry advisory-gate projection). The existing tests already cover copies test issue from agent #1–Go-live improvements for PR review agent #2.tests/dev-lead/unit/test_advisory_review_gate.bats: corrected the unreadable-registry assertion (BOTS=8 → BOTS=5,NOTICE=9kept); repointed runtime examples that had used the now-dropped bots onto still-advisory bots; and converted the two Qodo cases into regression tests asserting a dropped bot is no longer gated.#1425invariant preserved:creates_threads/dev_lead_trustedunchanged on every row.Local verification so far
shellcheckclean on the changed scripts; dev-lead lint passes.Leaving CI and reviewer verification to run before this is marked complete.
don-petry commentedon Oct 1, 2026
Dev-Lead: Implementation Complete — PR #2003
The implementation for issue #1997 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.
86b3029c8b5636362520aff92295faa1f54baa2bmain: 5 file(s), +58/-21 linesReview happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.
feat: implement issue #1997 — Lower the advisory quorum to 5: drop th…