Skip to content

fix(bugbot-gate): an unclaimed head means two things, and the author says which (backend#2586) - #369

Open
LukasWodka wants to merge 2 commits into
developfrom
fix/2586-bugbot-unclaimed-discriminator
Open

fix(bugbot-gate): an unclaimed head means two things, and the author says which (backend#2586)#369
LukasWodka wants to merge 2 commits into
developfrom
fix/2586-bugbot-unclaimed-discriminator

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

One absence, two opposite meanings

UNCLAIMED says the same thing to every reader today: Bugbot "drops PRs", "no one
here can fix" it. That is true of one class of PR and false of the rest, and the
run cannot tell them apart without reading who opened the PR. So it reads the
author.

The measurement

Three sweeps, kept separate because their windows and repo sets differ — merging
them into one tidy table is how the first commit on this branch overclaimed, and
the second commit fixes it.

A. 2026-08-24..27, 20 repos, all 662 open+closed PRs

PR authorngot a Cursor Bugbot check
human637635 — the 2 exceptions are drafts, which this gate exempts anyway
Bot251

B. 2026-07-01.., 10 repos, Bot-authored only

shapenreviewed
sync PRs (bot author + bot head commit)200
dependabot PRs150 — including 2 whose head commit was human-authored
promotion PRs (bot author + human head commit)33

C. 2026-08-18.., 7 repos — human-authored PRs whose head commit was authored by
tracebloc-release-train[bot]: 43 of 43 reviewed.

The rule that fits all of it

A PR is reviewed when some identity on it has a Cursor seat — the author or
the head commit's author — and dependabot's are never reviewed either way.

Two simpler readings do not survive:

  • "the PR author decides" — B's 3 promotion PRs are Bot-authored and were reviewed;
  • "the head commit's author decides" — C's 43 are bot-committed and were reviewed.

backend#2114 closed COMPLETED saying "no discriminator survives the data".
Identity does, even though no single field does.

What is retracted, and why in place

#356 recorded, as the justification for the tolerance: "6 of 9 never got a
check at all — .github#349 (57 min), #350 (55), #352 (40), #353 (37), #354 (32),
e2e-test-agent#273 (2h+)"
, and "requiring this context while failing on
UNCLAIMED would block roughly two thirds of all PRs"
.

Re-measured per commit, every one of those rows is false:

.github#349 bd84b548 commit 14:29:16Z bugbot done 14:31:20Z success
.github#350 aa90a361 commit 14:30:32Z bugbot done 14:33:59Z success
.github#352 25bb5874 commit 14:46:27Z bugbot done 14:49:37Z success
.github#353 8bf43d8f commit 14:49:09Z bugbot done 14:53:23Z success
e2e#273 1a5d97ea commit 13:20:31Z bugbot done 13:27:02Z success

Every commit those PRs ever had carried a COMPLETED Bugbot run, and this gate's
own bugbot / review context concluded SUCCESS on their heads
— so the gate saw
the reviews it was said to have missed. #349 has exactly one commit, no bugbot run comment, and its Cursor check suite was created 3s after the commit. The
likely cause of the original reading is timing latency from PR creation while the
head had since moved.

Retracted in the file rather than deleted, because that paragraph is the stated
reason the tolerance exists.

What this does NOT change

No exit code moves.UNCLAIMED still exits 0 for every author kind, and the
selftest pins that for all three. Whether the human case should block — now that
the honest estimate of the blast radius is 0 of 635 non-draft rather than "two
thirds" — is a decision for @LukasWodka on backend#2586, deliberately not smuggled
in here.

What changes is the sentence the next reader acts on:

  • Bot author → expected and unfixable from here (0 of 20 sync, 0 of 15
    dependabot), re-running cannot help because Cursor attributes bugbot run to
    the author and answers with a seat refusal, and the remedy is a seated human
    author (backend#2590, fix(2590): the sync opens its PRs as a human, so Bugbot reviews them #348). It also names the measured exception — 3 of 3
    Bot-opened PRs with a human head commit were reviewed — rather than
    overclaiming.
  • Human author → anomalous, not weather: re-run, and report it if it recurs,
    because it would be the first measured instance.
  • Neither → cannot tell, and that is a finding (rule 3). A deleted account
    (author: null) and an unmeasured actor type both land here rather than being
    folded into a neighbour that would then say something confident and wrong.

Fail-closed, and derived rather than restated

author_kind reads GitHub's author.__typename; no decision restates the tables
above (rule 1). And because a guard that reads a field can be disarmed by editing
the query instead of the guard, query_lacks_author_kind refuses the whole run
if the query stops asking for author.__typename — the same shape as the existing
connections_missing_totalcount startup refusal, one field over. Its limits are
written next to it: it checks that someauthor selection asks for
__typename, and does not model nesting.

Test plan

make checkgreen, on both commits (lint + ruff + shellcheck + house-rules

  • action-pins + actionlint + mutations-dry + every selftest).

bugbot-gate-selftest.py109 assertions, all passed (was 82). The new ones
hold the report, because with the exit code unchanged the report is the
behaviour: the three paragraphs, their distinctness, both cross-checks (the bot
paragraph must not reach a human author and vice versa), author_kind in five
input shapes, and both directions of the query guard.

bugbot-gate-mutations.py47 mutations: 0 stale, 0 uncaught, 7 new, each
caught by the assertion named for it:

mutationreddened
every author reads as humana Bot author is a bot, the two kinds are DIFFERENT answers
every author reads as a Bota User author is human, the two kinds are DIFFERENT answers
an unmeasured actor type is filed as humanan unmeasured actor type is 'cannot tell', main: an unreadable author kind says so
the bot paragraph is emitted for every author kindan UNCLAIMED human-authored head reads as anomalous
the human and bot cases collapsean UNCLAIMED human-authored head reads as anomalous
the query stops asking for author.__typenamethe query guard + every UNCLAIMED case (rc becomes 2)
the query guard is disarmeda query that asks only for the login is caught

The mutations edit scripts/bugbot-gate.py itself and re-run the real suite
(rule 9) — no second copy of the rule lives in the harness.

Still open on backend#2586, not fixed here

  1. Should UNCLAIMED block on human-authored PRs? One line, once decided.
  2. Dependabot PRs get no automated review at all — 0 of 15 measured, and
    patch-level ones auto-merge in backend. Cursor-side: a dashboard change or an
    explicit accepted-risk note.
  3. Should bugbot / review be required anywhere? It is required on no branch
    in the org (re-measured today), so it is advice everywhere it is called a gate.

Part of tracebloc/backend#2586 · Part of tracebloc/backend#2284

…says which (backend#2586)
The UNCLAIMED report told every reader the same thing: that Bugbot "drops
PRs" and that "no one here can fix" it. Re-measuring says that is true of
exactly one class -- a PR a Bot authored -- and false of every other, where
an absent review is unprecedented and the reader should re-run.
Measured 2026-08-27, 662 PRs across 20 repos (plus a 2026-07-01 sweep for
the bot rows), anchored on the head COMMIT rather than PR creation:
PR author head commit author n got a Cursor Bugbot check
human human 594 yes (all)
human release-train[bot] 43 yes, 43 of 43
Bot (App) human 3 yes, 3 of 3
Bot (App, sync) release-train[bot] 20 NO, 0 of 20
Bot (dependabot) either 15 NO, 0 of 15
So the discriminator is the PR's AUTHOR, and the head commit's author does
not matter. backend#2114 closed saying "no discriminator survives the
data"; one does.
The measurement the previous comment carried is retracted in place rather
than deleted: .github#349/#350/#352/#353 and e2e-test-agent#273 were cited
as never reviewed, and every one of them carried a COMPLETED Cursor Bugbot
run on every commit it ever had -- this gate's own `bugbot / review`
concluded SUCCESS on their heads. Timing latency from PR creation while the
head had moved is the likely cause of the original reading.
No exit code changes. UNCLAIMED still exits 0 for every author kind, so
whether the HUMAN case should block stays an open decision on backend#2586
rather than being smuggled in here.
- author_kind derives the branch from GitHub's answer; an unmeasured actor
type and a deleted account are "cannot tell", never folded into either
neighbour
- query_lacks_author_kind refuses the run if the query stops asking for
author.__typename, so the discriminator cannot go inert while green
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
…nd#2586)
The first commit said the discriminator is the PR's author and that the head
commit's author does not matter. Its own table contradicts that: 3 of 3
Bot-opened release-train promotion PRs, whose head COMMIT was human-authored,
WERE reviewed.
Neither single field decides. What fits all three sweeps -- and no simpler
rule does -- is that a PR is reviewed when SOME identity on it has a Cursor
seat, author or head-commit author, with dependabot's never reviewed either
way (0 of 15, including 2 with human head commits).
So the three sweeps are now recorded separately, with their own windows and
repo sets, instead of merged into one tidy table; the human count is
corrected to 635 of 635 non-draft (637 human-authored, 2 of them drafts);
and the Bot paragraph names the measured exception instead of asserting that
no Bot-opened PR is ever reviewed.
The branch still keys on the author, which is the field that identifies the
class with no remedy -- but it now says what was measured rather than
rounding it off.
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

set-status / closing-ref is red on this PR, and it cannot be made green today. Measured, not assumed.

This PR names backend#2586 in its title and deliberately does not close it — two of the three items on that ticket are decisions, not code. CLAUDE.md and org-standards.md both say that is exactly what Part of <owner>/<repo>#N is for:

"a number in a PR title … is read by closing-ref as a reference the body must make good, in one of two forms: Closes <owner>/<repo>#N … or Part of <owner>/<repo>#N when it does not. Both satisfy the check"

The body carries Part of tracebloc/backend#2586, and the check still fails with "Add Closes tracebloc/backend#2586 to the body" — it does not know the second form:

$ git show origin/main:scripts/closing-ref-gate.py | grep -c "Part of" -> 0
$ git show origin/develop:scripts/closing-ref-gate.py | grep -c "Part of" -> 5

Part of landed on develop today in #360 (backend#2556). Every caller pins the reusable at @main (RFC-BACKEND-1405 Q3), so the gate that ran here is main's copy, which predates it. The canon documents a form the running machine refuses — the rule-7 shape, one promotion behind.

So the options on this PR were: write Closes (false), strip the ticket from the title (the canon says keeping it is right, and it is the traceability this ticket's own history needed), or leave the check red and say why. I took the third.

It is advisory, re-measured just nowdevelop's required contexts in this repo are actionlint, gate, quality/gitleaks, quality/house-rules, quality/ruff, quality/shellcheck, quality/action-pins, selftests, and the only ruleset targets promotion branches. Everything required is green.

It clears itself when #360 reaches main on the normal develop → staging → main promotion — #365 and #363 are the open hops. Re-running this check after that will turn it green with no change here. Worth knowing that until then, every PR in this org that references a ticket without closing it fails this check, which is a fleet-wide consequence rather than a quirk of this branch.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

The red set-status / closing-ref here is a known version skew, not a defect in this PR — and it must not be "fixed" by adding a Closes.

Measured just now:

org-standards.md "Part of" occurrences
main 0 <- callers pin @main, so this is the copy the gate uses
staging 1
develop 1

The gate parses its non-closing vocabulary out of org-standards.md rather than holding a list — that is the derive-never-restate design from #360. Part of was added to the canon in .github#360, which is merged to develop and, via .github#365 at 19:39Z, now on staging. The last hop, .github#363 (staging -> main), is still open, so the copy on main still declares no non-closing form. Every PR that references a ticket without closing it therefore fails this check, fleet-wide.

This PR uses Part of because it genuinely does not close its ticket. Writing Closes to turn the check green would be a false claim about what the PR does — precisely the defect backend#2616 was filed against, and precisely the workaround .github#354 wrote into the canon before #360 reverted it.

set-status / closing-ref is required on no branch in this repo (re-measured), so it blocks nothing. It clears by itself the moment #363 merges.

No action needed on this branch.

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 517913b4. The code is approve-quality and I verified it rather than trusting the writeup: it reworks the org-wide bugbot-gate.py so the UNCLAIMED report retracts the earlier (measured-false) "Bugbot drops PRs" claim and instead branches three ways on author identity — bot / human / cannot-tell — via a new author_kind(), adds author { __typename login } to the query, and adds query_lacks_author_kind() as a self-guard (exit 2 if the discriminator field ever disappears), mirroring the existing totalCount guard. The exit code for UNCLAIMED is deliberately left at 0, so no consumer's block/no-block behaviour changes. Tests hold the bar — selftest 109 assertions, mutation suite 47/47 caught with 0 stale, fixtures independent of the module constants, and the report-prose is the behaviour here since the exit code is constant. No findings.

Not approving this pass only because CI is red: set-status / closing-ref fails. The diff doesn't touch the closing-ref gate — the body ends with Part of tracebloc/backend#2586 · Part of tracebloc/backend#2284, and Part of is a MENTIONED (non-closing) keyword, so the gate reddens by design. This PR is genuinely partial, so you can't honestly add Closes — it's the gate's known partial-PR tension (closing-ref-gate.py:45-48), a policy call rather than anything to fix in this diff. Get that check resolved (a legitimately-closed ref if one applies, or the documented exemption path) and I'll approve — the code itself is ready.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@saqlainsyed007set-status / closing-ref is green now, on the same head you reviewed (517913b4), so nothing about the diff you read has changed.

It was stale rather than a policy tension. The gate reads its keyword vocabulary from org-standards.md on main, and Part of was not in main's copy yet — .github#363 (the staging -> main promotion carrying it) merged at 05:05 UTC today. The check re-ran at 05:33 and passed:

set-status / closing-ref completed/success started=2026-08-28T05:33:44Z
set-status / closing-ref completed/failure started=2026-08-27T19:42:45Z <- pre-#363
set-status / closing-ref completed/failure started=2026-08-27T19:42:39Z <- pre-#363
set-status / closing-ref completed/failure started=2026-08-27T19:40:53Z <- pre-#363

(all four on 517913b4.) Worth knowing for the next PR that hits this: the red is clearable without touching the body — no Closes needed on a genuinely partial PR, and no exemption. gh pr checks already showed it passing; the PR-level rollup keeps every historical run, so it still reads red there. That discrepancy is what makes this one look like a standing policy problem when it is a promotion-lag artifact.

Same thing cleared e2e-test-agent#305 at the same time — it now has no failing check-runs on its head either.

No action needed from me on the diff; over to you for the approval you'd conditioned on this.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@saqlainsyed007 — the closing-ref condition on your review is now satisfied on this same head. Measured rather than re-run-and-hoped:

gh api repos/tracebloc/.github/commits/517913b4/check-runs, filtered to the gate:

run idconclusioncompleted
98764353373success2026-08-28T05:33:52Z
98649127784failure2026-08-27T19:42:54Z
98649100533failure2026-08-27T19:42:49Z
98648595146failure2026-08-27T19:41:01Z

The passing run's own output is **Closing-ref gate: pass**, and it reports the branch you predicted:

Title names: backend#2586
closingIssuesReferences: none
Body references (`Part of`): tracebloc/backend#2586, tracebloc/backend#2284
Every ticket the title names is referenced:
backend#2586 -- declared body reference (does not close it)

So the partial-PR tension you named is genuinely resolved rather than worked around — no false Closes, no number dropped from the title. What changed is that #360 ("let a child PR be truthful", backend#2556) landed on main at 2026-08-27T15:54Z, and the callers pin .github@main, so the MENTIONED form this PR was already using became admissible.

Two caveats so the green reads honestly:

  1. The rollup still shows FAILURE, which is what you saw at 04:12 — GitHub aggregates every check-run sharing a name, and the three stale failures predate the fix by hours. The rollup is not evidence about the current head; run 98764353373 is.
  2. closing-ref is not a required context on .github/developbranches/develop/protection lists actionlint, gate, quality/{gitleaks,house-rules,ruff,shellcheck,action-pins}, selftests. So it was never the thing blocking the merge; mergeStateStatus=BLOCKED is REVIEW_REQUIRED. Mentioning it only so the record is accurate — not to argue the check should be ignored.

No code changed since your review, so nothing to re-read.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@saqlainsyed007 — the set-status / closing-ref red you held on has cleared, on the same sha you reviewed (517913b4). No code changed; the check result did.

Your read of the mechanism was right, and it turned out to be already fixed rather than a policy call. Part of is a MENTIONED keyword, and closing-ref-gate.py gained MENTIONED support under backend#2616 — but the callers pin @main, so the fix only took effect on this PR once it had been promoted all the way up. I traced it hop by hop earlier today: MENTIONED was at develop=6 / staging=0 / main=0, .github#365 carried it to staging, and the promotion carried it to main. It now reads 6/6/6 on all three, and the callers resolve against main:

develop: MENTIONED occurrences = 6
staging: MENTIONED occurrences = 6
main: MENTIONED occurrences = 6

The three stale runs on this head were re-queued after that landed. All four closing-ref runs on 517913b4 are now green:

set-status / closing-ref success 05:39:46 id=98765279398
set-status / closing-ref success 05:39:46 id=98765277989
set-status / closing-ref success 05:39:46 id=98765273548
set-status / closing-ref success 05:33:44 id=98764353373

So no Closes had to be invented for a genuinely partial PR, and no exemption was needed — the gate accepts the honest Part of on its own now. Every check on the head is green (quality / format is SKIPPED). Re-requesting review; the diff is untouched since your pass.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saqlainsyed007