Repository navigation
Advisory gate cannot count Graphite's check-run clean pass: every clean PR waits out the 20-minute timeout #2005
Description
Problem
graphite-app is advisory_gate=yes in scripts/lib/reviewer-sources.tsv, but its clean pass is reported only as the Graphite / AI Reviews check run: no PR review, no comment. The advisory gate (scripts/lib/advisory-review-gate.sh, check_advisory_reviews) counts only reviews and comments, so a clean Graphite pass is invisible to it.
Effect: on a clean PR the gate can never reach the full quorum of 5 from real signal. It waits out ADVISORY_HEAD_AGE_TIMEOUT_SEC (1200s) and proceeds on partial evidence. It does not strand the PR, but it adds a fixed 20-minute delay to every clean PR.
Raised by cubic on PR #2003 and deliberately deferred there. Graphite was kept at advisory_gate=yes on purpose: its zero-review count is a measurement artefact, and the org is now on the paid plan.
Acceptance criteria
check_advisory_reviewscounts a completed, successful check run whose name matches the registry'scheck_run_namecolumn as a clean submission from that bot, for the current head SHA only.- A failed or neutral check run is not counted as clean; a pending one is not counted yet.
- The registry column
check_run_nameis the single source of truth. No check-run names are hard-coded in the gate. - Bats coverage: clean check run counted; failed check run not counted; check run on a stale SHA not counted; a bot with
check_run_name=-is unaffected. - The scorecard behaviour from Reviewer scorecard misses Graphite's check-run reviews (clean passes read as 'No response') and undercounts its PR reviews ~8× #1908 is unchanged.
Out of scope
Changing graphite's advisory_gate value. The decision is recorded on the registry row: do not re-drop it.
Follow-up to #1997 / PR #2003. Context: #1894.
Measured answer to the open question (2026-10-02)
Checked against real data, not assumed. Of the last 40 merged PRs, 22 carried a check run named exactly Graphite / AI Reviews (equal to the registry's check_run_name), posted by app slug graphite-app, and all 22 had status=completed, conclusion=success. Zero other conclusions were observed. So matching the registry column is sufficient for the name, with two additions:
- A missing check run is not a clean pass. Some heads (e.g. the head of PR 2003) have no Graphite check at all. Absence must stay "no submission", never counted.
- Pin the app slug as well as the name. Any app with
checks:writecan create a check run with an arbitrary name. Count a check run only whenapp.slugequals the bot's registry login (graphite-app), so another app cannot satisfy the quorum by naming a check identically. - A
successcheck run means "Graphite reviewed this head", not "Graphite has no findings". Findings still arrive as inline threads and are gated by the existing disposition logic. Count the check run as a submission from the bot for this head SHA, exactly as a review would be counted, and add no code that treats it as clearing any finding.
Activity
github-actions commentedon Oct 1, 2026
Thanks for the detailed report and for tracking the context back to #1997 / PR #2003. The description and acceptance criteria are clear, so I only have one question: on a clean PR where Graphite reports only the
Graphite / AI Reviewscheck run, can you confirm that check run's conclusion issuccessand that its name exactly matches the registry'scheck_run_namevalue for graphite-app? That would confirm that matching the registry column is enough to count it as a clean pass.don-petry commentedon Oct 2, 2026
Dev-Lead Implementation Plan
Issue: #2005 — Advisory gate cannot count Graphite's check-run clean pass: every clean PR waits out the 20-minute timeout
Scope
Make
check_advisory_reviewscount a completed, successful check run, named in the registry'scheck_run_namecolumn for anadvisory_gate=yesbot, as a clean (COMMENTED) submission from that bot, for the PR's current head SHA only.Implementation Checklist
scripts/lib/advisory-review-gate.sh: build a login→check-run-name map fromreviewer_sources_check_run_reporters, keeping only advisory-gate bots. No names are hard-coded; if the registry is absent, the map is empty and no check runs are counted.get_advisory_bot_states: addheadRefOidto the existinggh pr viewfetch. When the map is non-empty, fetchrepos/{owner}/{repo}/commits/{sha}/check-runs(paginated), then keep only runs wherehead_sha == headRefOid,namematches the registry,status == completedandconclusion == success. Each kept run becomes{bot, state: COMMENTED, time: completed_at}in the same latest-per-bot stream. A failed, neutral or pending run is ignored. If the fetch fails, the gate logs a warning and falls back to counting reviews and comments only, as it does today, so it never errors.scripts/lib/reviewer-sources.tsv: update thecheck_run_namecolumn docs and the graphite row's "Known limitation" note to say the gate now reads the column too. Noadvisory_gatevalue changes.scripts/lib/reviewer-sources.sh: update the doc comment onreviewer_sources_check_run_reporters.scripts/reviewer_report.sh, Reviewer scorecard misses Graphite's check-run reviews (clean passes read as 'No response') and undercounts its PR reviews ~8× #1908) unchanged.Tests to Write (
tests/dev-lead/unit/test_advisory_review_gate.bats)head_sha≠headRefOid) is not countedcheck_run_name=-is unaffected: a matching-name run attributed via an override registry gives no credit to the-botTest Command
bats tests/dev-lead/unit/test_advisory_review_gate.bats tests/test_reviewer_sources.bats tests/reviewer_report.bats+bash .dev-lead/scripts/dev-lead-lint.shdon-petry commentedon Oct 2, 2026
Answer to the clarifying question, measured on the last 40 merged PRs: 22 carried a check run named exactly
Graphite / AI Reviews(matches the registrycheck_run_name), app sluggraphite-app, and every one wascompleted/success. So yes, matching the registry column is enough for the name. Three additions are now acceptance criteria 6-8 in the issue body (missing check run is not clean; pin the app slug; a success check run is a submission, not a finding-clearance). Please proceed.don-petry commentedon Oct 2, 2026
Dev-Lead: Implementation Complete — PR #2028
The implementation for issue #2005 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.
b7530fcb5eedf83b4e501d0c66c618f91c47fab0main: 4 file(s), +169/-9 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 #2005 — Advisory gate cannot count Graphite's c…
Merge branch 'main' into dev-lead/issue-2008-20261001-2246
feat(#1894): answer "why isn't this PR approved?" from the real gate …