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.

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.

1 participant

@LukasWodka