diff --git a/.github/workflows/bugbot-gate.yml b/.github/workflows/bugbot-gate.yml index 936771b..dc71784 100644 --- a/.github/workflows/bugbot-gate.yml +++ b/.github/workflows/bugbot-gate.yml @@ -95,9 +95,16 @@ on: wait-seconds: description: >- How long to wait for Bugbot to deliver a terminal verdict on the head. - Default 900 is ~1.4x the slowest of 40 measured runs (635s). + Default 1500: the 900 that preceded it was ~1.4x the slowest of 40 + measured runs (635s), and Bugbot then took 15m22s on + client-runtime#544 (backend#3530) -- the gate gave up at 15m08s and + the red verdict stood over a head Bugbot passed fourteen seconds + later. Re-measured 2026-09-10 over 57 completed Cursor Bugbot runs on + five repos: p50 3.6m, p90 7.2m, p99 9.9m, max 10.3m, plus that 15.4m + outlier; 1500 s is ~1.6x the slowest observed. Five of the 24 gate + failures in the same window were this timeout, not a finding. type: number - default: 900 + default: 1500 poll-seconds: description: "Gap between polls while waiting for Bugbot." type: number @@ -124,10 +131,10 @@ jobs: review: name: review runs-on: ubuntu-latest - # Above the script's own 900s budget on purpose: the script must be the thing + # Above the script's own 1500s budget on purpose: the script must be the thing # that reports a timeout, with the measured latencies in the message. A # runner-level kill produces no explanation at all. - timeout-minutes: 20 + timeout-minutes: 30 steps: # Same shape as code-quality.yml and blocked-gate.yml: the checker lives # here, not in the calling repo, so the reusable fetches it. `.github` is diff --git a/repo-inventory.yml b/repo-inventory.yml index 015f243..b1b7aed 100644 --- a/repo-inventory.yml +++ b/repo-inventory.yml @@ -521,7 +521,7 @@ repos: evaluate_protection), so every baseline context has to be restated here or it stops being asserted on this branch. That is how `quality / action-pins` went missing when the staging baseline was - raised in backend#1976 (Bugbot, #277): the fleet gained a context and + raised in backend#1976 (Bugbot, .github#277): the fleet gained a context and this cell silently kept asserting the old set. required_checks: - gate / gate @@ -1143,7 +1143,7 @@ repos: ".cursor/BUGBOT.md": required docs: - visibility: public + visibility: private # flipped 2026-09-10; Mintlify keeps building from a private repo # Enrolled 2026-08-04 (backend#1420, D1 5-of-5). staging created from main # (256fa3e4); docs#80 backmerged main into develop and added the fr-gate # caller; release-train#30 added the repos.yml entry (that side lands first, @@ -1298,7 +1298,7 @@ repos: develop -> staging hop (backend#1975). MORE, never fewer - an override REPLACES the baseline list, so every baseline context is restated here or it stops being asserted on this branch - (backend#1976, Bugbot on #277). + (backend#1976, Bugbot on .github#277). required_checks: - gate / gate - quality / gitleaks @@ -1313,7 +1313,7 @@ repos: staging -> prod hop (backend#1975). MORE, never fewer - an override REPLACES the baseline list, so every baseline context is restated here or it stops being asserted on this branch (backend#1976, - Bugbot on #277). + Bugbot on .github#277). required_checks: - gate / gate - quality / gitleaks @@ -1389,6 +1389,83 @@ repos: CLAUDE.md: required ".cursor/BUGBOT.md": required + org-config: + # Private engine room seeded 2026-09-10 as a verbatim copy of `.github`'s + # operational logic (inventory, standards, scripts, reusable workflows); the + # seed merged 2026-09-10 (tracebloc/org-config#1), so its callers, copy and + # quality files are REQUIRED like every peer's. Until the fleet's callers + # switch, `.github` stays the source of truth and the live runner of the + # scheduled workflows (org-config's schedules are held, SEED-marked). + visibility: private + release_train: false + protection: + # WIDER than the fleet baseline on develop, the same shape as `.github` + # above and for the same reason: the fleet's guards are going to live here, + # so their selftests are armed as a required check on the branch they + # merge to. An override REPLACES the baseline list, so every baseline + # context is restated deliberately. Applied 2026-09-10, byte-for-byte the + # shape of `.github`'s develop. + develop: + divergent: + reason: >- + Requires MORE than the fleet baseline: the `selftests` job is armed + here on top of the develop baseline, so a red selftest blocks the + merge - the same shape as `.github`, whose operational logic this + repo is seeded from. MORE, never fewer; every baseline context is + restated because an override replaces the list. + required_checks: + - quality / gitleaks + - quality / house-rules + - quality / action-pins + - selftests + staging: + exempt: *no_staging_branch_not_on_train + prod: + divergent: + reason: >- + not on the release train (yet): `main` exists so reusable workflows can + be pinned `@main` once callers switch, and it is protected as a working + branch - one review, the quality checks, no admin lock - the same shape + as `rfcs` and `claude-skills`. Nothing promotes into it, so the train's + review bypass has no premise here and is not granted. When this repo + replaces `.github` on the train, this cell becomes the fleet `prod` + policy and this reason goes. + enforce_admins: false + require_conversation_resolution: false + bypass_reviews: [] # nothing promotes here; see the reason above + required_checks: + - quality / gitleaks + - quality / house-rules + - quality / action-pins + rulesets: + promotion_merge_commit_only: + exempt: "Not on the release train: no staging branch and no promotion PRs, so there are no promotion branches for a merge-method rule to protect." + tag_trust_root: + exempt: "Publishes nothing from a `v*` tag - no tag-triggered workflow and no v* tags cut (repo created 2026-09-10)." + callers: + version-bump-gate.yml: + exempt: *version_gate_not_applicable + advance-deploy-env.yml: required # advance-deploy-env-caller.yml + code-quality.yml: required # code-quality-caller.yml + customer-priority-bump.yml: required # customer-priority-bump-caller.yml + fr-gate.yml: required # fr-gate-caller.yml + stale-backlog.yml: required # stale-backlog-caller.yml + fr-pass-comment.yml: required # fr-pass-comment-caller.yml + kanban-closure-router.yml: required # kanban-closure-caller.yml + set-pr-status.yml: required # set-pr-status-caller.yml + wip-limit-check.yml: + exempt: *wip_limit_check_has_no_callers + blocked-gate.yml: + exempt: *blocked_gate_rollout_pending + post-release-bump.yml: + exempt: *post_release_bump_parked + bugbot-gate.yml: required # bugbot-gate-caller.yml + copies: + add-to-kanban.yml: required + quality_files: + CLAUDE.md: required + ".cursor/BUGBOT.md": required + release-train: visibility: private release_train: false diff --git a/scripts/bugbot-gate.py b/scripts/bugbot-gate.py index 0a8b9e0..bc471ea 100644 --- a/scripts/bugbot-gate.py +++ b/scripts/bugbot-gate.py @@ -313,20 +313,13 @@ # refuses an unresolvable tie. Dropping this filter turns every re-run into a # hard failure. `query_reads_commit_statuses` below refuses a query that goes # back to the rollup, for the same reason the other query self-checks exist. -QUERY = """ -query($owner: String!, $name: String!, $number: Int!) { - repository(owner: $owner, name: $name) { - pullRequest(number: $number) { - number - isDraft - headRefOid - author { __typename login } - commits(last: 1) { - nodes { - commit { - oid - checkSuites(first: 100) { +# THE FIELD LISTS ARE FRAGMENTS SHARED BY THE FIRST READ AND THE FOLLOW-UP +# PAGES (backend#3530). One definition each, so a follow-up page cannot ask for +# a different shape than the page it continues; the first draft of pagination +# would have been a second copy of the query with `after:` bolted on. +SUITES_FIELDS = """ totalCount + pageInfo { hasNextPage endCursor } nodes { app { slug } checkRuns(first: 100, filterBy: {checkType: LATEST}) { @@ -338,13 +331,11 @@ detailsUrl } } - } - } - } - } - } - reviewThreads(first: 100) { + }""" + +THREADS_FIELDS = """ totalCount + pageInfo { hasNextPage endCursor } nodes { isResolved isOutdated @@ -356,13 +347,78 @@ url } } + }""" + +QUERY = """ +query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + number + isDraft + headRefOid + author { __typename login } + commits(last: 1) { + nodes { + commit { + oid + checkSuites(first: 100) {""" + SUITES_FIELDS + """ + } + } } } + reviewThreads(first: 100) {""" + THREADS_FIELDS + """ + } } } } """ +# THE FOLLOW-UP PAGES (backend#3530). A head with more items on a connection +# than one page holds used to be permanently un-gateable: `require_complete` +# saw `totalCount > len(nodes)` and refused, correctly, on every re-run, and +# nothing the author did could change it. client#1017 reached that on the +# rollup with 103 contexts; `backend#3388` carries 70 review threads today. The +# pages are followed until `hasNextPage` is false, and `require_complete` then +# runs over the JOINED list -- the truncation test stays, it just measures the +# whole set. Only the two top-level connections are paged: `checkRuns` sits +# inside a suite and one suite is one workflow's jobs (28 on the busiest head +# measured), and only the producing app's suite is read at all. +FOLLOW_QUERIES = { + "checkSuites": """ +query($owner: String!, $name: String!, $number: Int!, $cursor: String!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + commits(last: 1) { + nodes { + commit { + checkSuites(first: 100, after: $cursor) {""" + SUITES_FIELDS + """ + } + } + } + } + } + } +} +""", + "reviewThreads": """ +query($owner: String!, $name: String!, $number: Int!, $cursor: String!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $cursor) {""" + THREADS_FIELDS + """ + } + } + } +} +""", +} + +#: Where each paged top-level connection sits in the first read's payload, and +#: in its own follow-up page. Derived once here; `fetch` walks it. +PAGED_TOPLEVEL = { + "checkSuites": ("commits", "nodes", 0, "commit", "checkSuites"), + "reviewThreads": ("reviewThreads",), +} + # Derived, not restated -- see the note above the QUERY's page sizes. PAGE_CAP = max([int(n) for n in re.findall(r"first:\s*(\d+)", QUERY)] or [0]) @@ -399,6 +455,22 @@ def connections_missing_totalcount(query=QUERY): return missing +def connections_missing_pageinfo(query=QUERY): + """Which paged TOP-LEVEL connections in `query` fail to request `pageInfo`. + + Without `pageInfo { hasNextPage endCursor }` the follow-up pages in `fetch` + are never asked for, and a head with more than one page of threads or + suites is back to being permanently refused (backend#3530). Same shape as + `connections_missing_totalcount`, read off the query so it cannot drift. + """ + missing = [] + for name in PAGED_TOPLEVEL: + match = re.search(name + r"\(first:\s*\d+[^)]*\)\s*\{(.*?)nodes\s*\{", query, re.S) + if match is None or "hasNextPage" not in match.group(1) or "endCursor" not in match.group(1): + missing.append(name) + return missing + + # THE TWO ACTOR KINDS THIS GATE CAN ACT ON, spelled as GitHub spells them. # `Actor.__typename` also admits `Organization`, `Mannequin` and # `EnterpriseUserAccount`; those are neither, and land in "cannot tell" on @@ -539,19 +611,12 @@ def _run_gh(args, env): ) -def fetch(owner, name, number, env=None, runner=_run_gh): - """Read the PR. Any failure raises rather than returning a partial view.""" - env = dict(os.environ if env is None else env) - proc = runner( - [ - "gh", "api", "graphql", - "-f", "query=" + QUERY, - "-F", "owner=" + owner, - "-F", "name=" + name, - "-F", "number=%d" % number, - ], - env, - ) +def _graphql(query, variables, env, runner): + """One GraphQL read, or Unreadable. Never a partial view.""" + args = ["gh", "api", "graphql", "-f", "query=" + query] + for key, value in variables.items(): + args += ["-F", "%s=%s" % (key, value)] + proc = runner(args, env) if proc.returncode != 0: raise Unreadable( "GraphQL read failed (exit %d): %s" @@ -568,10 +633,74 @@ def fetch(owner, name, number, env=None, runner=_run_gh): except (KeyError, TypeError): raise Unreadable("GraphQL response had no repository.pullRequest") if pr is None: - raise Unreadable("no such pull request: %s/%s#%d" % (owner, name, number)) + raise Unreadable("no such pull request") + return pr + + +def _at(obj, path): + """The connection at `path` inside a payload, or None when the path is absent.""" + cur = obj + for step in path: + if isinstance(step, int): + if not isinstance(cur, list) or len(cur) <= step: + return None + cur = cur[step] + else: + if not isinstance(cur, dict): + return None + cur = cur.get(step) + return cur + + +def follow_pages(pr, variables, env, runner): + """Append every further page of each paged top-level connection onto `pr`. + + Bounded: a connection cannot need more pages than `totalCount` divides + into, so a cursor that keeps saying `hasNextPage` past that is a broken + read and is refused rather than followed for ever. `require_complete` + stays the completeness test -- this only hands it the whole list. + """ + for name, path in PAGED_TOPLEVEL.items(): + conn = _at(pr, path) + nodes = conn.get("nodes") if isinstance(conn, dict) else None + if nodes is None: + continue + page_info = conn.get("pageInfo") or {} + total = conn.get("totalCount") + pages_left = (int(total) // 100) + 1 if isinstance(total, int) else 100 + seen_cursors = set() + while page_info.get("hasNextPage"): + cursor = page_info.get("endCursor") + if not cursor or cursor in seen_cursors or pages_left <= 0: + raise Unreadable( + "%s: the server kept reporting another page (cursor %r, %d " + "node(s) read of %r) -- a follow-up page that never ends is a " + "broken read, not a long list. Refusing to guess." % (name, cursor, len(nodes), total) + ) + seen_cursors.add(cursor) + pages_left -= 1 + page = _at(_graphql(FOLLOW_QUERIES[name], dict(variables, cursor=cursor), env, runner), path) + if not isinstance(page, dict) or page.get("nodes") is None: + raise Unreadable("%s: a follow-up page came back without nodes" % name) + nodes.extend(page["nodes"]) + page_info = page.get("pageInfo") or {} + conn["pageInfo"] = {"hasNextPage": False, "endCursor": page_info.get("endCursor")} return pr +def fetch(owner, name, number, env=None, runner=_run_gh): + """Read the PR, every page of it. Any failure raises rather than returning a partial view.""" + env = dict(os.environ if env is None else env) + variables = {"owner": owner, "name": name, "number": "%d" % number} + try: + pr = _graphql(QUERY, variables, env, runner) + except Unreadable as exc: + if str(exc) == "no such pull request": + raise Unreadable("no such pull request: %s/%s#%d" % (owner, name, number)) + raise + return follow_pages(pr, variables, env, runner) + + def bugbot_check(pr): """The Bugbot check run on the PR's CURRENT head, or None. @@ -959,6 +1088,12 @@ def main(argv=None): # commit status. Green everywhere else, which is what made it read as flaky # infra for three duplicate tickets. A defect in this file, so it fails the # run rather than the author's day. + unpaged = connections_missing_pageinfo() + if unpaged: + _emit(FAIL, [], "the GraphQL query no longer requests pageInfo for: %s. " + "Without it a head with more than one page is refused for " + "ever (backend#3530)." % ", ".join(unpaged)) + return 2 if query_reads_commit_statuses(): _emit(FAIL, [], "the GraphQL query reads the head's checks through " "statusCheckRollup again, whose StatusContext arm needs " diff --git a/scripts/tests/bugbot-gate-mutations.py b/scripts/tests/bugbot-gate-mutations.py index 02dba02..e63827c 100644 --- a/scripts/tests/bugbot-gate-mutations.py +++ b/scripts/tests/bugbot-gate-mutations.py @@ -69,6 +69,19 @@ # (label, old, new) MUTATIONS = [ + # --- (P) PAGINATION (backend#3530): a head with more than one page of + # threads or suites was permanently refused. The pages must be followed, and + # the query must keep asking for the cursor that makes following possible. + ("the follow-up pages are never fetched, so a two-page head is refused again", + ' while page_info.get("hasNextPage"):', + ' while False and page_info.get("hasNextPage"):'), + ("the pageInfo self-check goes blind, so a query that stopped asking passes", + ' if match is None or "hasNextPage" not in match.group(1) or "endCursor" not in match.group(1):', + ' if False and (match is None or "hasNextPage" not in match.group(1) or "endCursor" not in match.group(1)):'), + ("a repeating cursor is followed for ever instead of refused", + ' if not cursor or cursor in seen_cursors or pages_left <= 0:', + ' if not cursor:'), + # --- (A) the load-bearing claim: a TERMINAL verdict on THIS head -------- ("a missing Bugbot verdict reports PASS instead of UNCLAIMED", ' if check is None:\n return UNCLAIMED, [', diff --git a/scripts/tests/bugbot-gate-selftest.py b/scripts/tests/bugbot-gate-selftest.py index f902066..32107d0 100644 --- a/scripts/tests/bugbot-gate-selftest.py +++ b/scripts/tests/bugbot-gate-selftest.py @@ -824,6 +824,103 @@ def runner_of(proc): ) # -------------------------------------------------------------------------- +# -------------------------------------------------------------------------- +# 6c. Pagination (backend#3530). A head with more than one page of threads or +# suites used to be PERMANENTLY refused: `require_complete` compared the +# first page against totalCount, correctly, on every re-run, and nothing the +# author did could change it. The pages are followed now, the truncation +# test runs over the joined list, and a follow-up that never ends is refused +# rather than followed for ever. Driven through `gate.fetch` with a runner +# that answers page by page, so the loop, the variables it sends and the +# join are all exercised -- not a helper the loop might not call. + +def _paged_runner(first, pages): + """A runner that answers the first read, then each follow-up page in turn, + and records every call's argv so the test can assert what was asked.""" + calls = [] + def run(args, env): + calls.append(args) + if "cursor=" not in " ".join(args): + return Proc(out=json.dumps({"data": {"repository": {"pullRequest": first}}})) + if not pages: + return Proc(rc=1, err="a follow-up page was asked for that the test did not script") + return Proc(out=json.dumps({"data": {"repository": {"pullRequest": pages.pop(0)}}})) + run.calls = calls + return run + +def _threads(n, start=0): + return [thread(finding_body("Medium", "finding %d" % i), resolved=True) for i in range(start, start + n)] + +# 150 threads: page one carries 100 and says there is more, page two the last 50. +first = pr(contexts=[check_run()], threads=_threads(100), thread_total=150) +first["reviewThreads"]["pageInfo"] = {"hasNextPage": True, "endCursor": "c1"} +first["commits"]["nodes"][0]["commit"]["checkSuites"]["pageInfo"] = {"hasNextPage": False, "endCursor": None} +page2 = {"reviewThreads": {"totalCount": 150, "pageInfo": {"hasNextPage": False, "endCursor": "c2"}, + "nodes": _threads(50, 100)}} +runner = _paged_runner(first, [page2]) +got = gate.fetch("o", "n", 1, env={}, runner=runner) +check("paging: 150 threads over two pages are joined into one list", + len(got["reviewThreads"]["nodes"]) == 150, "got %d" % len(got["reviewThreads"]["nodes"])) +check("paging: the follow-up page was asked for with the first page's endCursor", + any("cursor=c1" in a for call in runner.calls for a in call), "calls=%r" % [c[-2:] for c in runner.calls]) +check("paging: exactly one follow-up page was fetched for one hasNextPage", + len(runner.calls) == 2, "calls=%d" % len(runner.calls)) +verdict = ev(got, "high") +check("paging: the joined 150-thread head evaluates instead of being refused as truncated", + verdict == gate.PASS, "verdict=%r" % verdict) + +# The truncation test still bites AFTER the join: the server says 150, the pages +# deliver 130 and stop. That is a cut list, and an absence in it is not evidence. +first = pr(contexts=[check_run()], threads=_threads(100), thread_total=150) +first["reviewThreads"]["pageInfo"] = {"hasNextPage": True, "endCursor": "c1"} +short = {"reviewThreads": {"totalCount": 150, "pageInfo": {"hasNextPage": False, "endCursor": "c2"}, + "nodes": _threads(30, 100)}} +got = gate.fetch("o", "n", 1, env={}, runner=_paged_runner(first, [short])) +expect_unreadable("paging: a list still short of totalCount after every page is refused", + lambda: gate.findings(got), because="the page is truncated") + +# A cursor that repeats, or more pages than totalCount can need, is a broken +# read and is refused -- never followed until the job clock kills the run. +first = pr(contexts=[check_run()], threads=_threads(100), thread_total=150) +first["reviewThreads"]["pageInfo"] = {"hasNextPage": True, "endCursor": "c1"} +looping = {"reviewThreads": {"totalCount": 150, "pageInfo": {"hasNextPage": True, "endCursor": "c1"}, + "nodes": _threads(10, 100)}} +expect_unreadable("paging: a follow-up page that repeats its cursor is refused", + lambda: gate.fetch("o", "n", 1, env={}, runner=_paged_runner(first, [looping, dict(looping)])), + because="never ends") + +# The suites connection pages the same way, through its own path in the payload. +first = pr(contexts=[check_run(slug="github-actions", name="unit")], threads=[]) +first["commits"]["nodes"][0]["commit"]["checkSuites"].update( + {"totalCount": 2, "pageInfo": {"hasNextPage": True, "endCursor": "s1"}}) +first["reviewThreads"]["pageInfo"] = {"hasNextPage": False, "endCursor": None} +bugbot_suite = pr(contexts=[check_run()])["commits"]["nodes"][0]["commit"]["checkSuites"]["nodes"] +page2 = {"commits": {"nodes": [{"commit": {"checkSuites": { + "totalCount": 2, "pageInfo": {"hasNextPage": False, "endCursor": "s2"}, "nodes": bugbot_suite}}}]}} +got = gate.fetch("o", "n", 1, env={}, runner=_paged_runner(first, [page2])) +try: + found = gate.bugbot_check(got) +except gate.Unreadable as exc: + # A page never followed leaves the suite list short of totalCount, and + # `bugbot_check` then refuses it. That is a FAILED assertion here, not a + # crash: the harness cannot score a suite that never reported. + found = "refused: %s" % exc +check("paging: a Bugbot suite on the SECOND page of suites is found", + isinstance(found, dict) and found.get("name") == gate.BUGBOT_REVIEW_CHECK_NAME, "found=%r" % (found,)) + +# The query must keep asking for pageInfo, or none of the above ever runs live. +check("the real QUERY asks both top-level connections for pageInfo", + gate.connections_missing_pageinfo() == [], "missing=%r" % gate.connections_missing_pageinfo()) +for name in gate.PAGED_TOPLEVEL: + stripped = re.sub(r"pageInfo \{ hasNextPage endCursor \}\n", "", gate.QUERY, count=1) \ + if name == "checkSuites" else gate.QUERY.replace(" pageInfo { hasNextPage endCursor }\n", "", 1) + # Whichever occurrence the stripper removed, the guard must name AT LEAST one + # connection -- the assertion is that the stripper applied and was seen. + check("the pageInfo stripper actually applied", stripped != gate.QUERY) + check("dropping pageInfo from the query is detected", + gate.connections_missing_pageinfo(stripped) != [], "guard stayed silent for %r" % name) + break + # 7. severity_of, directly. # -------------------------------------------------------------------------- check("severity_of lowercases", gate.severity_of("**High Severity**") == "high")