From 75efb018ca3748fc897108f5ac578a472b8c1a0a Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 18:52:59 -0700 Subject: [PATCH 1/9] Read app-pinned required checks from the head's check runs (#176) A required check that a ruleset (integration_id) or classic protection (app_id) pins to a GitHub App was matched through check.app.databaseId in gh's statusCheckRollup. gh 2.102.0 never asks GitHub for a run's app, so the check always read as missing, even when it passed. When any required check is pinned to an app, the inspection now also reads the check runs of the exact head the PR read returned, through gh api graphql: each check suite with its app's databaseId and the suite's latest run of each name. A pinned check matches only runs of its name from a suite of its app; a commit status, or a run whose suite GitHub names no app for, never matches it. Checks any app may report are still read from the rollup alone. The read fails closed and keeps its reason: a gh failure, errors or malformed fields, another commit, a suite with more than 100 runs, more than 1,000 suites, or suites that change between pages fail the inspection; a deadline keeps the inspection's timeout. The test fakes no longer give rollup entries an app, and answer the new read in the shape gh 2.102.0 prints. The vitejs/vite and sveltejs/svelte recordings were made again at the same heads with the new read: vite's three checks pinned to GitHub Actions now read as passing, and svelte's pinned check reads as missing because GitHub names no app for its suite. Co-Authored-By: Claude Opus 5.5 --- docs/designs/codeboost-plan-indexed-review.md | 2 +- docs/implementation/guarded-merge.md | 4 +- docs/implementation/merge-handoff.md | 2 +- github/merge.ts | 78 +++++++- scripts/record-merge-gh.ts | 3 +- test/fixtures/gh/nonadmin-mixed.json | 21 ++- test/fixtures/gh/nonadmin-ruleset.json | 19 +- test/merge-recorded.test.ts | 23 ++- test/merge.test.ts | 173 ++++++++++++++++-- 9 files changed, 289 insertions(+), 36 deletions(-) diff --git a/docs/designs/codeboost-plan-indexed-review.md b/docs/designs/codeboost-plan-indexed-review.md index 566e9505..cabcbc4b 100644 --- a/docs/designs/codeboost-plan-indexed-review.md +++ b/docs/designs/codeboost-plan-indexed-review.md @@ -2043,7 +2043,7 @@ These `T` IDs are requirement identifiers, not the build-order numbers. Build st - Surfaced by: R6 (D7: A) - Files: github/checks - Verify: recorded-output tests for zero checks, pending then passing, and timeout - - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). + - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). *Later (#176):* for a check pinned to an app, the inspection now reads the head's check runs with their apps through `gh api graphql`, since gh's rollup names no app; the vite and svelte recordings were made again with that read, and vite's pinned checks read as passing (`test/merge-recorded.test.ts`; `test/merge.test.ts`, "reads a required check pinned to an app from that app's run on the head, which %s"). The test named above was replaced. - [x] **T13 (P2, human: ~1 hour / CC: ~5 min)** — runner — Use `node:sqlite` only; enforce the minimum Node version; add a CI warning check - Surfaced by: R7 (D8: A) - Files: runner/store, web/cli diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index c0eb8045..a71afef1 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -27,7 +27,7 @@ The target is resolved again for each validation pass. Both passes, and the thir Merging blocks when any plan item is unreviewed or stale, an attributed file is outside that item's declared plan scope, an Ambiguous or Unplanned segment remains, a current change request is open, or a `cmd:` acceptance check lacks a passing result for the current head. Approval does not override an out-of-scope file; the plan must be amended. Historical change requests from an earlier revision or snapshot remain visible but do not block the current revision. -The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. `test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. +The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. A check that a ruleset (`integration_id`) or classic protection (`app_id`) pins to an app needs to know which app reported each check run, and gh's `statusCheckRollup` does not give it: gh 2.102.0 does not ask GitHub for a run's app (#176). So when any required check is pinned to an app, the adapter also reads the check runs of the exact head that the PR read returned, through `gh api graphql`: each check suite on that commit with its app's `databaseId`, and the suite's latest run of each name with its status, conclusion and times. Only those fields are asked for, which keeps the answer small (GitHub's REST check-run list would also return each run's output text). A pinned check matches only runs of its name from a suite of its app. A commit status has no app, so it never meets a pinned check, and neither does a run whose suite GitHub names no app for (sveltejs/svelte showed one to a token without admin rights): those read as missing. Checks any app may report are still read from the rollup alone. The read fails closed: a gh failure, an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs, more than 1,000 suites (10 pages) or suites that change between pages fails the inspection with that reason, which the merge status shows as “Could not read GitHub merge state.” followed by it; a deadline that stops the read keeps the inspection's timeout as the reason. GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. `test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The vite and svelte recordings were made again at the same heads on 2026-10-21 (UTC) to add the check-run read: vite's three checks pinned to GitHub Actions read as passing, and svelte's pinned check reads as missing, because GitHub names no app for the suite of its only run. The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge call aborted at its 14-second deadline settles by 14.4 s. *Later (#48, #170):* a click can also be waiting on its issue-trust read, whose stop wait is 0.75 s, or on a review load, whose history worker gets 1 s after the abort before it is terminated; so it settles about 0.75 s after its deadline, or about 1 s or a little more after it when a review load was running, past the 14.5-second shutdown drain (traced, not tested); see [merge-queue.md](merge-queue.md). Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected. @@ -39,4 +39,4 @@ This slice does not mutate Git history or run plan commands. If the base moved, ## Validation -Unit and integration tests cover local review blockers; current and stale base/head pairs; PR state and mergeability; known, unknown, empty, pending, and failing check requirements; app-bound and malformed app identities; server base protection; merge-queue refusal for adapters without queue inspection; runtime merge-method validation; the already-fixed check run through `GhAlreadyFixedGateway` (closing-only cross-references, the non-default-base rule, manual links, the closed state, base-branch commits, this PR excluded only while open, the merge inputs, fail-closed results and caller cancellation); a requirement changing between validation passes; deadline cancellation; post-action cache invalidation; and the exact `gh` merge argv. Browser coverage proves blockers are visible, the ready action uses the reviewed head, and duplicate clicks start only one merge. +Unit and integration tests cover local review blockers; current and stale base/head pairs; PR state and mergeability; known, unknown, empty, pending, and failing check requirements; app-bound and malformed app identities; app-pinned checks matched from the head's check runs (passed, pending, failed, reported by another app or by no named app) and each way that read can fail; server base protection; merge-queue refusal for adapters without queue inspection; runtime merge-method validation; the already-fixed check run through `GhAlreadyFixedGateway` (closing-only cross-references, the non-default-base rule, manual links, the closed state, base-branch commits, this PR excluded only while open, the merge inputs, fail-closed results and caller cancellation); a requirement changing between validation passes; deadline cancellation; post-action cache invalidation; and the exact `gh` merge argv. Browser coverage proves blockers are visible, the ready action uses the reviewed head, and duplicate clicks start only one merge. diff --git a/docs/implementation/merge-handoff.md b/docs/implementation/merge-handoff.md index 66ddf5e4..1814d853 100644 --- a/docs/implementation/merge-handoff.md +++ b/docs/implementation/merge-handoff.md @@ -35,7 +35,7 @@ Without a runner, **Merge PR** works as before. | Wait for checks | The job inspects the pull request (fresh) at once, then again after 10 s, with the wait doubling up to 60 s. | every required check succeeds, or the required set was read and is empty, and nothing else blocks | a required check fails, or 30 minutes pass (job: `blocked`, if the preparation still applies: the block re-checks readiness in its transaction, and a review edit or other task change meanwhile ends the job `failed` instead); the pull request moved, another blocker appears, or GitHub or the local review cannot be read (job: `failed`; one failed read fails closed rather than retrying; each inspection also loads the review, which awaits a history worker since #170, within the same 30-minute limit) | | Merge | The job calls the guarded merge with the current review token, not the click's: a rebase during preparation makes a new snapshot and so a new token. A review change after preparation is refused through readiness, which binds the review version. Its inspections rerun the already-fixed check. | GitHub merged or queued the head, as the job's own merge attempt records it when the job reads it after the guarded merge (job: `merged` or `queued`; if a merge-queue poll ended the attempt removed or failed while GitHub was asked, the guarded merge cannot save it queued and rejects, and the job ends as for a refusal; `submitted` when GitHub accepted the merge but saving its outcome failed, so the attempt is still submitting and reconciles from GitHub) | GitHub refused (job: `blocked`; `failed` when GitHub refused because the head changed, which needs a fresh review, or when the review, snapshot or plan revision changed while GitHub was asked: the block is for the review the merge attempt was admitted for. A review edit (a review note) may land while the attempt is in flight. The snapshot term is defensive for a merge job: jobs need a runner, and a runner server's review load reads the recorded head, so no HEAD observation records a new snapshot. A plan revision lands only after a merge-queue poll ended the attempt while GitHub was asked, as plan writes are refused while a merge attempt is in flight), validation refused (job: `failed`), or the outcome is unknown (job: `submitted`; the merge attempt reconciles as before) | -A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. +A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. gh's rollup does not say which app reported a run (#176), so for such a check the inspection reads the head's check runs with their apps from GitHub (`guarded-merge.md`); a run whose app GitHub does not name, and any commit status, never match it. If that read fails, the inspection fails, and the job ends `failed` with GitHub's error. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. A push to the base branch during the wait is such a blocker: the next inspection sees the moved base, and the job ends `failed` with "The base branch moved; merging rebases onto it and prepares again, so the result may need review." The task stays in review. The person clicks **Merge PR** again: the new job rebases onto the new base and prepares again. If the rebase makes an approval stale, that job ends `review-required`. diff --git a/github/merge.ts b/github/merge.ts index 512796c9..128855d4 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -215,6 +215,9 @@ function classicFromBranch(required: unknown): { enforced: boolean; strict: fals if (!enforced && (value.contexts.length || value.checks.length)) return null; return { enforced, strict: false, contexts: value.contexts, checks: value.checks }; } +/** Every value GitHub's GraphQL API documents for a check run's `status` and `conclusion` (CheckStatusState, CheckConclusionState). */ +const CHECK_RUN_STATUSES = ['REQUESTED','QUEUED','IN_PROGRESS','COMPLETED','WAITING','PENDING']; +const CHECK_RUN_CONCLUSIONS = ['ACTION_REQUIRED','CANCELLED','FAILURE','NEUTRAL','SKIPPED','STALE','STARTUP_FAILURE','SUCCESS','TIMED_OUT']; /** Among runs with the same start, the one that passes least counts, so a tie never reads as passing. */ const checkStateRank = { failure: 0, pending: 1, success: 2 } as const; @@ -320,7 +323,7 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { const alreadyFixed = this.#alreadyFixed(target, pr.baseRefName, base, deadlineAt, reads) .catch((error: unknown) => error instanceof Error ? error : new Error(String(error))); const [ruleRead, listRead] = await Promise.allSettled([ - this.#rules(pr.baseRefName, pr.statusCheckRollup as Array>, reads).catch(stopOthers), + this.#rules(pr.baseRefName, head, pr.statusCheckRollup as Array>, reads).catch(stopOthers), published ? this.#branchOpen(target.headBranch!, reads).catch(stopOthers) : undefined, ]); const fixed = await alreadyFixed; @@ -358,8 +361,65 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { }); } - /** The effective branch rules and the state of each required check on the PR. */ - async #rules(baseRefName: string, observed: Array>, signal?: AbortSignal): Promise { + /** + * Every check run on commit `head`, with the ID of the app that reported it, in the shape of gh's statusCheckRollup + * entries plus `appId` (null for a suite GitHub names no app for). gh's statusCheckRollup does not ask for a run's app + * (#176), so a required check that a rule pins to an app is matched from this read. Only the fields the gateway reads + * are asked for, which keeps the answer small: a run's output text, which the REST API returns, is not. Each suite's + * latest run of each name is read, as GitHub counts for required checks. The read fails closed: on a gh failure (its error is kept), on errors or a + * malformed field in GitHub's answer, on a suite with more than 100 runs, on more than 1,000 suites, and when the + * suites change between pages. + */ + async #checkRuns(head: string, signal?: AbortSignal): Promise & { appId: number | null }>> { + const [owner, name] = this.config.repository.split('/') as [string, string]; + const query = 'query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}'; + const runs: Array & { appId: number | null }> = []; + let cursor: string | null = null, suites = 0, total: number | null = null; + for (let page = 1; ; page++) { + const args = ['api','graphql','-f',`query=${query}`,'-f',`owner=${owner}`,'-f',`name=${name}`,'-f',`oid=${head}`]; + if (cursor !== null) args.push('-f', `after=${cursor}`); + const response = await this.#json(args, signal) as { data?: { repository?: { object?: Record | null } | null }; errors?: unknown } | null; + signal?.throwIfAborted(); + if (!response || typeof response !== 'object' || (Object.hasOwn(response, 'errors') && (!Array.isArray(response.errors) || response.errors.length > 0))) + throw new Error('GitHub returned check runs with errors.'); + const commit = response.data?.repository?.object; + if (!commit || commit.__typename !== 'Commit' || commit.oid !== head) throw new Error('GitHub returned no check runs for the pull request head.'); + const connection = commit.checkSuites as { totalCount?: unknown; pageInfo?: { hasNextPage?: unknown; endCursor?: unknown } | null; nodes?: unknown } | null | undefined; + if (!connection || typeof connection !== 'object' || !Number.isSafeInteger(connection.totalCount) || (connection.totalCount as number) < 0 || !Array.isArray(connection.nodes) + || !connection.pageInfo || typeof connection.pageInfo.hasNextPage !== 'boolean' || (connection.pageInfo.endCursor !== null && typeof connection.pageInfo.endCursor !== 'string')) + throw new Error('GitHub returned incomplete check runs.'); + if (total !== null && connection.totalCount !== total) throw new Error('The check suites on the pull request head changed while they were read.'); + total = connection.totalCount as number; + for (const suite of connection.nodes) { + const value = suite as { app?: { databaseId?: unknown } | null; checkRuns?: { totalCount?: unknown; nodes?: unknown } | null } | null; + if (!value || typeof value !== 'object' || !Object.hasOwn(value, 'app') || (value.app !== null && (typeof value.app !== 'object' || !validNumber(value.app.databaseId)))) + throw new Error('GitHub returned incomplete check runs.'); + const appId = value.app === null ? null : value.app!.databaseId as number; + const checkRuns = value.checkRuns; + if (!checkRuns || typeof checkRuns !== 'object' || !Number.isSafeInteger(checkRuns.totalCount) || !Array.isArray(checkRuns.nodes)) throw new Error('GitHub returned incomplete check runs.'); + // One page of runs per suite is read: a suite with more runs than its page holds could hide the required one. + if (checkRuns.nodes.length !== checkRuns.totalCount) throw new Error('A check suite on the pull request head has more check runs than codeboost can read.'); + for (const run of checkRuns.nodes) { + const check = run as Record | null; + if (!check || typeof check !== 'object' || typeof check.name !== 'string' || !CHECK_RUN_STATUSES.includes(check.status as string) + || (check.conclusion !== null && !CHECK_RUN_CONCLUSIONS.includes(check.conclusion as string)) + || (check.startedAt !== null && typeof check.startedAt !== 'string') || (check.completedAt !== null && typeof check.completedAt !== 'string')) + throw new Error('GitHub returned a malformed check run.'); + runs.push({ name: check.name, status: check.status, conclusion: check.conclusion, startedAt: check.startedAt, completedAt: check.completedAt, appId }); + } + } + suites += connection.nodes.length; + if (!connection.pageInfo.hasNextPage) break; + if (page === 10 || typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) + throw new Error('The pull request head has more check suites than codeboost can read.'); + cursor = connection.pageInfo.endCursor; + } + if (suites !== total) throw new Error('The check suites on the pull request head changed while they were read.'); + return runs; + } + + /** The effective branch rules and the state of each required check on the PR, whose head is `head`. */ + async #rules(baseRefName: string, head: string, observed: Array>, signal?: AbortSignal): Promise { const branch = encodeURIComponent(baseRefName); let rulesKnown = true; let rules: unknown[] = []; @@ -445,12 +505,14 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { } } } + // A check any app may report is matched from gh's rollup, check runs and commit statuses alike. One pinned to an app + // is matched only from the check runs that app reported on the head (#176): the rollup names no app, and a commit + // status has none, so a status never meets a pinned check. A failed read fails the inspection with its error. + const pinned = [...requirements.values()].some(required => required.appId !== null) ? await this.#checkRuns(head, signal) : []; const requiredChecks = [...requirements.values()].map(required => { - const matches = observed.filter(check => { - const context = typeof check.name === 'string' ? check.name : check.context; - const app = check.app && typeof check.app === 'object' ? (check.app as { databaseId?: unknown }).databaseId : null; - return context === required.context && (required.appId === null || app === required.appId); - }); + const matches = required.appId === null + ? observed.filter(check => (typeof check.name === 'string' ? check.name : check.context) === required.context) + : pinned.filter(run => run.name === required.context && run.appId === required.appId); if (!matches.length) return { ...required, state: 'missing' as const }; // A re-run adds a newer run (or status) with the same name; GitHub counts the latest one, so an older failure // does not outrank a newer pending or passing run. A check run and a commit status with the same name are not diff --git a/scripts/record-merge-gh.ts b/scripts/record-merge-gh.ts index 6405573b..c2a14167 100644 --- a/scripts/record-merge-gh.ts +++ b/scripts/record-merge-gh.ts @@ -3,7 +3,8 @@ // node scripts/record-merge-gh.ts [owner/repo] // // The production GhMergeGateway runs one fresh inspection of the pull request through the real `gh`, signed in as you. -// Every `gh` call it makes (the pull request, the ruleset, the branch and its classic protection) is saved with its exit +// Every `gh` call it makes (the pull request, the ruleset, the branch and its classic protection, and, when a required +// check is pinned to an app, the head's check runs with their apps) is saved with its exit // status, stdout and stderr, as runWithInput reports them (trimmed on a failure), to test/fixtures/gh/.json. The // already-fixed check is replaced by a stub, so its reads are not recorded. A branch read keeps only `name`, // `protected` and `protection` (the gateway reads only the last two), dropping the commit and its authors' emails. Use a public diff --git a/test/fixtures/gh/nonadmin-mixed.json b/test/fixtures/gh/nonadmin-mixed.json index 1f5580e0..21a11d11 100644 --- a/test/fixtures/gh/nonadmin-mixed.json +++ b/test/fixtures/gh/nonadmin-mixed.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "sveltejs/svelte", "pullRequest": 19050, - "recordedAt": "2026-10-20T20:50:33.643Z", + "recordedAt": "2026-10-21T00:45:50.695Z", "calls": [ { "args": [ @@ -15,6 +15,23 @@ "stdout": "[[{\"type\":\"creation\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155284},{\"type\":\"update\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155284},{\"type\":\"deletion\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155284},{\"type\":\"deletion\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155282},{\"type\":\"non_fast_forward\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155282},{\"type\":\"pull_request\",\"parameters\":{\"required_approving_review_count\":1,\"dismiss_stale_reviews_on_push\":false,\"required_reviewers\":[],\"require_code_owner_review\":false,\"dismissal_restriction\":{\"enabled\":false,\"allowed_actors\":[]},\"require_last_push_approval\":false,\"required_review_thread_resolution\":false,\"require_extra_approval_for_unattributed_changes\":true,\"allowed_merge_methods\":[\"merge\",\"squash\",\"rebase\"]},\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155282},{\"type\":\"required_status_checks\",\"parameters\":{\"strict_required_status_checks_policy\":false,\"do_not_enforce_on_create\":false,\"required_status_checks\":[{\"context\":\"vite-ecosystem-ci\",\"integration_id\":2935623}]},\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"sveltejs/svelte\",\"ruleset_id\":20155282}]]", "stderr": "" }, + { + "args": [ + "api", + "graphql", + "-f", + "query=query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "-f", + "owner=sveltejs", + "-f", + "name=svelte", + "-f", + "oid=111cc6411c9c647f691a159862da2b74212791e3" + ], + "exitCode": 0, + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"111cc6411c9c647f691a159862da2b74212791e3\",\"checkSuites\":{\"totalCount\":4,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGN-eVHo=\"},\"nodes\":[{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":null,\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"vite-ecosystem-ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"completedAt\":\"2026-10-20T12:41:24Z\"}]}},{\"app\":{\"databaseId\":4226557},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Vercel Internal Security Review\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-20T12:41:25Z\",\"completedAt\":\"2026-10-20T12:41:49Z\"}]}},{\"app\":{\"databaseId\":8329},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Vercel Agent Review\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-20T12:41:26Z\",\"completedAt\":\"2026-10-20T12:42:40Z\"}]}}]}}}}}", + "stderr": "" + }, { "args": [ "api", @@ -44,7 +61,7 @@ "number,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,state,mergeable,statusCheckRollup,url" ], "exitCode": 0, - "stdout": "{\"baseRefName\":\"main\",\"baseRefOid\":\"40bb08482f4857d91221638dec544a532bc35389\",\"headRefName\":\"fix/print-keep-nbsp\",\"headRefOid\":\"111cc6411c9c647f691a159862da2b74212791e3\",\"isDraft\":false,\"mergeable\":\"UNKNOWN\",\"number\":19050,\"state\":\"OPEN\",\"statusCheckRollup\":[{\"__typename\":\"StatusContext\",\"context\":\"svelte.dev / preview deployments\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"state\":\"PENDING\",\"targetUrl\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:41:24Z\",\"conclusion\":\"SKIPPED\",\"detailsUrl\":\"https://github.com/sveltejs\",\"name\":\"vite-ecosystem-ci\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:42:40Z\",\"conclusion\":\"SUCCESS\",\"detailsUrl\":\"https://vercel.com/github\",\"name\":\"Vercel Agent Review\",\"startedAt\":\"2026-10-20T12:41:26Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:41:49Z\",\"conclusion\":\"SUCCESS\",\"detailsUrl\":\"https://vercel.com\",\"name\":\"Vercel Internal Security Review\",\"startedAt\":\"2026-10-20T12:41:25Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"}],\"url\":\"https://github.com/sveltejs/svelte/pull/19050\"}\n", + "stdout": "{\"baseRefName\":\"main\",\"baseRefOid\":\"40bb08482f4857d91221638dec544a532bc35389\",\"headRefName\":\"fix/print-keep-nbsp\",\"headRefOid\":\"111cc6411c9c647f691a159862da2b74212791e3\",\"isDraft\":false,\"mergeable\":\"MERGEABLE\",\"number\":19050,\"state\":\"OPEN\",\"statusCheckRollup\":[{\"__typename\":\"StatusContext\",\"context\":\"svelte.dev / preview deployments\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"state\":\"PENDING\",\"targetUrl\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:41:24Z\",\"conclusion\":\"SKIPPED\",\"detailsUrl\":\"https://github.com/sveltejs\",\"name\":\"vite-ecosystem-ci\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:42:40Z\",\"conclusion\":\"SUCCESS\",\"detailsUrl\":\"https://vercel.com/github\",\"name\":\"Vercel Agent Review\",\"startedAt\":\"2026-10-20T12:41:26Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"},{\"__typename\":\"CheckRun\",\"completedAt\":\"2026-10-20T12:41:49Z\",\"conclusion\":\"SUCCESS\",\"detailsUrl\":\"https://vercel.com\",\"name\":\"Vercel Internal Security Review\",\"startedAt\":\"2026-10-20T12:41:25Z\",\"status\":\"COMPLETED\",\"workflowName\":\"\"}],\"url\":\"https://github.com/sveltejs/svelte/pull/19050\"}\n", "stderr": "" } ] diff --git a/test/fixtures/gh/nonadmin-ruleset.json b/test/fixtures/gh/nonadmin-ruleset.json index 55afab96..13bd4c25 100644 --- a/test/fixtures/gh/nonadmin-ruleset.json +++ b/test/fixtures/gh/nonadmin-ruleset.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "vitejs/vite", "pullRequest": 23837, - "recordedAt": "2026-10-20T20:50:30.205Z", + "recordedAt": "2026-10-21T00:45:48.202Z", "calls": [ { "args": [ @@ -15,6 +15,23 @@ "stdout": "[[{\"type\":\"deletion\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"vitejs/vite\",\"ruleset_id\":24556645},{\"type\":\"non_fast_forward\",\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"vitejs/vite\",\"ruleset_id\":24556645},{\"type\":\"pull_request\",\"parameters\":{\"required_approving_review_count\":1,\"dismiss_stale_reviews_on_push\":true,\"required_reviewers\":[],\"require_code_owner_review\":false,\"dismissal_restriction\":{\"enabled\":false,\"allowed_actors\":[]},\"require_last_push_approval\":false,\"required_review_thread_resolution\":false,\"require_extra_approval_for_unattributed_changes\":true,\"allowed_merge_methods\":[\"squash\",\"rebase\"]},\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"vitejs/vite\",\"ruleset_id\":24556645},{\"type\":\"required_status_checks\",\"parameters\":{\"strict_required_status_checks_policy\":false,\"do_not_enforce_on_create\":false,\"required_status_checks\":[{\"context\":\"Semantic Pull Request\",\"integration_id\":15368},{\"context\":\"Build & Test Passed or Skipped\",\"integration_id\":15368},{\"context\":\"Lint: node-24, ubuntu-latest\",\"integration_id\":15368}]},\"ruleset_source_type\":\"Repository\",\"ruleset_source\":\"vitejs/vite\",\"ruleset_id\":24556645}]]", "stderr": "" }, + { + "args": [ + "api", + "graphql", + "-f", + "query=query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "-f", + "owner=vitejs", + "-f", + "name=vite", + "-f", + "oid=6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790" + ], + "exitCode": 0, + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790\",\"checkSuites\":{\"totalCount\":21,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGMGADNI=\"},\"nodes\":[{\"app\":{\"databaseId\":13473},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Pages changed - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"NEUTRAL\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"},{\"name\":\"Header rules - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"},{\"name\":\"Redirect rules - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"}]}},{\"app\":{\"databaseId\":2740},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":8329},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":85455},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":882742},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Continuous Releases\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:53Z\",\"completedAt\":\"2026-10-19T10:38:53Z\"}]}},{\"app\":{\"databaseId\":158384},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Graphite / AI Reviews\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:16Z\",\"completedAt\":\"2026-10-19T10:38:35Z\"}]}},{\"app\":{\"databaseId\":2278701},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":10,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:45Z\",\"completedAt\":\"2026-10-19T10:34:04Z\"},{\"name\":\"Get changed files\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:47Z\",\"completedAt\":\"2026-10-19T10:32:55Z\"},{\"name\":\"Build&Test: node-24, windows-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:59Z\",\"completedAt\":\"2026-10-19T10:38:49Z\"},{\"name\":\"Build&Test: node-22, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:02Z\"},{\"name\":\"Build&Test: node-20, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"FAILURE\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:35:17Z\"},{\"name\":\"Build&Test: node-24, macos-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:33:04Z\",\"completedAt\":\"2026-10-19T10:39:43Z\"},{\"name\":\"Build&Test: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:22Z\"},{\"name\":\"Build&Test: node-26, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:22Z\"},{\"name\":\"Build & Test Failed\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:40:15Z\",\"completedAt\":\"2026-10-19T10:40:22Z\"},{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:39:44Z\",\"completedAt\":\"2026-10-19T10:39:43Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Analyze (actions)\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:05Z\",\"completedAt\":\"2026-10-19T10:38:41Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Semantic Pull Request\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:05Z\",\"completedAt\":\"2026-10-19T10:38:12Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:06Z\",\"completedAt\":\"2026-10-19T10:38:19Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Run zizmor\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:06Z\",\"completedAt\":\"2026-10-19T10:38:25Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":10,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:07Z\",\"completedAt\":\"2026-10-19T10:39:22Z\"},{\"name\":\"Get changed files\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:07Z\",\"completedAt\":\"2026-10-19T10:38:14Z\"},{\"name\":\"Build&Test: node-24, windows-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:44:35Z\"},{\"name\":\"Build&Test: node-20, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:41:39Z\"},{\"name\":\"Build&Test: node-22, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:42:50Z\"},{\"name\":\"Build&Test: node-24, macos-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:24Z\",\"completedAt\":\"2026-10-19T10:45:51Z\"},{\"name\":\"Build&Test: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:22Z\",\"completedAt\":\"2026-10-19T10:41:20Z\"},{\"name\":\"Build&Test: node-26, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:18Z\",\"completedAt\":\"2026-10-19T10:42:25Z\"},{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:45:57Z\",\"completedAt\":\"2026-10-19T10:46:04Z\"},{\"name\":\"Build & Test Failed\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:45:52Z\",\"completedAt\":\"2026-10-19T10:45:51Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:04Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":57789},\"checkRuns\":{\"totalCount\":2,\"nodes\":[{\"name\":\"zizmor\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:38:19Z\"},{\"name\":\"CodeQL\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:32Z\",\"completedAt\":\"2026-10-19T10:38:34Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:33Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:36Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"},{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:37Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:41Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"}]}}]}}}}}", + "stderr": "" + }, { "args": [ "api", diff --git a/test/merge-recorded.test.ts b/test/merge-recorded.test.ts index 4008cc51..1212f049 100644 --- a/test/merge-recorded.test.ts +++ b/test/merge-recorded.test.ts @@ -27,7 +27,7 @@ it('reads pending and passing at one pull request head, and no app in any record expect([pending.gh, passing.gh, zero.gh]).toEqual(Array(3).fill('gh version 2.102.0 (2026-09-30)')); expect(pullOf(passing)).toMatchObject({ number: 183, baseRefOid: pullOf(pending).baseRefOid, headRefOid: pullOf(pending).headRefOid }); // gh's statusCheckRollup never carries a check's app (#176), in any recording. - for (const recording of [zero, pending, passing]) { + for (const recording of [zero, pending, passing, ...Object.values(nonAdmin)]) { const rollup = (JSON.parse(recording.calls.find(call => call.args[0] === 'pr')!.stdout) as { statusCheckRollup: object[] }).statusCheckRollup; for (const check of rollup) expect(check).not.toHaveProperty('app'); } @@ -54,6 +54,8 @@ it('reads the ruleset\'s required checks as pending, then as passing at the same // The same reads by a token without admin rights, recorded 2026-10-20 with gh 2.102.0 from public repositories this // token may only read. Each classic protection read answers 404 "Not Found", so classic checks come from the branch read. +// nonadmin-ruleset and nonadmin-mixed were recorded again at the same heads on 2026-10-21 (UTC), when the inspection +// began to read the head's check runs with their apps for checks pinned to an app (#176); their other answers did not change. // - nonadmin-ruleset: vitejs/vite PR 23837 into main, which only a ruleset covers (`protection.enabled` false); // - nonadmin-classic: cli/cli PR 14716 into trunk, with classic required checks and no ruleset checks; // - nonadmin-mixed: sveltejs/svelte PR 19050 into main, where a ruleset and classic protection both require one check. @@ -81,12 +83,14 @@ it('records the 404 a token without admin rights gets, and the branch read\'s cl it('reads a ruleset-only branch\'s required checks with a token without admin rights', async () => { const run = replayGh(nonAdmin.ruleset), state = await gatewayFor(nonAdmin.ruleset, run).inspect({ fresh: true }); expect(run.unrecorded).toEqual([]); - // The ruleset binds each check to GitHub Actions (app 15368). gh's rollup names no app (#176), so app-bound checks - // read as missing, though runs of these names passed: fail closed until #176. + // The ruleset binds each check to GitHub Actions (app 15368). gh's rollup names no app (#176), so they are matched from + // the head's check runs, where GitHub Actions reported a passing run of each (one of two runs of `Build & Test Passed + // or Skipped` was skipped, which passes). + expect(run.calls.filter(args => args[1] === 'graphql')).toHaveLength(1); expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, mergeQueue: false, requiredChecks: [ - { context: 'Semantic Pull Request', appId: 15368, state: 'missing' }, - { context: 'Build & Test Passed or Skipped', appId: 15368, state: 'missing' }, - { context: 'Lint: node-24, ubuntu-latest', appId: 15368, state: 'missing' }] }); + { context: 'Semantic Pull Request', appId: 15368, state: 'success' }, + { context: 'Build & Test Passed or Skipped', appId: 15368, state: 'success' }, + { context: 'Lint: node-24, ubuntu-latest', appId: 15368, state: 'success' }] }); }); it('reads a classic-protected branch\'s required checks with a token without admin rights', async () => { @@ -103,6 +107,13 @@ it('reads a classic-protected branch\'s required checks with a token without adm it('reads a check both a ruleset and classic protection require once, with a token without admin rights', async () => { const run = replayGh(nonAdmin.mixed), state = await gatewayFor(nonAdmin.mixed, run).inspect({ fresh: true }); expect(run.unrecorded).toEqual([]); + // The head has a skipped `vite-ecosystem-ci` run, but GitHub names no app for its check suite to this reader, so the + // run cannot be shown to come from app 2935623, and the pinned check reads as missing: it fails closed. + const suites = (JSON.parse(nonAdmin.mixed.calls.find(call => call.args[1] === 'graphql')!.stdout) as + { data: { repository: { object: { checkSuites: { nodes: { app: { databaseId: number } | null; checkRuns: { nodes: { name: string; conclusion: string }[] } }[] } } } } }) + .data.repository.object.checkSuites.nodes; + expect(suites.filter(suite => suite.checkRuns.nodes.some(check => check.name === 'vite-ecosystem-ci'))) + .toEqual([{ app: null, checkRuns: { totalCount: 1, nodes: [expect.objectContaining({ name: 'vite-ecosystem-ci', conclusion: 'SKIPPED' })] } }]); expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, requiredChecks: [{ context: 'vite-ecosystem-ci', appId: 2935623, state: 'missing' }] }); }); diff --git a/test/merge.test.ts b/test/merge.test.ts index 269ce29d..a24df5f4 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -36,6 +36,28 @@ function mergeReads(args: readonly string[], pull: Record = {}) if (/\/branches\/[^/]+$/.test(joined)) return JSON.stringify({ protected: false }); throw new Error(`Unexpected gh call: ${joined}`); } +/** + * The gateway's read of the head's check runs with their apps (`gh api graphql`, github/merge.ts `#checkRuns`). gh's + * statusCheckRollup names no app (#176), so this is the read a required check pinned to an app is matched from. + */ +const isCheckRunRead = (args: readonly string[]) => args[0] === 'api' && args[1] === 'graphql' && args.some(arg => arg.startsWith('query=') && arg.includes('checkSuites(')); +/** + * gh 2.102.0's answer to that read, as recorded in test/fixtures/gh/nonadmin-ruleset.json and nonadmin-mixed.json: exit 0 + * with GitHub's response on stdout, one entry per check suite with its app (`null` where GitHub names none, as for one + * suite in nonadmin-mixed.json) and its runs. The fake gives every field the query asks for; `page` overrides the + * suites' count and page info for the paging tests. A failed read is a CommandFailed (`ghExit`, below). + */ +function checkRunsAnswer(head: string, suites: Array<{ app: number | null; runs: Array> }>, + page: { totalCount?: number; hasNextPage?: boolean; endCursor?: string | null } = {}): string { + return JSON.stringify({ data: { repository: { object: { __typename: 'Commit', oid: head, checkSuites: { + totalCount: page.totalCount ?? suites.length, + pageInfo: { hasNextPage: page.hasNextPage ?? false, endCursor: page.endCursor !== undefined ? page.endCursor : suites.length ? 'Y3Vyc29yOnYyOpHPAAAAGN-eVHo=' : null }, + nodes: suites.map(suite => ({ app: suite.app === null ? null : { databaseId: suite.app }, checkRuns: { totalCount: suite.runs.length, nodes: suite.runs } })), + } } } } }); +} +/** One check run as that read gives it: GitHub's GraphQL API answers `null`, not gh's zero time, for a time not reached. */ +const checkRun = (name: string, status: string, conclusion: string | null, startedAt: string | null = '2026-10-11T00:00:00Z', completedAt: string | null = status === 'COMPLETED' ? '2026-10-11T00:01:00Z' : null) => + ({ name, status, conclusion, startedAt, completedAt }); function readyView(): ReviewView { return { items: [{ id: 'P1', state: 'approved', outside: [], acceptance: [{ type: 'check', text: 'Works' }], checks: { tests: '– No tests defined' } }], @@ -915,10 +937,11 @@ it('parses required checks from both rule sources and pins the gh merge head', a const run = async (args: readonly string[]) => { calls.push([...args]); const joined = args.join(' '); if (joined.startsWith('pr view 7')) { pullReads++; return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [ - { name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', app: { databaseId: 10 } }, + { name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS' }, { name: 'race', status: 'IN_PROGRESS', conclusion: 'SUCCESS' }, { context: 'lint', state: 'SUCCESS' }, ] }); } + if (isCheckRunRead(args)) return checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'merge_queue' }, { type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: 10 }, { context: 'race', integration_id: null }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); if (joined.includes('/protection')) return JSON.stringify({ required_status_checks: { strict: false, checks: [{ context: 'lint', app_id: null }] } }); @@ -1047,11 +1070,16 @@ it('ignores required reviews in both rule sources and reads only their required .toMatchObject({ ready: true, blockers: [] }); }); -/** Inspects a pull request whose one required check, `test` from any app, has these observed runs and statuses. */ -async function requiredTestState(statusCheckRollup: unknown[], appId: number | null = null) { +/** + * Inspects a pull request whose one required check is `test`, from any app or, with `appId`, from that app, and whose head + * has these observed runs and statuses in gh's rollup. `checkRuns` answers the read of the head's check runs with their + * apps (a string, or a function of the call's arguments that returns one or throws); without it that read is unexpected. + */ +async function requiredTestState(statusCheckRollup: unknown[], appId: number | null = null, checkRuns?: string | ((args: readonly string[]) => string | Promise)) { const run = async (args: readonly string[]) => { const joined = args.join(' '); if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup }); + if (checkRuns !== undefined && isCheckRunRead(args)) return typeof checkRuns === 'string' ? checkRuns : checkRuns(args); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: appId }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); if (joined.endsWith('/protection')) throw new Error('gh: Branch not protected (HTTP 404)'); @@ -1202,19 +1230,136 @@ it.each([ expect(await requiredTestState([...rollup])).toEqual([expected]); }); -// #176, until it is fixed: gh's rollup never carries a check run's app (test/fixtures/gh/, gh 2.102.0), so a requirement -// pinned to an app finds no matching run and reads as missing. That fails closed: a passing run from any app never -// satisfies it. -it('reads a requirement pinned to an app as missing against gh\'s app-less check runs', async () => { - expect(await requiredTestState([{ __typename: 'CheckRun', name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', - startedAt: '2026-10-20T00:00:00Z', completedAt: '2026-10-20T00:01:00Z', workflowName: 'CI' }], 10)).toEqual(['missing']); +// #176: gh's rollup never carries a check run's app (test/fixtures/gh/, gh 2.102.0), so a requirement pinned to an app is +// matched from the head's check runs, read with their apps. Each rollup below holds an app-less passing `test` run, as +// gh reports one; before #176 every requirement here read as missing. +const appLessPass = [{ __typename: 'CheckRun', name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', + startedAt: '2026-10-20T00:00:00Z', completedAt: '2026-10-20T00:01:00Z', workflowName: 'CI', detailsUrl: 'https://github.com/owner/repo/actions/runs/1/job/2' }]; +type Suite = { app: number | null; runs: Array> }; + +it.each([ + ['passed', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], 'success'], + ['was skipped', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SKIPPED')] }], 'success'], + ['is in progress', [{ app: 10, runs: [checkRun('test', 'IN_PROGRESS', null)] }], 'pending'], + ['is queued and has not started', [{ app: 10, runs: [checkRun('test', 'QUEUED', null, null)] }], 'pending'], + ['failed', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }], 'failure'], + ['passed, while another app\'s run of that name failed', [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }, { app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], 'success'], + ['was re-run in a new suite that is in progress', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }, + { app: 10, runs: [checkRun('test', 'IN_PROGRESS', null, '2026-10-11T00:05:00Z')] }], 'pending'], +] as const)('reads a required check pinned to an app from that app\'s run on the head, which %s', async (_case, suites, expected) => { + expect(await requiredTestState(appLessPass, 10, checkRunsAnswer(sha('b'), suites as unknown as Suite[]))).toEqual([expected]); }); -it('matches a requirement bound to an app only against that app\'s check runs, never a commit status', async () => { - expect(await requiredTestState([ - { name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', startedAt: '2026-10-11T00:00:00Z', app: { databaseId: 10 } }, - { context: 'test', state: 'FAILURE', startedAt: '2026-10-11T00:05:00Z' }, - ], 10)).toEqual(['success']); +it.each([ + ['a different app', [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]], + // As for one suite in nonadmin-mixed.json: GitHub names no app, so the run cannot be shown to be the pinned app's. + ['a suite GitHub names no app for', [{ app: null, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]], + ['nowhere: the head has no check suites', []], + ['the pinned app, under another name', [{ app: 10, runs: [checkRun('tests', 'COMPLETED', 'SUCCESS')] }]], +] as const)('reads a required check pinned to an app as missing when its only passing run is from %s', async (_case, suites) => { + expect(await requiredTestState(appLessPass, 10, checkRunsAnswer(sha('b'), suites as unknown as Suite[]))).toEqual(['missing']); +}); + +it('matches a requirement pinned to an app only against that app\'s check runs, never a commit status', async () => { + expect(await requiredTestState([{ context: 'test', state: 'SUCCESS', startedAt: '2026-10-11T00:05:00Z' }], 10, + checkRunsAnswer(sha('b'), []))).toEqual(['missing']); + expect(await requiredTestState([{ context: 'test', state: 'FAILURE', startedAt: '2026-10-11T00:05:00Z' }], 10, + checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]))).toEqual(['success']); +}); + +it('reads the check runs of the exact head the pull request read returned, and only for a pinned check', async () => { + const reads: string[][] = []; + expect(await requiredTestState(appLessPass, 10, args => { reads.push([...args]); return checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); })).toEqual(['success']); + expect(reads).toHaveLength(1); + expect(reads[0]!.slice(0, 3)).toEqual(['api', 'graphql', '-f']); + expect(reads[0]!.slice(4)).toEqual(['-f', 'owner=owner', '-f', 'name=repo', '-f', `oid=${sha('b')}`]); + // A check any app may report is read from the rollup alone: here gh has no answer for the check-run read. + expect(await requiredTestState(appLessPass, null)).toEqual(['success']); +}); + +it('reads every page of check suites on the head', async () => { + const pages: (string | null)[] = []; + const state = await requiredTestState(appLessPass, 10, args => { + const after = args.find(arg => arg.startsWith('after='))?.slice('after='.length) ?? null; + pages.push(after); + return after === null + ? checkRunsAnswer(sha('b'), [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], { totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }) + : checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'IN_PROGRESS', null)] }], { totalCount: 2, hasNextPage: false, endCursor: 'PAGE_END' }); + }); + expect(pages).toEqual([null, 'PAGE_2']); + expect(state).toEqual(['pending']); +}); + +/** A gateway for a pull request requiring `test` from app 10, where `answer` answers the read of the head's check runs. */ +function pinnedGateway(answer: (args: readonly string[], signal?: AbortSignal) => Promise) { + const run = async (args: readonly string[], runOptions?: { signal?: AbortSignal }) => { + const joined = args.join(' '); + if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: appLessPass }); + if (isCheckRunRead(args)) return answer(args, runOptions?.signal); + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (joined.endsWith('/protection')) throw NOT_PROTECTED; + { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } + throw new Error(`Unexpected gh call: ${joined}`); + }; + return new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21 }, run); +} + +// gh 2.102.0 exits 1 when GitHub's GraphQL answer has errors (stderr `gh: `, the response on stdout), so a +// response with errors on exit 0 is not what gh prints; it is kept as a case because the gateway checks for it. +const failedCheckRunReads: Array<[string, (args: readonly string[]) => Promise, RegExp]> = [ + ['gh exits with GitHub\'s error', async () => { throw ghExit('gh: Could not resolve to a Repository with the name \'owner/repo\'.', + '{"data":{"repository":null},"errors":[{"type":"NOT_FOUND","path":["repository"],"locations":[{"line":1,"column":69}],"message":"Could not resolve to a Repository with the name \'owner/repo\'."}]}'); }, + /^gh failed \(exit 1\): gh: Could not resolve to a Repository/], + ['GitHub answers with errors', async () => JSON.stringify({ data: null, errors: [{ message: 'Timeout on validation of query' }] }), /^GitHub returned check runs with errors\.$/], + // GitHub's answer for a commit it does not have (recorded with gh 2.102.0: exit 0). + ['GitHub does not find the head', async () => JSON.stringify({ data: { repository: { object: null } } }), /^GitHub returned no check runs for the pull request head\.$/], + ['GitHub answers for another commit', async () => checkRunsAnswer(sha('c'), []), /^GitHub returned no check runs for the pull request head\.$/], + ['a suite has more runs than one page holds', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]) + .replace('"checkRuns":{"totalCount":1', '"checkRuns":{"totalCount":101'), /more check runs than codeboost can read/], + ['a run has an unknown status', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'DONE', 'SUCCESS')] }]), /^GitHub returned a malformed check run\.$/], + ['a suite\'s app has no ID', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }]).replace('{"databaseId":10}', '{}'), /^GitHub returned incomplete check runs\.$/], + ['the suites change between pages', async args => args.some(arg => arg.startsWith('after=')) + ? checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }], { totalCount: 3 }) + : checkRunsAnswer(sha('b'), [{ app: 11, runs: [] }], { totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }), /changed while they were read/], + ['the pages hold fewer suites than GitHub counts', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }], { totalCount: 2 }), /changed while they were read/], + ['the head has more than 1,000 check suites', async () => checkRunsAnswer(sha('b'), [{ app: 11, runs: [] }], { totalCount: 1001, hasNextPage: true, endCursor: 'NEXT' }), + /more check suites than codeboost can read/], +]; + +it.each(failedCheckRunReads)('fails the inspection with its reason when the read of the pinned check\'s app fails: %s', async (_case, answer, reason) => { + await expect(pinnedGateway(answer).inspect({ fresh: true })).rejects.toThrow(reason); +}); + +it.each(failedCheckRunReads)('blocks the merge with the reason when the read of the pinned check\'s app fails: %s', async (_case, answer, reason) => { + const view = readyView(); + const status = await new MergeCoordinator(serviceFor(view), pinnedGateway(answer)).displayStatus(view); + expect(status).toMatchObject({ ready: false, action: null, remote: null }); + expect(status.blockers).toHaveLength(1); + expect(status.blockers[0]!.code).toBe('github'); + expect(status.blockers[0]!.message).toMatch(/^Could not read GitHub merge state\. /); + expect(status.blockers[0]!.message.replace(/^Could not read GitHub merge state\. /, '')).toMatch(reason); +}); + +it('keeps the inspection\'s timeout as the reason when the read of the pinned check\'s app outlasts it', async () => { + let stopped: unknown = null; + const gateway = pinnedGateway((_args, signal) => new Promise((_resolve, reject) => { + signal!.addEventListener('abort', () => { stopped = signal!.reason; reject(new Error('gh was stopped')); }, { once: true }); + })); + await expect(gateway.inspect({ fresh: true, timeoutMs: 50 })).rejects.toThrow('GitHub merge-state inspection timed out.'); + // The deadline stopped this read: it was in flight when the deadline fired. + expect(stopped).toBeInstanceOf(Error); + expect((stopped as Error).message).toBe('GitHub merge-state inspection timed out.'); +}); + +it('is ready to merge when the app-pinned required check passed on the head, and not when another app reported it', async () => { + for (const [app, ready] of [[10, true], [11, false]] as const) { + const view = readyView(); + const gateway = pinnedGateway(async () => checkRunsAnswer(sha('b'), [{ app, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }])); + const status = await new MergeCoordinator(serviceFor(view), gateway).status(view, true); + expect(status.remote.requiredChecks).toEqual([{ context: 'test', appId: 10, state: ready ? 'success' : 'missing' }]); + expect(status.ready).toBe(ready); + } }); it('rejects an unsupported runtime merge method', () => { From 5ec3b0c3e83b121df979c071ee83da7814692484 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 19:33:30 -0700 Subject: [PATCH 2/9] Ask GitHub for each pinned check's runs by app and name (#176 review) Round 1 of the independent review of PR #195 found that the read of every check run on the head, with each suite's app, misread real repositories: - A private app's suite has `app: null` in GraphQL, so its runs never met the check pinned to it (sveltejs/svelte's vite-ecosystem-ci). - A suite with more than 100 runs failed the whole inspection, though the pinned check was elsewhere (microsoft/vscode has one with 101). The read now asks GitHub, per pinned check, for the suites of its app (checkSuites filterBy appId, which also returns a private app's suite) and, in each, the latest run of its name (checkRuns filterBy checkName, checkType LATEST). Each check is one alias; up to 25 go in one call, and a check whose app has more than 100 suites is paged up to 10 pages. The read still fails closed on any overflow left in that narrowed query. The read is skipped when the rules are unknown, so it cannot replace that blocker. When stages stop each other, the inspection now reports the first stage's failure, even when a stopped read then ends with a gh exit of its own. Tests: the fake answers the new query as GitHub does, filtering by app and name; the exit-0 GraphQL error case is labelled defensive. New: several checks per call, more checks than one call holds, paging to exactly the page limit, a pinned check with the rules unknown, and a stop-reason case with a pinned check. The vite and svelte recordings were made again (svelte now reads as passing), and new recordings show a re-run inside one suite (codeboost PR 151), a suite with 101 runs (vscode PR 343099) and the private app's suite in GraphQL and REST. Co-Authored-By: Claude Opus 5.5 --- docs/designs/codeboost-plan-indexed-review.md | 2 +- docs/implementation/guarded-merge.md | 4 +- docs/implementation/merge-handoff.md | 2 +- github/merge.ts | 141 +++++++----- scripts/record-merge-gh.ts | 2 +- test/fixtures/gh/nonadmin-mixed.json | 14 +- test/fixtures/gh/nonadmin-ruleset.json | 22 +- test/fixtures/gh/pinned-large-suite.json | 40 ++++ test/fixtures/gh/pinned-rerun.json | 40 ++++ test/fixtures/gh/private-app-suites.json | 36 +++ test/merge-recorded.test.ts | 79 ++++++- test/merge-stop-reasons.test.ts | 30 +++ test/merge.test.ts | 210 ++++++++++++------ 13 files changed, 479 insertions(+), 143 deletions(-) create mode 100644 test/fixtures/gh/pinned-large-suite.json create mode 100644 test/fixtures/gh/pinned-rerun.json create mode 100644 test/fixtures/gh/private-app-suites.json diff --git a/docs/designs/codeboost-plan-indexed-review.md b/docs/designs/codeboost-plan-indexed-review.md index cabcbc4b..c2d566ad 100644 --- a/docs/designs/codeboost-plan-indexed-review.md +++ b/docs/designs/codeboost-plan-indexed-review.md @@ -2043,7 +2043,7 @@ These `T` IDs are requirement identifiers, not the build-order numbers. Build st - Surfaced by: R6 (D7: A) - Files: github/checks - Verify: recorded-output tests for zero checks, pending then passing, and timeout - - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). *Later (#176):* for a check pinned to an app, the inspection now reads the head's check runs with their apps through `gh api graphql`, since gh's rollup names no app; the vite and svelte recordings were made again with that read, and vite's pinned checks read as passing (`test/merge-recorded.test.ts`; `test/merge.test.ts`, "reads a required check pinned to an app from that app's run on the head, which %s"). The test named above was replaced. + - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). *Later (#176):* for a check pinned to an app, the inspection now asks GitHub, through `gh api graphql`, for that app's runs of that name on the head, since gh's rollup names no app; the vite and svelte recordings were made again with that read, and their pinned checks read as passing, svelte's from a private app (`test/merge-recorded.test.ts`; `test/merge.test.ts`, "reads a required check pinned to an app from that app's run on the head, which %s"). The test named above was replaced. - [x] **T13 (P2, human: ~1 hour / CC: ~5 min)** — runner — Use `node:sqlite` only; enforce the minimum Node version; add a CI warning check - Surfaced by: R7 (D8: A) - Files: runner/store, web/cli diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index a71afef1..5acbbe2b 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -27,7 +27,7 @@ The target is resolved again for each validation pass. Both passes, and the thir Merging blocks when any plan item is unreviewed or stale, an attributed file is outside that item's declared plan scope, an Ambiguous or Unplanned segment remains, a current change request is open, or a `cmd:` acceptance check lacks a passing result for the current head. Approval does not override an out-of-scope file; the plan must be amended. Historical change requests from an earlier revision or snapshot remain visible but do not block the current revision. -The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. A check that a ruleset (`integration_id`) or classic protection (`app_id`) pins to an app needs to know which app reported each check run, and gh's `statusCheckRollup` does not give it: gh 2.102.0 does not ask GitHub for a run's app (#176). So when any required check is pinned to an app, the adapter also reads the check runs of the exact head that the PR read returned, through `gh api graphql`: each check suite on that commit with its app's `databaseId`, and the suite's latest run of each name with its status, conclusion and times. Only those fields are asked for, which keeps the answer small (GitHub's REST check-run list would also return each run's output text). A pinned check matches only runs of its name from a suite of its app. A commit status has no app, so it never meets a pinned check, and neither does a run whose suite GitHub names no app for (sveltejs/svelte showed one to a token without admin rights): those read as missing. Checks any app may report are still read from the rollup alone. The read fails closed: a gh failure, an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs, more than 1,000 suites (10 pages) or suites that change between pages fails the inspection with that reason, which the merge status shows as “Could not read GitHub merge state.” followed by it; a deadline that stops the read keeps the inspection's timeout as the reason. GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. `test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The vite and svelte recordings were made again at the same heads on 2026-10-21 (UTC) to add the check-run read: vite's three checks pinned to GitHub Actions read as passing, and svelte's pinned check reads as missing, because GitHub names no app for the suite of its only run. The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. +The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. A check that a ruleset (`integration_id`) or classic protection (`app_id`) pins to an app needs to know which app reported each check run, and gh's `statusCheckRollup` does not give it: gh 2.102.0 does not ask GitHub for a run's app (#176). So when any required check is pinned to an app, and the rules are known, the adapter also asks GitHub for the runs of each pinned check on the exact head that the PR read returned, through `gh api graphql`. GitHub does the filtering: for each pinned check the query lists the check suites of its app on that commit (`checkSuites(filterBy: {appId})`) and, in each, the latest run of its name (`checkRuns(filterBy: {checkName, checkType: LATEST})`, which matches the name exactly), with status, conclusion and times. The app filter also returns the suite of a private app, whose `app` GitHub's GraphQL API gives as `null` to a reader outside the app's owner (sveltejs/svelte's `vite-ecosystem-ci`; REST names the app), so such a check is read like any other. Asking only for the pinned names keeps the answer small, and a suite with many other runs (microsoft/vscode has a GitHub Actions suite with 101) does not matter. `LATEST` leaves out a run re-run inside its suite (codeabovelab/codeboost PR 151: `test` failed, then passed when re-run, in one suite); runs of the name in several suites are ordered as the rollup's are. One call holds up to 25 pinned checks, so most inspections make one call. A check whose app has more than 100 suites on the head is paged, up to 10 pages. Checks any app may report are still read from the rollup alone. No commit status meets a pinned check. GitHub may count a status that the pinned app set; this is not checked, and such a check reads as missing or pending, which fails closed. With the rules unknown the read is skipped, so its failure cannot replace the rules blocker. The read fails closed. A gh failure keeps gh's error; an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs of the pinned name, more than 10 pages of suites, or suites that change between pages fails the inspection with codeboost's own reason. The merge status shows either as “Could not read GitHub merge state.” followed by the reason; a deadline that stops the read keeps the inspection's timeout as the reason. When stages stop each other, the inspection reports the first stage's failure, even if a stopped read then ends with a gh exit of its own. GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. `test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The vite and svelte recordings were made again at the same heads on 2026-10-21 (UTC) to add the pinned-check read: vite's three checks pinned to GitHub Actions and svelte's check pinned to its private app read as passing. Two more files record only that read, on commits whose own rules pin nothing: a re-run inside one suite (`pinned-rerun.json`) and a suite with 101 runs (`pinned-large-suite.json`); `private-app-suites.json` records the svelte suite's app as GraphQL and REST give it. The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge call aborted at its 14-second deadline settles by 14.4 s. *Later (#48, #170):* a click can also be waiting on its issue-trust read, whose stop wait is 0.75 s, or on a review load, whose history worker gets 1 s after the abort before it is terminated; so it settles about 0.75 s after its deadline, or about 1 s or a little more after it when a review load was running, past the 14.5-second shutdown drain (traced, not tested); see [merge-queue.md](merge-queue.md). Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected. @@ -39,4 +39,4 @@ This slice does not mutate Git history or run plan commands. If the base moved, ## Validation -Unit and integration tests cover local review blockers; current and stale base/head pairs; PR state and mergeability; known, unknown, empty, pending, and failing check requirements; app-bound and malformed app identities; app-pinned checks matched from the head's check runs (passed, pending, failed, reported by another app or by no named app) and each way that read can fail; server base protection; merge-queue refusal for adapters without queue inspection; runtime merge-method validation; the already-fixed check run through `GhAlreadyFixedGateway` (closing-only cross-references, the non-default-base rule, manual links, the closed state, base-branch commits, this PR excluded only while open, the merge inputs, fail-closed results and caller cancellation); a requirement changing between validation passes; deadline cancellation; post-action cache invalidation; and the exact `gh` merge argv. Browser coverage proves blockers are visible, the ready action uses the reviewed head, and duplicate clicks start only one merge. +Unit and integration tests cover local review blockers; current and stale base/head pairs; PR state and mergeability; known, unknown, empty, pending, and failing check requirements; app-bound and malformed app identities; app-pinned checks read by app and name (passed, pending, failed, reported by another app or under another name, several per call, paged suites up to the limit) and each way that read can fail; no pinned-check read with the rules unknown; the first stage's failure reported when a stopped pinned-check read ends with its own gh exit; server base protection; merge-queue refusal for adapters without queue inspection; runtime merge-method validation; the already-fixed check run through `GhAlreadyFixedGateway` (closing-only cross-references, the non-default-base rule, manual links, the closed state, base-branch commits, this PR excluded only while open, the merge inputs, fail-closed results and caller cancellation); a requirement changing between validation passes; deadline cancellation; post-action cache invalidation; and the exact `gh` merge argv. Browser coverage proves blockers are visible, the ready action uses the reviewed head, and duplicate clicks start only one merge. diff --git a/docs/implementation/merge-handoff.md b/docs/implementation/merge-handoff.md index 1814d853..1c95c174 100644 --- a/docs/implementation/merge-handoff.md +++ b/docs/implementation/merge-handoff.md @@ -35,7 +35,7 @@ Without a runner, **Merge PR** works as before. | Wait for checks | The job inspects the pull request (fresh) at once, then again after 10 s, with the wait doubling up to 60 s. | every required check succeeds, or the required set was read and is empty, and nothing else blocks | a required check fails, or 30 minutes pass (job: `blocked`, if the preparation still applies: the block re-checks readiness in its transaction, and a review edit or other task change meanwhile ends the job `failed` instead); the pull request moved, another blocker appears, or GitHub or the local review cannot be read (job: `failed`; one failed read fails closed rather than retrying; each inspection also loads the review, which awaits a history worker since #170, within the same 30-minute limit) | | Merge | The job calls the guarded merge with the current review token, not the click's: a rebase during preparation makes a new snapshot and so a new token. A review change after preparation is refused through readiness, which binds the review version. Its inspections rerun the already-fixed check. | GitHub merged or queued the head, as the job's own merge attempt records it when the job reads it after the guarded merge (job: `merged` or `queued`; if a merge-queue poll ended the attempt removed or failed while GitHub was asked, the guarded merge cannot save it queued and rejects, and the job ends as for a refusal; `submitted` when GitHub accepted the merge but saving its outcome failed, so the attempt is still submitting and reconciles from GitHub) | GitHub refused (job: `blocked`; `failed` when GitHub refused because the head changed, which needs a fresh review, or when the review, snapshot or plan revision changed while GitHub was asked: the block is for the review the merge attempt was admitted for. A review edit (a review note) may land while the attempt is in flight. The snapshot term is defensive for a merge job: jobs need a runner, and a runner server's review load reads the recorded head, so no HEAD observation records a new snapshot. A plan revision lands only after a merge-queue poll ended the attempt while GitHub was asked, as plan writes are refused while a merge attempt is in flight), validation refused (job: `failed`), or the outcome is unknown (job: `submitted`; the merge attempt reconciles as before) | -A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. gh's rollup does not say which app reported a run (#176), so for such a check the inspection reads the head's check runs with their apps from GitHub (`guarded-merge.md`); a run whose app GitHub does not name, and any commit status, never match it. If that read fails, the inspection fails, and the job ends `failed` with GitHub's error. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. +A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. gh's rollup does not say which app reported a run (#176), so for such a check the inspection asks GitHub for that app's runs of that name on the head (`guarded-merge.md`). No commit status matches it; GitHub may count a status the app set, so such a check waits or reads missing (fails closed). If that read fails, the inspection fails, and the job ends `failed` with the reason: gh's error, or codeboost's own when GitHub's answer is malformed, too large to read whole, or changed between pages. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. A push to the base branch during the wait is such a blocker: the next inspection sees the moved base, and the job ends `failed` with "The base branch moved; merging rebases onto it and prepares again, so the result may need review." The task stays in review. The person clicks **Merge PR** again: the new job rebases onto the new base and prepares again. If the rebase makes an approval stale, that job ends `review-required`. diff --git a/github/merge.ts b/github/merge.ts index 128855d4..ab838d9e 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -218,6 +218,12 @@ function classicFromBranch(required: unknown): { enforced: boolean; strict: fals /** Every value GitHub's GraphQL API documents for a check run's `status` and `conclusion` (CheckStatusState, CheckConclusionState). */ const CHECK_RUN_STATUSES = ['REQUESTED','QUEUED','IN_PROGRESS','COMPLETED','WAITING','PENDING']; const CHECK_RUN_CONCLUSIONS = ['ACTION_REQUIRED','CANCELLED','FAILURE','NEUTRAL','SKIPPED','STALE','STARTUP_FAILURE','SUCCESS','TIMED_OUT']; +/** + * Pinned checks read per `gh api graphql` call, and pages of 100 suites read per check. Each check asks for at most + * 100 suites of 100 runs, so 25 stay well inside GitHub's limit of 500,000 nodes per query. + */ +const PINNED_CHECKS_PER_READ = 25, PINNED_SUITE_PAGES = 10; +const pinnedKey = (context: string, appId: number) => `${context}\0${appId}`; /** Among runs with the same start, the one that passes least counts, so a tie never reads as passing. */ const checkStateRank = { failure: 0, pending: 1, success: 2 } as const; @@ -327,6 +333,10 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { published ? this.#branchOpen(target.headBranch!, reads).catch(stopOthers) : undefined, ]); const fixed = await alreadyFixed; + // The first stage to fail stopped the others, and its error is `failed`'s reason. A stopped stage can still settle + // with an error of its own (a gh exit that came while gh was stopping), so the first failure is reported, not the + // first stage in this list that failed. Every rejection above aborts `failed`, so the loop after it is a backstop. + if (failed.signal.aborted) throw failed.signal.reason; for (const read of [ruleRead, listRead]) if (read.status === 'rejected') throw read.reason; if (fixed instanceof Error) throw fixed; const rules = (ruleRead as PromiseFulfilledResult).value; @@ -362,60 +372,82 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { } /** - * Every check run on commit `head`, with the ID of the app that reported it, in the shape of gh's statusCheckRollup - * entries plus `appId` (null for a suite GitHub names no app for). gh's statusCheckRollup does not ask for a run's app - * (#176), so a required check that a rule pins to an app is matched from this read. Only the fields the gateway reads - * are asked for, which keeps the answer small: a run's output text, which the REST API returns, is not. Each suite's - * latest run of each name is read, as GitHub counts for required checks. The read fails closed: on a gh failure (its error is kept), on errors or a - * malformed field in GitHub's answer, on a suite with more than 100 runs, on more than 1,000 suites, and when the - * suites change between pages. + * For each required check pinned to an app, the runs of that check's name on commit `head` from that app's check suites, + * in the shape of gh's statusCheckRollup entries, keyed by `pinnedKey`. gh's statusCheckRollup does not ask for a + * run's app (#176), so a pinned check is matched from this read, `gh api graphql`. GitHub filters on its side, by + * suite app (`checkSuites(filterBy: {appId})`) and by run name (`checkRuns(filterBy: {checkName})`, which matches + * the name exactly), so the answer holds only the runs that can meet a pinned check. The app filter also returns a + * suite whose `app` GitHub hides from this reader (a private app), which an app read from the answer would not name. + * Only each suite's latest run of a name is asked for (`checkType: LATEST`): a run re-run inside its suite is left out. + * Each check is one alias in the query, with up to PINNED_CHECKS_PER_READ checks per call; a check whose app has more + * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. + * + * The read fails closed. A gh failure keeps gh's error. GitHub's own errors in an answer, a malformed field, an answer + * for another commit, a suite with more runs of the name than one page holds, more suites than the page limit, and + * suites that change between pages each fail it with codeboost's reason. */ - async #checkRuns(head: string, signal?: AbortSignal): Promise & { appId: number | null }>> { + async #pinnedRuns(head: string, pinned: ReadonlyArray<{ context: string; appId: number }>, signal?: AbortSignal): Promise>>> { const [owner, name] = this.config.repository.split('/') as [string, string]; - const query = 'query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}'; - const runs: Array & { appId: number | null }> = []; - let cursor: string | null = null, suites = 0, total: number | null = null; - for (let page = 1; ; page++) { - const args = ['api','graphql','-f',`query=${query}`,'-f',`owner=${owner}`,'-f',`name=${name}`,'-f',`oid=${head}`]; - if (cursor !== null) args.push('-f', `after=${cursor}`); - const response = await this.#json(args, signal) as { data?: { repository?: { object?: Record | null } | null }; errors?: unknown } | null; - signal?.throwIfAborted(); - if (!response || typeof response !== 'object' || (Object.hasOwn(response, 'errors') && (!Array.isArray(response.errors) || response.errors.length > 0))) - throw new Error('GitHub returned check runs with errors.'); - const commit = response.data?.repository?.object; - if (!commit || commit.__typename !== 'Commit' || commit.oid !== head) throw new Error('GitHub returned no check runs for the pull request head.'); - const connection = commit.checkSuites as { totalCount?: unknown; pageInfo?: { hasNextPage?: unknown; endCursor?: unknown } | null; nodes?: unknown } | null | undefined; - if (!connection || typeof connection !== 'object' || !Number.isSafeInteger(connection.totalCount) || (connection.totalCount as number) < 0 || !Array.isArray(connection.nodes) - || !connection.pageInfo || typeof connection.pageInfo.hasNextPage !== 'boolean' || (connection.pageInfo.endCursor !== null && typeof connection.pageInfo.endCursor !== 'string')) - throw new Error('GitHub returned incomplete check runs.'); - if (total !== null && connection.totalCount !== total) throw new Error('The check suites on the pull request head changed while they were read.'); - total = connection.totalCount as number; - for (const suite of connection.nodes) { - const value = suite as { app?: { databaseId?: unknown } | null; checkRuns?: { totalCount?: unknown; nodes?: unknown } | null } | null; - if (!value || typeof value !== 'object' || !Object.hasOwn(value, 'app') || (value.app !== null && (typeof value.app !== 'object' || !validNumber(value.app.databaseId)))) - throw new Error('GitHub returned incomplete check runs.'); - const appId = value.app === null ? null : value.app!.databaseId as number; - const checkRuns = value.checkRuns; - if (!checkRuns || typeof checkRuns !== 'object' || !Number.isSafeInteger(checkRuns.totalCount) || !Array.isArray(checkRuns.nodes)) throw new Error('GitHub returned incomplete check runs.'); - // One page of runs per suite is read: a suite with more runs than its page holds could hide the required one. - if (checkRuns.nodes.length !== checkRuns.totalCount) throw new Error('A check suite on the pull request head has more check runs than codeboost can read.'); - for (const run of checkRuns.nodes) { - const check = run as Record | null; - if (!check || typeof check !== 'object' || typeof check.name !== 'string' || !CHECK_RUN_STATUSES.includes(check.status as string) - || (check.conclusion !== null && !CHECK_RUN_CONCLUSIONS.includes(check.conclusion as string)) - || (check.startedAt !== null && typeof check.startedAt !== 'string') || (check.completedAt !== null && typeof check.completedAt !== 'string')) - throw new Error('GitHub returned a malformed check run.'); - runs.push({ name: check.name, status: check.status, conclusion: check.conclusion, startedAt: check.startedAt, completedAt: check.completedAt, appId }); + const found = new Map>>(); + for (let start = 0; start < pinned.length; start += PINNED_CHECKS_PER_READ) { + // Every check of this call, with its cursor (null for its first page), its suites seen and GitHub's suite count. + const checks = pinned.slice(start, start + PINNED_CHECKS_PER_READ).map(check => ({ ...check, cursor: null as string | null, suites: 0, total: null as number | null, done: false })); + for (const check of checks) found.set(pinnedKey(check.context, check.appId), []); + for (let page = 1; checks.some(check => !check.done); page++) { + const open = checks.map((check, index) => ({ check, index })).filter(({ check }) => !check.done); + const declared = open.map(({ index }) => `,$app${index}:Int!,$name${index}:String!,$after${index}:String`).join(''); + const selected = open.map(({ index }) => ` c${index}:checkSuites(first:100,after:$after${index},filterBy:{appId:$app${index}}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name${index},checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}`).join(''); + const query = `query($owner:String!,$repo:String!,$oid:GitObjectID!${declared}){repository(owner:$owner,name:$repo){object(oid:$oid){__typename ... on Commit{oid${selected}}}}}`; + // `-F` makes an app ID a number; a name always goes with `-f`, which gh passes as a literal string. + const args = ['api','graphql','-f',`query=${query}`,'-f',`owner=${owner}`,'-f',`repo=${name}`,'-f',`oid=${head}`]; + for (const { check, index } of open) { + args.push('-F', `app${index}=${check.appId}`, '-f', `name${index}=${check.context}`); + if (check.cursor !== null) args.push('-f', `after${index}=${check.cursor}`); + } + const response = await this.#json(args, signal) as { data?: { repository?: { object?: Record | null } | null }; errors?: unknown } | null; + signal?.throwIfAborted(); + // gh exits 1 on an answer with errors, so this is defensive: such an answer never reaches here as gh's output. + if (!response || typeof response !== 'object' || (Object.hasOwn(response, 'errors') && (!Array.isArray(response.errors) || response.errors.length > 0))) + throw new Error('GitHub returned check runs with errors.'); + const commit = response.data?.repository?.object; + if (!commit || commit.__typename !== 'Commit' || commit.oid !== head) throw new Error('GitHub returned no check runs for the pull request head.'); + for (const { check, index } of open) { + const connection = commit[`c${index}`] as { totalCount?: unknown; pageInfo?: { hasNextPage?: unknown; endCursor?: unknown } | null; nodes?: unknown } | null | undefined; + if (!connection || typeof connection !== 'object' || !Number.isSafeInteger(connection.totalCount) || (connection.totalCount as number) < 0 || !Array.isArray(connection.nodes) + || !connection.pageInfo || typeof connection.pageInfo.hasNextPage !== 'boolean' || (connection.pageInfo.endCursor !== null && typeof connection.pageInfo.endCursor !== 'string')) + throw new Error('GitHub returned incomplete check runs.'); + if (check.total !== null && connection.totalCount !== check.total) throw new Error('The check suites on the pull request head changed while they were read.'); + check.total = connection.totalCount as number; + const runs = found.get(pinnedKey(check.context, check.appId))!; + for (const suite of connection.nodes) { + const checkRuns = (suite as { checkRuns?: { totalCount?: unknown; nodes?: unknown } | null } | null)?.checkRuns; + if (!checkRuns || typeof checkRuns !== 'object' || !Number.isSafeInteger(checkRuns.totalCount) || !Array.isArray(checkRuns.nodes)) throw new Error('GitHub returned incomplete check runs.'); + // One page of runs per suite is read: more latest runs of one name in one suite than it holds could hide one. + if (checkRuns.nodes.length !== checkRuns.totalCount) throw new Error(`A check suite on the pull request head has more runs named ${JSON.stringify(check.context)} than codeboost can read.`); + for (const run of checkRuns.nodes) { + const value = run as Record | null; + if (!value || typeof value !== 'object' || typeof value.name !== 'string' || !CHECK_RUN_STATUSES.includes(value.status as string) + || (value.conclusion !== null && !CHECK_RUN_CONCLUSIONS.includes(value.conclusion as string)) + || (value.startedAt !== null && typeof value.startedAt !== 'string') || (value.completedAt !== null && typeof value.completedAt !== 'string')) + throw new Error('GitHub returned a malformed check run.'); + // GitHub's name filter is exact (checked against GitHub); a run of another name could only be dropped here. + if (value.name === check.context) + runs.push({ name: value.name, status: value.status, conclusion: value.conclusion, startedAt: value.startedAt, completedAt: value.completedAt }); + } + } + check.suites += connection.nodes.length; + if (!connection.pageInfo.hasNextPage) { + if (check.suites !== check.total) throw new Error('The check suites on the pull request head changed while they were read.'); + check.done = true; + continue; + } + if (page === PINNED_SUITE_PAGES || typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) + throw new Error(`The pull request head has more check suites from app ${check.appId} than codeboost can read.`); + check.cursor = connection.pageInfo.endCursor; } } - suites += connection.nodes.length; - if (!connection.pageInfo.hasNextPage) break; - if (page === 10 || typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) - throw new Error('The pull request head has more check suites than codeboost can read.'); - cursor = connection.pageInfo.endCursor; } - if (suites !== total) throw new Error('The check suites on the pull request head changed while they were read.'); - return runs; + return found; } /** The effective branch rules and the state of each required check on the PR, whose head is `head`. */ @@ -506,13 +538,16 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { } } // A check any app may report is matched from gh's rollup, check runs and commit statuses alike. One pinned to an app - // is matched only from the check runs that app reported on the head (#176): the rollup names no app, and a commit - // status has none, so a status never meets a pinned check. A failed read fails the inspection with its error. - const pinned = [...requirements.values()].some(required => required.appId !== null) ? await this.#checkRuns(head, signal) : []; + // is matched only from the check runs that app reported on the head (#176): the rollup names no app. No commit status + // meets a pinned check, though GitHub may count one an app set: that fails closed (the check waits or reads missing). + // With the rules unknown the merge is blocked anyway, so the read is skipped and cannot replace that blocker. A failed + // read fails the inspection with its reason. + const pinnedChecks = [...requirements.values()].filter((required): required is { context: string; appId: number } => required.appId !== null); + const pinned = rulesKnown && pinnedChecks.length ? await this.#pinnedRuns(head, pinnedChecks, signal) : new Map>>(); const requiredChecks = [...requirements.values()].map(required => { const matches = required.appId === null ? observed.filter(check => (typeof check.name === 'string' ? check.name : check.context) === required.context) - : pinned.filter(run => run.name === required.context && run.appId === required.appId); + : pinned.get(pinnedKey(required.context, required.appId)) ?? []; if (!matches.length) return { ...required, state: 'missing' as const }; // A re-run adds a newer run (or status) with the same name; GitHub counts the latest one, so an older failure // does not outrank a newer pending or passing run. A check run and a commit status with the same name are not diff --git a/scripts/record-merge-gh.ts b/scripts/record-merge-gh.ts index c2a14167..2b032f5e 100644 --- a/scripts/record-merge-gh.ts +++ b/scripts/record-merge-gh.ts @@ -4,7 +4,7 @@ // // The production GhMergeGateway runs one fresh inspection of the pull request through the real `gh`, signed in as you. // Every `gh` call it makes (the pull request, the ruleset, the branch and its classic protection, and, when a required -// check is pinned to an app, the head's check runs with their apps) is saved with its exit +// check is pinned to an app, the runs of each pinned check) is saved with its exit // status, stdout and stderr, as runWithInput reports them (trimmed on a failure), to test/fixtures/gh/.json. The // already-fixed check is replaced by a stub, so its reads are not recorded. A branch read keeps only `name`, // `protected` and `protection` (the gateway reads only the last two), dropping the commit and its authors' emails. Use a public diff --git a/test/fixtures/gh/nonadmin-mixed.json b/test/fixtures/gh/nonadmin-mixed.json index 21a11d11..74137542 100644 --- a/test/fixtures/gh/nonadmin-mixed.json +++ b/test/fixtures/gh/nonadmin-mixed.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "sveltejs/svelte", "pullRequest": 19050, - "recordedAt": "2026-10-21T00:45:50.695Z", + "recordedAt": "2026-10-21T02:11:13.416Z", "calls": [ { "args": [ @@ -20,16 +20,20 @@ "api", "graphql", "-f", - "query=query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "query=query($owner:String!,$repo:String!,$oid:GitObjectID!,$app0:Int!,$name0:String!,$after0:String){repository(owner:$owner,name:$repo){object(oid:$oid){__typename ... on Commit{oid c0:checkSuites(first:100,after:$after0,filterBy:{appId:$app0}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name0,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", "-f", "owner=sveltejs", "-f", - "name=svelte", + "repo=svelte", "-f", - "oid=111cc6411c9c647f691a159862da2b74212791e3" + "oid=111cc6411c9c647f691a159862da2b74212791e3", + "-F", + "app0=2935623", + "-f", + "name0=vite-ecosystem-ci" ], "exitCode": 0, - "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"111cc6411c9c647f691a159862da2b74212791e3\",\"checkSuites\":{\"totalCount\":4,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGN-eVHo=\"},\"nodes\":[{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":null,\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"vite-ecosystem-ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"completedAt\":\"2026-10-20T12:41:24Z\"}]}},{\"app\":{\"databaseId\":4226557},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Vercel Internal Security Review\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-20T12:41:25Z\",\"completedAt\":\"2026-10-20T12:41:49Z\"}]}},{\"app\":{\"databaseId\":8329},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Vercel Agent Review\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-20T12:41:26Z\",\"completedAt\":\"2026-10-20T12:42:40Z\"}]}}]}}}}}", + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"111cc6411c9c647f691a159862da2b74212791e3\",\"c0\":{\"totalCount\":1,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGN-eHXo=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"vite-ecosystem-ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-20T12:41:24Z\",\"completedAt\":\"2026-10-20T12:41:24Z\"}]}}]}}}}}", "stderr": "" }, { diff --git a/test/fixtures/gh/nonadmin-ruleset.json b/test/fixtures/gh/nonadmin-ruleset.json index 13bd4c25..dc32923e 100644 --- a/test/fixtures/gh/nonadmin-ruleset.json +++ b/test/fixtures/gh/nonadmin-ruleset.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "vitejs/vite", "pullRequest": 23837, - "recordedAt": "2026-10-21T00:45:48.202Z", + "recordedAt": "2026-10-21T02:11:09.241Z", "calls": [ { "args": [ @@ -20,16 +20,28 @@ "api", "graphql", "-f", - "query=query($owner:String!,$name:String!,$oid:GitObjectID!,$after:String){repository(owner:$owner,name:$name){object(oid:$oid){__typename ... on Commit{oid checkSuites(first:100,after:$after){totalCount pageInfo{hasNextPage endCursor} nodes{app{databaseId} checkRuns(first:100,filterBy:{checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "query=query($owner:String!,$repo:String!,$oid:GitObjectID!,$app0:Int!,$name0:String!,$after0:String,$app1:Int!,$name1:String!,$after1:String,$app2:Int!,$name2:String!,$after2:String){repository(owner:$owner,name:$repo){object(oid:$oid){__typename ... on Commit{oid c0:checkSuites(first:100,after:$after0,filterBy:{appId:$app0}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name0,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}} c1:checkSuites(first:100,after:$after1,filterBy:{appId:$app1}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name1,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}} c2:checkSuites(first:100,after:$after2,filterBy:{appId:$app2}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name2,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", "-f", "owner=vitejs", "-f", - "name=vite", + "repo=vite", "-f", - "oid=6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790" + "oid=6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790", + "-F", + "app0=15368", + "-f", + "name0=Semantic Pull Request", + "-F", + "app1=15368", + "-f", + "name1=Build & Test Passed or Skipped", + "-F", + "app2=15368", + "-f", + "name2=Lint: node-24, ubuntu-latest" ], "exitCode": 0, - "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790\",\"checkSuites\":{\"totalCount\":21,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGMGADNI=\"},\"nodes\":[{\"app\":{\"databaseId\":13473},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Pages changed - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"NEUTRAL\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"},{\"name\":\"Header rules - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"},{\"name\":\"Redirect rules - vite-docs-main\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"}]}},{\"app\":{\"databaseId\":2740},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":8329},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":85455},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":882742},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Continuous Releases\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:53Z\",\"completedAt\":\"2026-10-19T10:38:53Z\"}]}},{\"app\":{\"databaseId\":158384},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Graphite / AI Reviews\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:16Z\",\"completedAt\":\"2026-10-19T10:38:35Z\"}]}},{\"app\":{\"databaseId\":2278701},\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":10,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:45Z\",\"completedAt\":\"2026-10-19T10:34:04Z\"},{\"name\":\"Get changed files\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:47Z\",\"completedAt\":\"2026-10-19T10:32:55Z\"},{\"name\":\"Build&Test: node-24, windows-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:59Z\",\"completedAt\":\"2026-10-19T10:38:49Z\"},{\"name\":\"Build&Test: node-22, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:02Z\"},{\"name\":\"Build&Test: node-20, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"FAILURE\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:35:17Z\"},{\"name\":\"Build&Test: node-24, macos-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:33:04Z\",\"completedAt\":\"2026-10-19T10:39:43Z\"},{\"name\":\"Build&Test: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:22Z\"},{\"name\":\"Build&Test: node-26, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:58Z\",\"completedAt\":\"2026-10-19T10:36:22Z\"},{\"name\":\"Build & Test Failed\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:40:15Z\",\"completedAt\":\"2026-10-19T10:40:22Z\"},{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:39:44Z\",\"completedAt\":\"2026-10-19T10:39:43Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Analyze (actions)\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:05Z\",\"completedAt\":\"2026-10-19T10:38:41Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Semantic Pull Request\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:05Z\",\"completedAt\":\"2026-10-19T10:38:12Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:06Z\",\"completedAt\":\"2026-10-19T10:38:19Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:02Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"},{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:02Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:03Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:12Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Run zizmor\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:06Z\",\"completedAt\":\"2026-10-19T10:38:25Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":10,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:07Z\",\"completedAt\":\"2026-10-19T10:39:22Z\"},{\"name\":\"Get changed files\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:07Z\",\"completedAt\":\"2026-10-19T10:38:14Z\"},{\"name\":\"Build&Test: node-24, windows-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:44:35Z\"},{\"name\":\"Build&Test: node-20, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:41:39Z\"},{\"name\":\"Build&Test: node-22, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:42:50Z\"},{\"name\":\"Build&Test: node-24, macos-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:24Z\",\"completedAt\":\"2026-10-19T10:45:51Z\"},{\"name\":\"Build&Test: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:22Z\",\"completedAt\":\"2026-10-19T10:41:20Z\"},{\"name\":\"Build&Test: node-26, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:18Z\",\"completedAt\":\"2026-10-19T10:42:25Z\"},{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:45:57Z\",\"completedAt\":\"2026-10-19T10:46:04Z\"},{\"name\":\"Build & Test Failed\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:45:52Z\",\"completedAt\":\"2026-10-19T10:45:51Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:04Z\",\"completedAt\":\"2026-10-19T10:38:03Z\"}]}},{\"app\":{\"databaseId\":57789},\"checkRuns\":{\"totalCount\":2,\"nodes\":[{\"name\":\"zizmor\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:17Z\",\"completedAt\":\"2026-10-19T10:38:19Z\"},{\"name\":\"CodeQL\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:32Z\",\"completedAt\":\"2026-10-19T10:38:34Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"preview\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:33Z\",\"completedAt\":\"2026-10-19T10:38:57Z\"}]}},{\"app\":{\"databaseId\":15368},\"checkRuns\":{\"totalCount\":3,\"nodes\":[{\"name\":\"AgentScan\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:36Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"},{\"name\":\"Comment and close PR\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:37Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"},{\"name\":\"Comment and close issue\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:38:41Z\",\"completedAt\":\"2026-10-19T10:38:30Z\"}]}}]}}}}}", + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"6b6d2b6e0e4794c9e5a18a94eaf29fbfef919790\",\"c0\":{\"totalCount\":13,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGMGADNI=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Semantic Pull Request\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:05Z\",\"completedAt\":\"2026-10-19T10:38:12Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}}]},\"c1\":{\"totalCount\":13,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGMGADNI=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\",\"startedAt\":\"2026-10-19T10:39:44Z\",\"completedAt\":\"2026-10-19T10:39:43Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Build & Test Passed or Skipped\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:45:57Z\",\"completedAt\":\"2026-10-19T10:46:04Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}}]},\"c2\":{\"totalCount\":13,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGMGADNI=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:32:45Z\",\"completedAt\":\"2026-10-19T10:34:04Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Lint: node-24, ubuntu-latest\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-19T10:38:07Z\",\"completedAt\":\"2026-10-19T10:39:22Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}}]}}}}}", "stderr": "" }, { diff --git a/test/fixtures/gh/pinned-large-suite.json b/test/fixtures/gh/pinned-large-suite.json new file mode 100644 index 00000000..97ebb94b --- /dev/null +++ b/test/fixtures/gh/pinned-large-suite.json @@ -0,0 +1,40 @@ +{ + "gh": "gh version 2.102.0 (2026-09-30)", + "repository": "microsoft/vscode", + "pullRequest": 343099, + "recordedAt": "2026-10-21T02:14:52.164Z", + "calls": [ + { + "args": [ + "api", + "graphql", + "-f", + "query=query($owner:String!,$repo:String!,$oid:GitObjectID!,$app0:Int!,$name0:String!,$after0:String){repository(owner:$owner,name:$repo){object(oid:$oid){__typename ... on Commit{oid c0:checkSuites(first:100,after:$after0,filterBy:{appId:$app0}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name0,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "-f", + "owner=microsoft", + "-f", + "repo=vscode", + "-f", + "oid=b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f", + "-F", + "app0=15368", + "-f", + "name0=Detect Changes" + ], + "exitCode": 0, + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f\",\"c0\":{\"totalCount\":5,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAGPEbyF8=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}},{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"Detect Changes\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-21T01:54:29Z\",\"completedAt\":\"2026-10-21T01:54:35Z\"}]}}]}}}}}", + "stderr": "" + }, + { + "args": [ + "api", + "repos/microsoft/vscode/check-suites/107124344927/check-runs?per_page=1", + "--jq", + "{total_count, check_suite: {id: .check_runs[0].check_suite.id, head_sha: .check_runs[0].head_sha}, app: {id: .check_runs[0].app.id}}" + ], + "exitCode": 0, + "stdout": "{\"app\":{\"id\":15368},\"check_suite\":{\"head_sha\":\"b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f\",\"id\":107124344927},\"total_count\":101}\n", + "stderr": "" + } + ] +} diff --git a/test/fixtures/gh/pinned-rerun.json b/test/fixtures/gh/pinned-rerun.json new file mode 100644 index 00000000..8afda3ec --- /dev/null +++ b/test/fixtures/gh/pinned-rerun.json @@ -0,0 +1,40 @@ +{ + "gh": "gh version 2.102.0 (2026-09-30)", + "repository": "codeabovelab/codeboost", + "pullRequest": 151, + "recordedAt": "2026-10-21T02:11:40.539Z", + "calls": [ + { + "args": [ + "api", + "graphql", + "-f", + "query=query($owner:String!,$repo:String!,$oid:GitObjectID!,$app0:Int!,$name0:String!,$after0:String){repository(owner:$owner,name:$repo){object(oid:$oid){__typename ... on Commit{oid c0:checkSuites(first:100,after:$after0,filterBy:{appId:$app0}){totalCount pageInfo{hasNextPage endCursor} nodes{checkRuns(first:100,filterBy:{checkName:$name0,checkType:LATEST}){totalCount nodes{name status conclusion startedAt completedAt}}}}}}}}", + "-f", + "owner=codeabovelab", + "-f", + "repo=codeboost", + "-f", + "oid=e3697dd532643de0f831b678856b2d677338231d", + "-F", + "app0=15368", + "-f", + "name0=test" + ], + "exitCode": 0, + "stdout": "{\"data\":{\"repository\":{\"object\":{\"__typename\":\"Commit\",\"oid\":\"e3697dd532643de0f831b678856b2d677338231d\",\"c0\":{\"totalCount\":3,\"pageInfo\":{\"hasNextPage\":false,\"endCursor\":\"Y3Vyc29yOnYyOpHPAAAAF-CVb90=\"},\"nodes\":[{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"test\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-08T22:02:19Z\",\"completedAt\":\"2026-10-08T22:09:24Z\"}]}},{\"checkRuns\":{\"totalCount\":1,\"nodes\":[{\"name\":\"test\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\",\"startedAt\":\"2026-10-08T21:54:42Z\",\"completedAt\":\"2026-10-08T22:01:14Z\"}]}},{\"checkRuns\":{\"totalCount\":0,\"nodes\":[]}}]}}}}}", + "stderr": "" + }, + { + "args": [ + "api", + "repos/codeabovelab/codeboost/commits/e3697dd532643de0f831b678856b2d677338231d/check-runs?filter=all&per_page=100", + "--jq", + "[.check_runs[] | {name, status, conclusion, started_at, completed_at, check_suite: {id: .check_suite.id}, app: {id: .app.id}}]" + ], + "exitCode": 0, + "stdout": "[{\"app\":{\"id\":15368},\"check_suite\":{\"id\":102552054061},\"completed_at\":\"2026-10-08T22:09:24Z\",\"conclusion\":\"success\",\"name\":\"test\",\"started_at\":\"2026-10-08T22:02:19Z\",\"status\":\"completed\"},{\"app\":{\"id\":15368},\"check_suite\":{\"id\":102552137693},\"completed_at\":\"2026-10-08T21:57:30Z\",\"conclusion\":\"success\",\"name\":\"copilot-pull-request-reviewer\",\"started_at\":\"2026-10-08T21:55:00Z\",\"status\":\"completed\"},{\"app\":{\"id\":15368},\"check_suite\":{\"id\":102552064689},\"completed_at\":\"2026-10-08T22:01:14Z\",\"conclusion\":\"success\",\"name\":\"test\",\"started_at\":\"2026-10-08T21:54:42Z\",\"status\":\"completed\"},{\"app\":{\"id\":15368},\"check_suite\":{\"id\":102552054061},\"completed_at\":\"2026-10-08T22:01:47Z\",\"conclusion\":\"failure\",\"name\":\"test\",\"started_at\":\"2026-10-08T21:54:40Z\",\"status\":\"completed\"}]\n", + "stderr": "" + } + ] +} diff --git a/test/fixtures/gh/private-app-suites.json b/test/fixtures/gh/private-app-suites.json new file mode 100644 index 00000000..acc2242f --- /dev/null +++ b/test/fixtures/gh/private-app-suites.json @@ -0,0 +1,36 @@ +{ + "gh": "gh version 2.102.0 (2026-09-30)", + "repository": "sveltejs/svelte", + "pullRequest": 19050, + "recordedAt": "2026-10-21T02:15:18.764Z", + "calls": [ + { + "args": [ + "api", + "repos/sveltejs/svelte/commits/111cc6411c9c647f691a159862da2b74212791e3/check-suites", + "--jq", + "[.check_suites[] | {id, app: {id: .app.id, slug: .app.slug}}]" + ], + "exitCode": 0, + "stdout": "[{\"app\":{\"id\":15368,\"slug\":\"github-actions\"},\"id\":106830892590},{\"app\":{\"id\":2935623,\"slug\":\"svite-ecosystem-ci\"},\"id\":106830896506},{\"app\":{\"id\":4226557,\"slug\":\"vercel-security-reviewer\"},\"id\":106830908878},{\"app\":{\"id\":8329,\"slug\":\"vercel\"},\"id\":106830910586}]\n", + "stderr": "" + }, + { + "args": [ + "api", + "graphql", + "-f", + "query=query($owner:String!,$repo:String!,$oid:GitObjectID!){repository(owner:$owner,name:$repo){object(oid:$oid){... on Commit{checkSuites(first:100){nodes{databaseId app{databaseId slug}}}}}}}", + "-f", + "owner=sveltejs", + "-f", + "repo=svelte", + "-f", + "oid=111cc6411c9c647f691a159862da2b74212791e3" + ], + "exitCode": 0, + "stdout": "{\"data\":{\"repository\":{\"object\":{\"checkSuites\":{\"nodes\":[{\"databaseId\":106830892590,\"app\":{\"databaseId\":15368,\"slug\":\"github-actions\"}},{\"databaseId\":106830896506,\"app\":null},{\"databaseId\":106830908878,\"app\":{\"databaseId\":4226557,\"slug\":\"vercel-security-reviewer\"}},{\"databaseId\":106830910586,\"app\":{\"databaseId\":8329,\"slug\":\"vercel\"}}]}}}}}", + "stderr": "" + } + ] +} diff --git a/test/merge-recorded.test.ts b/test/merge-recorded.test.ts index 1212f049..1978579b 100644 --- a/test/merge-recorded.test.ts +++ b/test/merge-recorded.test.ts @@ -55,7 +55,7 @@ it('reads the ruleset\'s required checks as pending, then as passing at the same // The same reads by a token without admin rights, recorded 2026-10-20 with gh 2.102.0 from public repositories this // token may only read. Each classic protection read answers 404 "Not Found", so classic checks come from the branch read. // nonadmin-ruleset and nonadmin-mixed were recorded again at the same heads on 2026-10-21 (UTC), when the inspection -// began to read the head's check runs with their apps for checks pinned to an app (#176); their other answers did not change. +// began to read the runs of checks pinned to an app from GitHub (#176); their other answers did not change. // - nonadmin-ruleset: vitejs/vite PR 23837 into main, which only a ruleset covers (`protection.enabled` false); // - nonadmin-classic: cli/cli PR 14716 into trunk, with classic required checks and no ruleset checks; // - nonadmin-mixed: sveltejs/svelte PR 19050 into main, where a ruleset and classic protection both require one check. @@ -83,9 +83,9 @@ it('records the 404 a token without admin rights gets, and the branch read\'s cl it('reads a ruleset-only branch\'s required checks with a token without admin rights', async () => { const run = replayGh(nonAdmin.ruleset), state = await gatewayFor(nonAdmin.ruleset, run).inspect({ fresh: true }); expect(run.unrecorded).toEqual([]); - // The ruleset binds each check to GitHub Actions (app 15368). gh's rollup names no app (#176), so they are matched from - // the head's check runs, where GitHub Actions reported a passing run of each (one of two runs of `Build & Test Passed - // or Skipped` was skipped, which passes). + // The ruleset binds each check to GitHub Actions (app 15368). gh's rollup names no app (#176), so they are read from + // GitHub by app and name, in one call; GitHub Actions reported a passing run of each (`Build & Test Passed or Skipped` + // ran in two suites, skipped in the older and passed in the newer). expect(run.calls.filter(args => args[1] === 'graphql')).toHaveLength(1); expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, mergeQueue: false, requiredChecks: [ { context: 'Semantic Pull Request', appId: 15368, state: 'success' }, @@ -107,14 +107,69 @@ it('reads a classic-protected branch\'s required checks with a token without adm it('reads a check both a ruleset and classic protection require once, with a token without admin rights', async () => { const run = replayGh(nonAdmin.mixed), state = await gatewayFor(nonAdmin.mixed, run).inspect({ fresh: true }); expect(run.unrecorded).toEqual([]); - // The head has a skipped `vite-ecosystem-ci` run, but GitHub names no app for its check suite to this reader, so the - // run cannot be shown to come from app 2935623, and the pinned check reads as missing: it fails closed. - const suites = (JSON.parse(nonAdmin.mixed.calls.find(call => call.args[1] === 'graphql')!.stdout) as - { data: { repository: { object: { checkSuites: { nodes: { app: { databaseId: number } | null; checkRuns: { nodes: { name: string; conclusion: string }[] } }[] } } } } }) - .data.repository.object.checkSuites.nodes; - expect(suites.filter(suite => suite.checkRuns.nodes.some(check => check.name === 'vite-ecosystem-ci'))) - .toEqual([{ app: null, checkRuns: { totalCount: 1, nodes: [expect.objectContaining({ name: 'vite-ecosystem-ci', conclusion: 'SKIPPED' })] } }]); - expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, requiredChecks: [{ context: 'vite-ecosystem-ci', appId: 2935623, state: 'missing' }] }); + // App 2935623 is private: GitHub's GraphQL API gives this reader `app: null` for its check suite, though REST names + // the app (private-app-suites.json, recorded 2026-10-21). Filtering suites by app ID on GitHub's side still returns + // that suite, with its skipped `vite-ecosystem-ci` run, so the check reads as passing (#176 review). + const privateApp = loadRecording('private-app-suites'); + const rest = JSON.parse(privateApp.calls.find(call => call.args[1]!.endsWith('/check-suites'))!.stdout) as { id: number; app: { id: number; slug: string } }[]; + const graphql = JSON.parse(privateApp.calls.find(call => call.args[1] === 'graphql')!.stdout) as + { data: { repository: { object: { checkSuites: { nodes: { databaseId: number; app: { databaseId: number } | null }[] } } } } }; + const suite = rest.find(entry => entry.app.id === 2935623)!; + expect(suite.app.slug).toBe('svite-ecosystem-ci'); + expect(graphql.data.repository.object.checkSuites.nodes.find(node => node.databaseId === suite.id)).toEqual({ databaseId: suite.id, app: null }); + const answer = JSON.parse(nonAdmin.mixed.calls.find(call => call.args[1] === 'graphql')!.stdout) as + { data: { repository: { object: { c0: { totalCount: number; nodes: { checkRuns: { nodes: { name: string; conclusion: string }[] } }[] } } } } }; + expect(answer.data.repository.object.c0).toMatchObject({ totalCount: 1, nodes: [{ checkRuns: { nodes: [{ name: 'vite-ecosystem-ci', conclusion: 'SKIPPED' }] } }] }); + expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, requiredChecks: [{ context: 'vite-ecosystem-ci', appId: 2935623, state: 'success' }] }); +}); + +// Pinned-check reads on public commits whose rules pin no check, recorded 2026-10-21 with gh 2.102.0 by a script that +// ran the gateway with the fake pull request and ruleset of `inspectPinned` below and sent only the pinned-check read to +// the real gh. Each file also holds a REST read that shows what makes the commit a case worth recording. +// - pinned-rerun: codeabovelab/codeboost commit e3697dd (PR 151's head). REST's list of every run (`filter=all`) shows +// suite 102552054061 ran `test` twice: failed at 21:54, passed when re-run at 22:02. +// - pinned-large-suite: microsoft/vscode commit b3aa5cb (PR 343099's head). REST counts 101 runs in GitHub Actions +// suite 107124344927, more than one page of runs holds. +const pinnedRecordings = { rerun: loadRecording('pinned-rerun'), largeSuite: loadRecording('pinned-large-suite') }; +/** Inspects `recording`'s head with `context` pinned to `appId`; only the pinned-check read is answered from the recording. */ +async function inspectPinned(recording: RecordedGh, head: string, context: string, appId: number) { + const replay = replayGh(recording); + const run: RunGh = async (args, options) => { + const joined = args.join(' '); + if (joined.startsWith('pr view')) return JSON.stringify({ number: recording.pullRequest, isDraft: false, baseRefName: 'main', baseRefOid: '0'.repeat(40), headRefName: 'x', headRefOid: head, state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [] }); + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context, integration_id: appId }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (joined.endsWith('/protection')) throw new Error('HTTP 404: Not Found'); + return replay(args, options); + }; + const state = await new GhMergeGateway({ repository: recording.repository, pullRequest: recording.pullRequest, issue: 1 }, run, clearCheck(recording.repository)).inspect({ fresh: true }); + expect(replay.unrecorded).toEqual([]); + expect(replay.calls.filter(args => args[1] === 'graphql')).toHaveLength(1); + return state; +} +type PinnedAnswer = { data: { repository: { object: { c0: { nodes: { checkRuns: { totalCount: number; nodes: { name: string; conclusion: string }[] } }[] } } } } }; +const pinnedAnswer = (recording: RecordedGh) => JSON.parse(recording.calls.find(call => call.args[1] === 'graphql')!.stdout) as PinnedAnswer; + +it('reads only the latest run of a pinned check inside a suite that re-ran it', async () => { + const { rerun } = pinnedRecordings; + expect(rerun.gh).toBe('gh version 2.102.0 (2026-09-30)'); + const every = JSON.parse(rerun.calls.find(call => call.args[1]!.includes('filter=all'))!.stdout) as { name: string; conclusion: string; check_suite: { id: number } }[]; + expect(every.filter(check => check.name === 'test' && check.check_suite.id === 102552054061).map(check => check.conclusion).sort()).toEqual(['failure', 'success']); + // The read asks for each suite's latest run of a name, and GitHub leaves the older failure out. + expect(pinnedAnswer(rerun).data.repository.object.c0.nodes.flatMap(suite => suite.checkRuns.nodes.map(check => check.conclusion))).toEqual(['SUCCESS', 'SUCCESS']); + expect((await inspectPinned(rerun, 'e3697dd532643de0f831b678856b2d677338231d', 'test', 15368)).requiredChecks).toEqual([{ context: 'test', appId: 15368, state: 'success' }]); +}); + +it('reads a pinned check from an app whose suite has more than 100 runs', async () => { + const { largeSuite } = pinnedRecordings; + expect(largeSuite.gh).toBe('gh version 2.102.0 (2026-09-30)'); + const count = JSON.parse(largeSuite.calls.find(call => call.args[1]!.includes('/check-suites/'))!.stdout) as { total_count: number; app: { id: number }; check_suite: { id: number; head_sha: string } }; + expect(count).toEqual({ total_count: 101, app: { id: 15368 }, check_suite: { id: 107124344927, head_sha: 'b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f' } }); + // GitHub filters the runs by name, so that suite answers with only the one run asked for. + const suites = pinnedAnswer(largeSuite).data.repository.object.c0.nodes; + expect(suites.map(suite => suite.checkRuns.totalCount).sort()).toEqual([0, 0, 0, 0, 1]); + expect((await inspectPinned(largeSuite, 'b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f', 'Detect Changes', 15368)).requiredChecks) + .toEqual([{ context: 'Detect Changes', appId: 15368, state: 'success' }]); }); // The job, against the real Store and guarded merge, with GitHub answered from the recordings. Its preparation is a diff --git a/test/merge-stop-reasons.test.ts b/test/merge-stop-reasons.test.ts index f19350b1..1ab49d95 100644 --- a/test/merge-stop-reasons.test.ts +++ b/test/merge-stop-reasons.test.ts @@ -483,3 +483,33 @@ it('merge-state inspection answers a caller abort, not the already-fixed check\' // Not the check's gh text: a blocker result that arrives after the abort is discarded, so the click may be resent. expect(await inspection).toBe(reason); }); + +it('merge-state inspection reports the first stage\'s gh exit when a stopped pinned-check read then settles with its own gh exit', async () => { + // A published target, so the branch's PR list runs beside the rule reads. The rules pin `test` to app 10, so the rule + // stage ends with the pinned-check read (#176). The list's gh exit comes first and stops that read; gh then exits on + // its own with another error while it is stopping, which runWithInput reports instead of the abort (#48). + const first = new CommandFailed('gh failed (exit 1): gh: Bad credentials (HTTP 401)', 'gh: Bad credentials (HTTP 401)', '{"message":"Bad credentials","status":"401"}'); + const later = new CommandFailed('gh failed (exit 1): gh: Something went wrong while executing your query.', 'gh: Something went wrong while executing your query.', ''); + const pinnedStarted = deferred(); + let pinnedStopReason: unknown; + const run: Run = async (args, options) => { + const joined = args.join(' '); + if (joined.startsWith('pr view')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'task', + headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [], body: '', isCrossRepository: false }); + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, + required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (joined.endsWith('/protection')) throw new CommandFailed('gh failed (exit 1): gh: Branch not protected (HTTP 404)', 'gh: Branch not protected (HTTP 404)', '{"message":"Branch not protected","status":"404"}'); + if (args[1] === 'graphql' && joined.includes('checkSuites(')) return new Promise((_, reject) => { + pinnedStarted.resolve(); + options?.signal?.addEventListener('abort', () => { pinnedStopReason = options.signal!.reason; setTimeout(() => reject(later), 20); }, { once: true }); + }); + if (joined.includes('/pulls?')) { await pinnedStarted.promise; throw first; } + throw new Error(`Unexpected gh call: ${joined}`); + }; + const checks: AlreadyFixedGateway = { repository: 'owner/repo', check: async () => ({ outcome: 'clear', baseHead: sha('9') }) }; + const error = await new GhMergeGateway(config, run, checks).inspect({ fresh: true, target: { pullRequest: 7, headBranch: 'task' } }) + .then(() => null, (reason: unknown) => reason); + expect(pinnedStopReason).toBe(first); // The interleaving: the list's exit stopped the pinned-check read. + expect(error).toBe(first); +}); diff --git a/test/merge.test.ts b/test/merge.test.ts index a24df5f4..fb44db4e 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -37,23 +37,40 @@ function mergeReads(args: readonly string[], pull: Record = {}) throw new Error(`Unexpected gh call: ${joined}`); } /** - * The gateway's read of the head's check runs with their apps (`gh api graphql`, github/merge.ts `#checkRuns`). gh's + * The gateway's read of the runs of each pinned check (`gh api graphql`, github/merge.ts `#pinnedRuns`). gh's * statusCheckRollup names no app (#176), so this is the read a required check pinned to an app is matched from. */ const isCheckRunRead = (args: readonly string[]) => args[0] === 'api' && args[1] === 'graphql' && args.some(arg => arg.startsWith('query=') && arg.includes('checkSuites(')); +/** A check suite on the head: the app that reported it and its latest runs (GitHub's `checkType: LATEST`). */ +type Suite = { app: number; runs: Array> }; +/** Each pinned check the call asks for: its alias index, app, name and cursor, from the call's `-F appN`, `-f nameN`, `-f afterN`. */ +function pinnedVariables(args: readonly string[]): Array<{ index: number; app: number; name: string; after: string | null }> { + const value = (flag: string, key: string) => { + for (let i = 0; i < args.length - 1; i++) if (args[i] === flag && args[i + 1]!.startsWith(`${key}=`)) return args[i + 1]!.slice(key.length + 1); + return undefined; + }; + const indexes = args.flatMap((arg, i) => args[i - 1] === '-F' ? [/^app(\d+)=/.exec(arg)?.[1]].filter(Boolean) as string[] : []); + return indexes.map(index => ({ index: Number(index), app: Number(value('-F', `app${index}`)), name: value('-f', `name${index}`)!, after: value('-f', `after${index}`) ?? null })); +} /** - * gh 2.102.0's answer to that read, as recorded in test/fixtures/gh/nonadmin-ruleset.json and nonadmin-mixed.json: exit 0 - * with GitHub's response on stdout, one entry per check suite with its app (`null` where GitHub names none, as for one - * suite in nonadmin-mixed.json) and its runs. The fake gives every field the query asks for; `page` overrides the - * suites' count and page info for the paging tests. A failed read is a CommandFailed (`ghExit`, below). + * gh 2.102.0's answer to that read: exit 0 with GitHub's response on stdout, in the shape recorded in + * test/fixtures/gh/nonadmin-ruleset.json, nonadmin-mixed.json and pinned-rerun.json. As GitHub does, it keeps per alias + * only the suites of the asked app and, in each, only the runs of the asked name, in one page (`page` overrides an + * alias's count and page info, for the paging tests). It does not model a private app: the query does not ask for a + * suite's app, and GitHub's app filter returns such a suite all the same (nonadmin-mixed.json). A failed read is a + * CommandFailed (`ghExit`, below). */ -function checkRunsAnswer(head: string, suites: Array<{ app: number | null; runs: Array> }>, - page: { totalCount?: number; hasNextPage?: boolean; endCursor?: string | null } = {}): string { - return JSON.stringify({ data: { repository: { object: { __typename: 'Commit', oid: head, checkSuites: { - totalCount: page.totalCount ?? suites.length, - pageInfo: { hasNextPage: page.hasNextPage ?? false, endCursor: page.endCursor !== undefined ? page.endCursor : suites.length ? 'Y3Vyc29yOnYyOpHPAAAAGN-eVHo=' : null }, - nodes: suites.map(suite => ({ app: suite.app === null ? null : { databaseId: suite.app }, checkRuns: { totalCount: suite.runs.length, nodes: suite.runs } })), - } } } } }); +function checkRunsAnswer(args: readonly string[], suites: readonly Suite[], options: { head?: string; page?: (alias: { app: number; name: string; after: string | null }) => { totalCount?: number; hasNextPage?: boolean; endCursor?: string | null } } = {}): string { + const aliases = Object.fromEntries(pinnedVariables(args).map(alias => { + const nodes = suites.filter(suite => suite.app === alias.app).map(suite => { + const runs = suite.runs.filter(run => run.name === alias.name); + return { checkRuns: { totalCount: runs.length, nodes: runs } }; + }); + const page = options.page?.(alias) ?? {}; + return [`c${alias.index}`, { totalCount: page.totalCount ?? nodes.length, + pageInfo: { hasNextPage: page.hasNextPage ?? false, endCursor: page.endCursor !== undefined ? page.endCursor : nodes.length ? 'Y3Vyc29yOnYyOpHPAAAAGN-eHXo=' : null }, nodes }]; + })); + return JSON.stringify({ data: { repository: { object: { __typename: 'Commit', oid: options.head ?? sha('b'), ...aliases } } } }); } /** One check run as that read gives it: GitHub's GraphQL API answers `null`, not gh's zero time, for a time not reached. */ const checkRun = (name: string, status: string, conclusion: string | null, startedAt: string | null = '2026-10-11T00:00:00Z', completedAt: string | null = status === 'COMPLETED' ? '2026-10-11T00:01:00Z' : null) => @@ -941,7 +958,7 @@ it('parses required checks from both rule sources and pins the gh merge head', a { name: 'race', status: 'IN_PROGRESS', conclusion: 'SUCCESS' }, { context: 'lint', state: 'SUCCESS' }, ] }); } - if (isCheckRunRead(args)) return checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); + if (isCheckRunRead(args)) return checkRunsAnswer(args, [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'merge_queue' }, { type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: 10 }, { context: 'race', integration_id: null }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); if (joined.includes('/protection')) return JSON.stringify({ required_status_checks: { strict: false, checks: [{ context: 'lint', app_id: null }] } }); @@ -1072,14 +1089,14 @@ it('ignores required reviews in both rule sources and reads only their required /** * Inspects a pull request whose one required check is `test`, from any app or, with `appId`, from that app, and whose head - * has these observed runs and statuses in gh's rollup. `checkRuns` answers the read of the head's check runs with their - * apps (a string, or a function of the call's arguments that returns one or throws); without it that read is unexpected. + * has these observed runs and statuses in gh's rollup. `checkRuns` answers the read of a pinned check's runs: the head's + * check suites, or a function of the call's arguments that answers or throws; without it that read is unexpected. */ -async function requiredTestState(statusCheckRollup: unknown[], appId: number | null = null, checkRuns?: string | ((args: readonly string[]) => string | Promise)) { +async function requiredTestState(statusCheckRollup: unknown[], appId: number | null = null, checkRuns?: readonly Suite[] | ((args: readonly string[]) => string | Promise)) { const run = async (args: readonly string[]) => { const joined = args.join(' '); if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup }); - if (checkRuns !== undefined && isCheckRunRead(args)) return typeof checkRuns === 'string' ? checkRuns : checkRuns(args); + if (checkRuns !== undefined && isCheckRunRead(args)) return typeof checkRuns === 'function' ? checkRuns(args) : checkRunsAnswer(args, checkRuns); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: appId }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); if (joined.endsWith('/protection')) throw new Error('gh: Branch not protected (HTTP 404)'); @@ -1231,11 +1248,10 @@ it.each([ }); // #176: gh's rollup never carries a check run's app (test/fixtures/gh/, gh 2.102.0), so a requirement pinned to an app is -// matched from the head's check runs, read with their apps. Each rollup below holds an app-less passing `test` run, as -// gh reports one; before #176 every requirement here read as missing. +// matched from the runs GitHub lists for that app and name on the head. Each rollup below holds an app-less passing +// `test` run, as gh reports one; before #176 every requirement here read as missing. const appLessPass = [{ __typename: 'CheckRun', name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', startedAt: '2026-10-20T00:00:00Z', completedAt: '2026-10-20T00:01:00Z', workflowName: 'CI', detailsUrl: 'https://github.com/owner/repo/actions/runs/1/job/2' }]; -type Suite = { app: number | null; runs: Array> }; it.each([ ['passed', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], 'success'], @@ -1244,53 +1260,120 @@ it.each([ ['is queued and has not started', [{ app: 10, runs: [checkRun('test', 'QUEUED', null, null)] }], 'pending'], ['failed', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }], 'failure'], ['passed, while another app\'s run of that name failed', [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }, { app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], 'success'], + ['passed, in a suite where another check has more than 100 runs', [{ app: 10, runs: [...Array.from({ length: 101 }, (_, n) => checkRun(`job ${n}`, 'COMPLETED', 'FAILURE')), checkRun('test', 'COMPLETED', 'SUCCESS')] }], 'success'], ['was re-run in a new suite that is in progress', [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'FAILURE')] }, { app: 10, runs: [checkRun('test', 'IN_PROGRESS', null, '2026-10-11T00:05:00Z')] }], 'pending'], ] as const)('reads a required check pinned to an app from that app\'s run on the head, which %s', async (_case, suites, expected) => { - expect(await requiredTestState(appLessPass, 10, checkRunsAnswer(sha('b'), suites as unknown as Suite[]))).toEqual([expected]); + expect(await requiredTestState(appLessPass, 10, suites as unknown as Suite[])).toEqual([expected]); }); it.each([ ['a different app', [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]], - // As for one suite in nonadmin-mixed.json: GitHub names no app, so the run cannot be shown to be the pinned app's. - ['a suite GitHub names no app for', [{ app: null, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]], ['nowhere: the head has no check suites', []], - ['the pinned app, under another name', [{ app: 10, runs: [checkRun('tests', 'COMPLETED', 'SUCCESS')] }]], + ['the pinned app, under another name', [{ app: 10, runs: [checkRun('tests', 'COMPLETED', 'SUCCESS'), checkRun('Test', 'COMPLETED', 'SUCCESS')] }]], ] as const)('reads a required check pinned to an app as missing when its only passing run is from %s', async (_case, suites) => { - expect(await requiredTestState(appLessPass, 10, checkRunsAnswer(sha('b'), suites as unknown as Suite[]))).toEqual(['missing']); + expect(await requiredTestState(appLessPass, 10, suites as unknown as Suite[])).toEqual(['missing']); }); it('matches a requirement pinned to an app only against that app\'s check runs, never a commit status', async () => { - expect(await requiredTestState([{ context: 'test', state: 'SUCCESS', startedAt: '2026-10-11T00:05:00Z' }], 10, - checkRunsAnswer(sha('b'), []))).toEqual(['missing']); + expect(await requiredTestState([{ context: 'test', state: 'SUCCESS', startedAt: '2026-10-11T00:05:00Z' }], 10, [])).toEqual(['missing']); expect(await requiredTestState([{ context: 'test', state: 'FAILURE', startedAt: '2026-10-11T00:05:00Z' }], 10, - checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]))).toEqual(['success']); + [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }])).toEqual(['success']); }); -it('reads the check runs of the exact head the pull request read returned, and only for a pinned check', async () => { +it('asks GitHub for each pinned check\'s app and name on the exact head the pull request read returned, and only for a pinned check', async () => { const reads: string[][] = []; - expect(await requiredTestState(appLessPass, 10, args => { reads.push([...args]); return checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); })).toEqual(['success']); + expect(await requiredTestState(appLessPass, 10, args => { reads.push([...args]); return checkRunsAnswer(args, [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]); })).toEqual(['success']); expect(reads).toHaveLength(1); expect(reads[0]!.slice(0, 3)).toEqual(['api', 'graphql', '-f']); - expect(reads[0]!.slice(4)).toEqual(['-f', 'owner=owner', '-f', 'name=repo', '-f', `oid=${sha('b')}`]); - // A check any app may report is read from the rollup alone: here gh has no answer for the check-run read. + expect(reads[0]![3]).toMatch(/c0:checkSuites\(first:100,after:\$after0,filterBy:\{appId:\$app0\}\).*checkRuns\(first:100,filterBy:\{checkName:\$name0,checkType:LATEST\}\)/); + expect(reads[0]!.slice(4)).toEqual(['-f', 'owner=owner', '-f', 'repo=repo', '-f', `oid=${sha('b')}`, '-F', 'app0=10', '-f', 'name0=test']); + // A check any app may report is read from the rollup alone: here gh has no answer for the pinned-check read. expect(await requiredTestState(appLessPass, null)).toEqual(['success']); }); -it('reads every page of check suites on the head', async () => { - const pages: (string | null)[] = []; - const state = await requiredTestState(appLessPass, 10, args => { - const after = args.find(arg => arg.startsWith('after='))?.slice('after='.length) ?? null; - pages.push(after); - return after === null - ? checkRunsAnswer(sha('b'), [{ app: 11, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }], { totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }) - : checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'IN_PROGRESS', null)] }], { totalCount: 2, hasNextPage: false, endCursor: 'PAGE_END' }); - }); - expect(pages).toEqual([null, 'PAGE_2']); - expect(state).toEqual(['pending']); +it('reads every page of an app\'s check suites on the head, and up to the page limit exactly', async () => { + for (const last of [2, 10]) { + const pages: (string | null)[] = []; + const state = await requiredTestState(appLessPass, 10, args => { + const after = pinnedVariables(args)[0]!.after, page = after === null ? 1 : Number(after.slice('PAGE_'.length)); + pages.push(after); + // Each page holds one suite; the last one has the pending run. + return checkRunsAnswer(args, [{ app: 10, runs: [page === last ? checkRun('test', 'IN_PROGRESS', null) : checkRun('test', 'COMPLETED', 'SUCCESS')] }], + { page: () => ({ totalCount: last, hasNextPage: page < last, endCursor: `PAGE_${page + 1}` }) }); + }); + expect(pages).toEqual([null, ...Array.from({ length: last - 1 }, (_, n) => `PAGE_${n + 2}`)]); + expect(state).toEqual(['pending']); + } +}); + +it('reads several pinned checks in one call, and pages only the check whose app has more suites', async () => { + const reads: ReturnType[] = []; + const run = async (args: readonly string[]) => { + const joined = args.join(' '); + if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [] }); + if (isCheckRunRead(args)) { + const asked = pinnedVariables(args); reads.push(asked); + const second = asked.some(alias => alias.after !== null); + return checkRunsAnswer(args, second ? [{ app: 11, runs: [checkRun('lint', 'COMPLETED', 'SUCCESS')] }] + : [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }, { app: 11, runs: [checkRun('lint', 'COMPLETED', 'FAILURE', '2026-10-10T00:00:00Z')] }], + { page: alias => alias.app === 11 ? { totalCount: 2, hasNextPage: !second, endCursor: 'PAGE_2' } : {} }); + } + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, + required_status_checks: [{ context: 'test', integration_id: 10 }, { context: 'lint', integration_id: 11 }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (joined.endsWith('/protection')) throw NOT_PROTECTED; + { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } + throw new Error(`Unexpected gh call: ${joined}`); + }; + const state = await new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21 }, run).inspect(); + expect(reads).toEqual([[{ index: 0, app: 10, name: 'test', after: null }, { index: 1, app: 11, name: 'lint', after: null }], [{ index: 1, app: 11, name: 'lint', after: 'PAGE_2' }]]); + // The newer `lint` run, on the second page, passed. + expect(state.requiredChecks).toEqual([{ context: 'test', appId: 10, state: 'success' }, { context: 'lint', appId: 11, state: 'success' }]); +}); + +it('reads more pinned checks than one call holds in several calls', async () => { + const contexts = Array.from({ length: 30 }, (_, n) => `job ${n}`), calls: number[] = []; + const run = async (args: readonly string[]) => { + const joined = args.join(' '); + if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [] }); + if (isCheckRunRead(args)) { calls.push(pinnedVariables(args).length); return checkRunsAnswer(args, [{ app: 10, runs: contexts.map(context => checkRun(context, 'COMPLETED', 'SUCCESS')) }]); } + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, + required_status_checks: contexts.map(context => ({ context, integration_id: 10 })) } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (joined.endsWith('/protection')) throw NOT_PROTECTED; + { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } + throw new Error(`Unexpected gh call: ${joined}`); + }; + const state = await new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21 }, run).inspect(); + expect(calls).toEqual([25, 5]); + expect(state.requiredChecks.map(check => check.state)).toEqual(Array(30).fill('success')); +}); + +it('does not read pinned checks when the rules are unknown, so the rules blocker stays', async () => { + let reads = 0; + const run = async (args: readonly string[]) => { + const joined = args.join(' '); + if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: appLessPass }); + if (isCheckRunRead(args)) { reads++; throw ghExit('gh: Something went wrong while executing your query. This may be the result of a timeout, or it could be a GitHub bug.', ''); } + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); + // Classic protection whose `strict` is malformed: the rules are unknown, after the ruleset's pinned check was read. + if (joined.endsWith('/protection')) return JSON.stringify({ required_status_checks: { strict: 'yes', checks: [] } }); + { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } + throw new Error(`Unexpected gh call: ${joined}`); + }; + const gateway = new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21 }, run); + const state = await gateway.inspect({ fresh: true }); + expect(state).toMatchObject({ rulesKnown: false, requiredChecks: [{ context: 'test', appId: 10, state: 'missing' }] }); + expect(reads).toBe(0); + const view = readyView(); + const status = await new MergeCoordinator(serviceFor(view), gateway).status(view, true); + expect(status.blockers).toContainEqual({ code: 'rules', message: 'Required branch checks could not be read.' }); + expect(status.blockers.some(blocker => blocker.code === 'github')).toBe(false); }); -/** A gateway for a pull request requiring `test` from app 10, where `answer` answers the read of the head's check runs. */ +/** A gateway for a pull request requiring `test` from app 10, where `answer` answers the read of its runs. */ function pinnedGateway(answer: (args: readonly string[], signal?: AbortSignal) => Promise) { const run = async (args: readonly string[], runOptions?: { signal?: AbortSignal }) => { const joined = args.join(' '); @@ -1305,33 +1388,34 @@ function pinnedGateway(answer: (args: readonly string[], signal?: AbortSignal) = return new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21 }, run); } -// gh 2.102.0 exits 1 when GitHub's GraphQL answer has errors (stderr `gh: `, the response on stdout), so a -// response with errors on exit 0 is not what gh prints; it is kept as a case because the gateway checks for it. +const passing: Suite[] = [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]; const failedCheckRunReads: Array<[string, (args: readonly string[]) => Promise, RegExp]> = [ + // As gh 2.102.0 answers a GraphQL error (checked against GitHub): exit 1, `gh: ` on stderr, the response on stdout. ['gh exits with GitHub\'s error', async () => { throw ghExit('gh: Could not resolve to a Repository with the name \'owner/repo\'.', '{"data":{"repository":null},"errors":[{"type":"NOT_FOUND","path":["repository"],"locations":[{"line":1,"column":69}],"message":"Could not resolve to a Repository with the name \'owner/repo\'."}]}'); }, /^gh failed \(exit 1\): gh: Could not resolve to a Repository/], - ['GitHub answers with errors', async () => JSON.stringify({ data: null, errors: [{ message: 'Timeout on validation of query' }] }), /^GitHub returned check runs with errors\.$/], - // GitHub's answer for a commit it does not have (recorded with gh 2.102.0: exit 0). + // Defensive: gh 2.102.0 exits 1 on such an answer (the case above), so it never prints one on exit 0. + ['GitHub answers with errors on exit 0 (defensive)', async () => JSON.stringify({ data: null, errors: [{ message: 'Timeout on validation of query' }] }), /^GitHub returned check runs with errors\.$/], + // GitHub's answer for a commit it does not have (checked with gh 2.102.0: exit 0). ['GitHub does not find the head', async () => JSON.stringify({ data: { repository: { object: null } } }), /^GitHub returned no check runs for the pull request head\.$/], - ['GitHub answers for another commit', async () => checkRunsAnswer(sha('c'), []), /^GitHub returned no check runs for the pull request head\.$/], - ['a suite has more runs than one page holds', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }]) - .replace('"checkRuns":{"totalCount":1', '"checkRuns":{"totalCount":101'), /more check runs than codeboost can read/], - ['a run has an unknown status', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [checkRun('test', 'DONE', 'SUCCESS')] }]), /^GitHub returned a malformed check run\.$/], - ['a suite\'s app has no ID', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }]).replace('{"databaseId":10}', '{}'), /^GitHub returned incomplete check runs\.$/], - ['the suites change between pages', async args => args.some(arg => arg.startsWith('after=')) - ? checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }], { totalCount: 3 }) - : checkRunsAnswer(sha('b'), [{ app: 11, runs: [] }], { totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }), /changed while they were read/], - ['the pages hold fewer suites than GitHub counts', async () => checkRunsAnswer(sha('b'), [{ app: 10, runs: [] }], { totalCount: 2 }), /changed while they were read/], - ['the head has more than 1,000 check suites', async () => checkRunsAnswer(sha('b'), [{ app: 11, runs: [] }], { totalCount: 1001, hasNextPage: true, endCursor: 'NEXT' }), - /more check suites than codeboost can read/], + ['GitHub answers for another commit', async args => checkRunsAnswer(args, passing, { head: sha('c') }), /^GitHub returned no check runs for the pull request head\.$/], + ['the answer leaves out the check', async args => checkRunsAnswer(args, passing).replace('"c0":', '"c9":'), /^GitHub returned incomplete check runs\.$/], + ['a suite has more runs of the check\'s name than one page holds', async args => checkRunsAnswer(args, passing).replace('"checkRuns":{"totalCount":1', '"checkRuns":{"totalCount":101'), + /more runs named "test" than codeboost can read/], + ['a run has an unknown status', async args => checkRunsAnswer(args, [{ app: 10, runs: [checkRun('test', 'DONE', 'SUCCESS')] }]), /^GitHub returned a malformed check run\.$/], + ['the suites change between pages', async args => pinnedVariables(args)[0]!.after !== null + ? checkRunsAnswer(args, passing, { page: () => ({ totalCount: 3 }) }) + : checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }) }), /changed while they were read/], + ['the pages hold fewer suites than GitHub counts', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2 }) }), /changed while they were read/], + ['the app has more than 1,000 check suites on the head', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 1001, hasNextPage: true, endCursor: 'NEXT' }) }), + /more check suites from app 10 than codeboost can read/], ]; -it.each(failedCheckRunReads)('fails the inspection with its reason when the read of the pinned check\'s app fails: %s', async (_case, answer, reason) => { +it.each(failedCheckRunReads)('fails the inspection with its reason when the read of the pinned check fails: %s', async (_case, answer, reason) => { await expect(pinnedGateway(answer).inspect({ fresh: true })).rejects.toThrow(reason); }); -it.each(failedCheckRunReads)('blocks the merge with the reason when the read of the pinned check\'s app fails: %s', async (_case, answer, reason) => { +it.each(failedCheckRunReads)('blocks the merge with the reason when the read of the pinned check fails: %s', async (_case, answer, reason) => { const view = readyView(); const status = await new MergeCoordinator(serviceFor(view), pinnedGateway(answer)).displayStatus(view); expect(status).toMatchObject({ ready: false, action: null, remote: null }); @@ -1341,7 +1425,7 @@ it.each(failedCheckRunReads)('blocks the merge with the reason when the read of expect(status.blockers[0]!.message.replace(/^Could not read GitHub merge state\. /, '')).toMatch(reason); }); -it('keeps the inspection\'s timeout as the reason when the read of the pinned check\'s app outlasts it', async () => { +it('keeps the inspection\'s timeout as the reason when the read of the pinned check outlasts it', async () => { let stopped: unknown = null; const gateway = pinnedGateway((_args, signal) => new Promise((_resolve, reject) => { signal!.addEventListener('abort', () => { stopped = signal!.reason; reject(new Error('gh was stopped')); }, { once: true }); @@ -1355,7 +1439,7 @@ it('keeps the inspection\'s timeout as the reason when the read of the pinned ch it('is ready to merge when the app-pinned required check passed on the head, and not when another app reported it', async () => { for (const [app, ready] of [[10, true], [11, false]] as const) { const view = readyView(); - const gateway = pinnedGateway(async () => checkRunsAnswer(sha('b'), [{ app, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }])); + const gateway = pinnedGateway(async args => checkRunsAnswer(args, [{ app, runs: [checkRun('test', 'COMPLETED', 'SUCCESS')] }])); const status = await new MergeCoordinator(serviceFor(view), gateway).status(view, true); expect(status.remote.requiredChecks).toEqual([{ context: 'test', appId: 10, state: ready ? 'success' : 'missing' }]); expect(status.ready).toBe(ready); From 429d9c25af0c176460e5f6bdc007c5bb5f8590c2 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 19:49:47 -0700 Subject: [PATCH 3/9] Match test fakes to real gh and document the pinned-check read (#176 review) Round 2 of the independent review of PR #195 found five low issues: - The fakes in inspectPinned, the stop-reason test and requiredTestState now answer as gh 2.102.0 does: the protection read's 404 as a CommandFailed with GitHub's body on stdout, the branch read with its protection, and a GraphQL error with GitHub's response on stdout. Each notes the fields it leaves out and that the gateway does not read them. - scripts/record-pinned-gh.ts remakes pinned-rerun, pinned-large-suite and private-app-suites, and recorded.ts says which script writes which files and what each holds. Remade today, vscode's suite now has 116 runs (101 earlier); the test checks the new count. - The docs and the gateway's comment say the inspection's deadline, not the 10-page cap, is the real limit on paging: a head with more suites than fit fails closed with the inspection's timeout. - guarded-merge.md's long Gate paragraph is split, and the pinned-check read has its own short section. The T12 evidence names the removed test as removed and points to its replacement; build step 4's status lines are left for the refresh after the parallel PRs merge. Co-Authored-By: Claude Opus 5.5 --- docs/designs/codeboost-plan-indexed-review.md | 2 +- docs/implementation/guarded-merge.md | 46 ++++++++++- docs/implementation/merge-handoff.md | 2 +- github/merge.ts | 4 +- scripts/record-pinned-gh.ts | 77 +++++++++++++++++++ test/fixtures/gh/pinned-large-suite.json | 4 +- test/fixtures/gh/pinned-rerun.json | 2 +- test/fixtures/gh/private-app-suites.json | 2 +- test/fixtures/gh/recorded.ts | 15 ++-- test/merge-recorded.test.ts | 31 +++++--- test/merge-stop-reasons.test.ts | 7 +- test/merge.test.ts | 9 ++- 12 files changed, 175 insertions(+), 26 deletions(-) create mode 100644 scripts/record-pinned-gh.ts diff --git a/docs/designs/codeboost-plan-indexed-review.md b/docs/designs/codeboost-plan-indexed-review.md index c2d566ad..a43b4495 100644 --- a/docs/designs/codeboost-plan-indexed-review.md +++ b/docs/designs/codeboost-plan-indexed-review.md @@ -2043,7 +2043,7 @@ These `T` IDs are requirement identifiers, not the build-order numbers. Build st - Surfaced by: R6 (D7: A) - Files: github/checks - Verify: recorded-output tests for zero checks, pending then passing, and timeout - - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs"; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). *Later (#176):* for a check pinned to an app, the inspection now asks GitHub, through `gh api graphql`, for that app's runs of that name on the head, since gh's rollup names no app; the vite and svelte recordings were made again with that read, and their pinned checks read as passing, svelte's from a private app (`test/merge-recorded.test.ts`; `test/merge.test.ts`, "reads a required check pinned to an app from that app's run on the head, which %s"). The test named above was replaced. + - Evidence (2026-10-20): `test/merge-recorded.test.ts` replays GitHub's real answers, recorded with gh 2.102.0 from this repository by `scripts/record-merge-gh.ts` (`test/fixtures/gh/`). Zero checks: pull request 104, into a branch no rule covers, reads as an empty required set with the rules known. Pending then passing: pull request 183 at one head, read while `test` and `real-docker` ran and after they passed. Through the merge job, the wait sees both checks pending, then merges once they pass; at the time limit with the checks still pending, the job ends `blocked`. For the job, the recorded ruleset is changed in one field, its strict policy, because the repository's ruleset is not strict, so a job against it stops at once for the missing atomic base guard (T7), which is also tested ("ends the job for a person, before waiting, against the repository's real non-strict ruleset"). Zero checks pass a job only with an atomic base guard, and with no required checks only a merge queue gives one (a strict policy guards only a non-empty set). That job case is tested with injected states: a merge queue and no required checks (`test/merge-job.test.ts`, "ends queued when the merge queue accepted the head"). The recordings found two bugs, fixed in PR #183: a ruleset check with no `integration_id` and a ruleset-only branch's 404 "Branch not protected" both made the rules unknown, so a branch whose required checks come only from a ruleset (this repository's `main` among them) blocked merging. The 404 was recorded with a token with admin rights; a token without them may get "Not Found" instead, which still blocks merging as unknown rules. Reviews are not checks: required approving reviews in a ruleset or classic protection are ignored, and only their required checks count (`test/merge.test.ts`, "ignores required reviews in both rule sources and reads only their required checks"). #176 stays open and is tracked under build step 4, not here: a required check pinned to an app always reads as missing, which fails closed (the job blocks at its time limit) rather than merging without the check (`test/merge.test.ts`, "reads a requirement pinned to an app as missing against gh's app-less check runs", a test the #176 fix removed; a missing check blocks the job at its time limit: `test/merge-job.test.ts`, "ends approved, merge blocked when required checks do not pass in time"). The PR #183 recordings have no app-pinned check. *Later:* a token without admin rights does get "Not Found", on ruleset-only and classic-protected branches alike; the rules are now read for it, with classic checks from the branch read, replayed from recordings of vitejs/vite, cli/cli and sveltejs/svelte (`guarded-merge.md`). The vite and svelte recordings have app-pinned checks, which read as missing (#176). *Later (#176):* for a check pinned to an app, the inspection now asks GitHub, through `gh api graphql`, for that app's runs of that name on the head, since gh's rollup names no app; the vite and svelte recordings were made again with that read, and their pinned checks read as passing, svelte's from a private app (`test/merge-recorded.test.ts`; `test/merge.test.ts`, "reads a required check pinned to an app from that app's run on the head, which %s", which replaces the removed test). - [x] **T13 (P2, human: ~1 hour / CC: ~5 min)** — runner — Use `node:sqlite` only; enforce the minimum Node version; add a CI warning check - Surfaced by: R7 (D8: A) - Files: runner/store, web/cli diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index 5acbbe2b..a545b73e 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -27,12 +27,56 @@ The target is resolved again for each validation pass. Both passes, and the thir Merging blocks when any plan item is unreviewed or stale, an attributed file is outside that item's declared plan scope, an Ambiguous or Unplanned segment remains, a current change request is open, or a `cmd:` acceptance check lacks a passing result for the current head. Approval does not override an out-of-scope file; the plan must be amended. Historical change requests from an earlier revision or snapshot remain visible but do not block the current revision. -The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. A check that a ruleset (`integration_id`) or classic protection (`app_id`) pins to an app needs to know which app reported each check run, and gh's `statusCheckRollup` does not give it: gh 2.102.0 does not ask GitHub for a run's app (#176). So when any required check is pinned to an app, and the rules are known, the adapter also asks GitHub for the runs of each pinned check on the exact head that the PR read returned, through `gh api graphql`. GitHub does the filtering: for each pinned check the query lists the check suites of its app on that commit (`checkSuites(filterBy: {appId})`) and, in each, the latest run of its name (`checkRuns(filterBy: {checkName, checkType: LATEST})`, which matches the name exactly), with status, conclusion and times. The app filter also returns the suite of a private app, whose `app` GitHub's GraphQL API gives as `null` to a reader outside the app's owner (sveltejs/svelte's `vite-ecosystem-ci`; REST names the app), so such a check is read like any other. Asking only for the pinned names keeps the answer small, and a suite with many other runs (microsoft/vscode has a GitHub Actions suite with 101) does not matter. `LATEST` leaves out a run re-run inside its suite (codeabovelab/codeboost PR 151: `test` failed, then passed when re-run, in one suite); runs of the name in several suites are ordered as the rollup's are. One call holds up to 25 pinned checks, so most inspections make one call. A check whose app has more than 100 suites on the head is paged, up to 10 pages. Checks any app may report are still read from the rollup alone. No commit status meets a pinned check. GitHub may count a status that the pinned app set; this is not checked, and such a check reads as missing or pending, which fails closed. With the rules unknown the read is skipped, so its failure cannot replace the rules blocker. The read fails closed. A gh failure keeps gh's error; an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs of the pinned name, more than 10 pages of suites, or suites that change between pages fails the inspection with codeboost's own reason. The merge status shows either as “Could not read GitHub merge state.” followed by the reason; a deadline that stops the read keeps the inspection's timeout as the reason. When stages stop each other, the inspection reports the first stage's failure, even if a stopped read then ends with a gh exit of its own. GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. `test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The vite and svelte recordings were made again at the same heads on 2026-10-21 (UTC) to add the pinned-check read: vite's three checks pinned to GitHub Actions and svelte's check pinned to its private app read as passing. Two more files record only that read, on commits whose own rules pin nothing: a re-run inside one suite (`pinned-rerun.json`) and a suite with 101 runs (`pinned-large-suite.json`); `private-app-suites.json` records the svelte suite's app as GraphQL and REST give it. The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. +The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. + +Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. A check that a ruleset or classic protection pins to an app is read as described in “Required checks pinned to an app” below. + +GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. + +`test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. + +Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge call aborted at its 14-second deadline settles by 14.4 s. *Later (#48, #170):* a click can also be waiting on its issue-trust read, whose stop wait is 0.75 s, or on a review load, whose history worker gets 1 s after the abort before it is terminated; so it settles about 0.75 s after its deadline, or about 1 s or a little more after it when a review load was running, past the 14.5-second shutdown drain (traced, not tested); see [merge-queue.md](merge-queue.md). Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected. Shutdown sets a terminal admission flag before inspecting active work and checks it again after partial request bodies are received. It then stops HTTP admission, drains admitted requests, closes the pre-merge lifecycle and the Store write gate, and only then aborts and awaits an active merge CLI process (`web/server.ts`), so a refusal gh gave before the click was admitted, if it is handled only after the write gate closed, cannot be saved and answers a resendable 503 with the refusal's text; after admission, settlement keeps its shutdown capability and saves GitHub's refusal (#48). A request admitted before shutdown cannot start a new merge afterward, and a cancelled command cannot outlive the local state that authorized it. +### Required checks pinned to an app + +A ruleset (`integration_id`) or classic protection (`app_id`) can pin a required check to one GitHub App. Then only a run that app reported counts. gh's `statusCheckRollup` does not say which app reported a run: gh 2.102.0 does not ask GitHub for it (#176). So the adapter asks GitHub separately, through `gh api graphql`. + +The read happens only when the rules are known and at least one required check is pinned. It asks about the exact head that the PR read returned. GitHub does the filtering, for each pinned check: + +- the check suites of the pinned app on that commit (`checkSuites(filterBy: {appId})`); +- in each suite, the latest run with the check's name (`checkRuns(filterBy: {checkName, checkType: LATEST})`), with its status, conclusion and times. The name must match exactly, including case. + +What this covers, each checked against GitHub: + +- A private app. GitHub's GraphQL API shows its suite's `app` as `null` to a reader outside the app's owner, but the app filter still returns the suite (sveltejs/svelte's `vite-ecosystem-ci`; REST names the app). +- A suite with many other runs. Only the pinned name is asked for, so a suite with more than 100 runs (microsoft/vscode) does not matter. +- A re-run inside one suite. `LATEST` leaves out the older run (codeabovelab/codeboost PR 151: `test` failed, then passed when re-run). Runs of the name in several suites are ordered as the rollup's are. + +One call holds up to 25 pinned checks, so most inspections make one call of about one second. A check whose app has more than 100 suites on the head is read in pages of 100, up to 10 pages. In practice the inspection's deadline ends the read first. Each call takes about a second, so a fresh six-second inspection fits only a few pages after its other reads. A head with more suites than that fails closed with “GitHub merge-state inspection timed out.” + +Checks that any app may report are still read from the rollup alone. + +No commit status meets a pinned check. GitHub may count a status that the pinned app set; this is not checked. Such a check reads as missing or pending, which fails closed. + +The read fails closed: + +- A gh failure keeps gh's error. +- These fail the inspection with codeboost's own reason: an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs of the pinned name, more than 10 pages of suites, and suites that change between pages. +- The merge status shows the reason after “Could not read GitHub merge state.” +- A deadline that stops the read keeps the inspection's timeout as the reason. +- When the inspection's stages stop each other, the first stage's failure is reported, even if a stopped read then ends with a gh exit of its own. + +Recordings in `test/fixtures/gh/` back this: + +- The vite and svelte recordings were made again at the same heads on 2026-10-21 (UTC), with the pinned-check read. Vite's three checks pinned to GitHub Actions and svelte's check pinned to its private app read as passing. +- `pinned-rerun.json` (the re-run in one suite) and `pinned-large-suite.json` (the suite with more than 100 runs) hold only the pinned-check read, on commits whose own rules pin nothing, and a REST read that shows the case. +- `private-app-suites.json` holds the svelte suite's app as GraphQL and REST give it. +- `scripts/record-pinned-gh.ts` makes these three files again. + ## Deferred runner work This slice does not mutate Git history or run plan commands. If the base moved, the gate sends the task back to review. A plan item with a `cmd:` acceptance check remains blocked because the current review service has no trusted container result. Issue #22 tracks rebasing through the ledger, recomputing review state, running commands in the container, and persisting results against the exact head. F6b closes the already-fixed gap: once a rebase moves the base, the scan still covers every base commit after the task's original base. diff --git a/docs/implementation/merge-handoff.md b/docs/implementation/merge-handoff.md index 1c95c174..19899465 100644 --- a/docs/implementation/merge-handoff.md +++ b/docs/implementation/merge-handoff.md @@ -35,7 +35,7 @@ Without a runner, **Merge PR** works as before. | Wait for checks | The job inspects the pull request (fresh) at once, then again after 10 s, with the wait doubling up to 60 s. | every required check succeeds, or the required set was read and is empty, and nothing else blocks | a required check fails, or 30 minutes pass (job: `blocked`, if the preparation still applies: the block re-checks readiness in its transaction, and a review edit or other task change meanwhile ends the job `failed` instead); the pull request moved, another blocker appears, or GitHub or the local review cannot be read (job: `failed`; one failed read fails closed rather than retrying; each inspection also loads the review, which awaits a history worker since #170, within the same 30-minute limit) | | Merge | The job calls the guarded merge with the current review token, not the click's: a rebase during preparation makes a new snapshot and so a new token. A review change after preparation is refused through readiness, which binds the review version. Its inspections rerun the already-fixed check. | GitHub merged or queued the head, as the job's own merge attempt records it when the job reads it after the guarded merge (job: `merged` or `queued`; if a merge-queue poll ended the attempt removed or failed while GitHub was asked, the guarded merge cannot save it queued and rejects, and the job ends as for a refusal; `submitted` when GitHub accepted the merge but saving its outcome failed, so the attempt is still submitting and reconciles from GitHub) | GitHub refused (job: `blocked`; `failed` when GitHub refused because the head changed, which needs a fresh review, or when the review, snapshot or plan revision changed while GitHub was asked: the block is for the review the merge attempt was admitted for. A review edit (a review note) may land while the attempt is in flight. The snapshot term is defensive for a merge job: jobs need a runner, and a runner server's review load reads the recorded head, so no HEAD observation records a new snapshot. A plan revision lands only after a merge-queue poll ended the attempt while GitHub was asked, as plan writes are refused while a merge attempt is in flight), validation refused (job: `failed`), or the outcome is unknown (job: `submitted`; the merge attempt reconciles as before) | -A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. gh's rollup does not say which app reported a run (#176), so for such a check the inspection asks GitHub for that app's runs of that name on the head (`guarded-merge.md`). No commit status matches it; GitHub may count a status the app set, so such a check waits or reads missing (fails closed). If that read fails, the inspection fails, and the job ends `failed` with the reason: gh's error, or codeboost's own when GitHub's answer is malformed, too large to read whole, or changed between pages. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. +A required check run passes when it completed with success, neutral or skipped, as GitHub counts required checks; a commit status passes only with success. When a check has several runs or several statuses with its name (a re-run), the one that started last counts. The same rule applies to commit statuses. A run or status not yet completed (queued, in progress or pending) with no start time counts as the latest. A completed run or status with no start time started before its completion time, so it is known to be older than, and is dropped for, any run or status of its kind that started after that time (or that is not yet completed and has no start time). Otherwise, including when it has no completion time either, its order relative to the latest is unknown, so it is never ordered below another: it ties with the latest. Among runs that tie, the one that passes least counts, so a run or status with no time can make the check fail or wait, but never makes it pass (fail closed). A check run and a commit status with the same name are not re-runs of each other: GitHub requires both to pass, so the latest of each counts, and the check is failing if either fails, otherwise pending if either is pending. A check that names an app matches only that app's check runs. gh's rollup does not say which app reported a run (#176), so for such a check the inspection asks GitHub for that app's runs of that name on the head (`guarded-merge.md`). No commit status matches it; GitHub may count a status the app set, so such a check waits or reads missing (fails closed). If that read fails, the inspection fails, and the job ends `failed` with the reason: gh's error, or codeboost's own when GitHub's answer is malformed, too large to read whole, or changed between pages. An app with so many suites on the head that their pages cannot be read in time ends the inspection with its timeout instead. While waiting, an unknown mergeability (GitHub is still computing it) and a pending or missing required check keep the job waiting. No inspection starts after the 30 minutes, and one still running when they end is stopped, so only checks seen passing within the limit count. An inspection that finds every check passing but finishes after the limit, before its stop arrives, also ends the job `blocked`, with "Required checks passed only after the 30-minute limit. Retry merge." and no waiting checks listed; one that finds a failing check or another blocker decides as it does within the limit. Only that stop counts as the wait timing out: an inspection that GitHub failed before the limit, but that settles after it (its stopped reads finishing), ends the job `failed` with GitHub's error. The guarded merge that follows inspects again and may ask GitHub to merge shortly after the limit; it refuses anything no longer clear. Unreadable branch rules never pass. Another blocker outranks a failing check: checks that fail on a head a collaborator pushed say nothing about this review, so the job ends `failed` and the task's status is unchanged. A push to the base branch during the wait is such a blocker: the next inspection sees the moved base, and the job ends `failed` with "The base branch moved; merging rebases onto it and prepares again, so the result may need review." The task stays in review. The person clicks **Merge PR** again: the new job rebases onto the new base and prepares again. If the rebase makes an approval stale, that job ends `review-required`. diff --git a/github/merge.ts b/github/merge.ts index ab838d9e..5fe0d545 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -380,7 +380,9 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { * suite whose `app` GitHub hides from this reader (a private app), which an app read from the answer would not name. * Only each suite's latest run of a name is asked for (`checkType: LATEST`): a run re-run inside its suite is left out. * Each check is one alias in the query, with up to PINNED_CHECKS_PER_READ checks per call; a check whose app has more - * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. + * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. That cap only bounds the work: at about a second + * per call, the inspection's deadline (six seconds for a fresh one) usually stops the read after a few pages, and the + * inspection then fails with its timeout. * * The read fails closed. A gh failure keeps gh's error. GitHub's own errors in an answer, a malformed field, an answer * for another commit, a suite with more runs of the name than one page holds, more suites than the page limit, and diff --git a/scripts/record-pinned-gh.ts b/scripts/record-pinned-gh.ts new file mode 100644 index 00000000..195428ad --- /dev/null +++ b/scripts/record-pinned-gh.ts @@ -0,0 +1,77 @@ +// Record GitHub's real answers for the pinned-check tests in test/merge-recorded.test.ts (#176). +// +// node scripts/record-pinned-gh.ts +// +// Remakes three files in test/fixtures/gh/ through the real `gh`, signed in as you. Every call is read-only, against +// public repositories. Each file holds a RecordedGh with these calls, as runWithInput reports them (stdout in full, +// stderr ''; a call that fails stops the script): +// - pinned-rerun.json and pinned-large-suite.json: the production GhMergeGateway runs one fresh inspection of a public +// commit whose own rules pin nothing. Its pull request, ruleset, branch and classic protection reads are faked, as in +// `inspectPinned` in the test, so the ruleset pins one check to an app; only the pinned-check read goes to gh and is +// saved. Then one REST read, filtered with `--jq`, shows what makes the commit a case: a re-run inside one suite, or +// a suite with more than 100 runs. +// - private-app-suites.json: no gateway call. Two reads of the same commit's check suites, through REST and through +// GraphQL, show that GraphQL gives a private app's suite `app: null` while REST names the app. +// Read the files before committing them. +import { execFileSync } from 'node:child_process'; +import { writeFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { GhMergeGateway, MERGE_KILL_GRACE_MS, MERGE_PIPE_GRACE_MS } from '../github/merge.ts'; +import { ghEnvironment } from '../github/gh-env.ts'; +import { CommandFailed, runWithInput } from '../github/run-with-input.ts'; +import type { RecordedGh } from '../test/fixtures/gh/recorded.ts'; + +const directory = fileURLToPath(new URL('../test/fixtures/gh/', import.meta.url)); +const gh = execFileSync('gh', ['--version'], { encoding: 'utf8' }).split('\n', 1)[0]!; +type Call = RecordedGh['calls'][number]; +const real = async (args: readonly string[], signal?: AbortSignal): Promise => { + const stdout = await runWithInput('gh', args, { timeout: 30_000, maxBuffer: 8 * 1024 * 1024, signal, env: ghEnvironment(), + killGraceMs: MERGE_KILL_GRACE_MS, pipeGraceMs: MERGE_PIPE_GRACE_MS }); + return { args: [...args], exitCode: 0, stdout, stderr: '' }; +}; +const save = (name: string, repository: string, pullRequest: number, calls: Call[]) => { + const recording: RecordedGh = { gh, repository, pullRequest, recordedAt: new Date().toISOString(), calls }; + writeFileSync(`${directory}${name}.json`, `${JSON.stringify(recording, null, 2)}\n`); + console.log(`Saved ${calls.length} gh calls to test/fixtures/gh/${name}.json`); +}; + +/** The gateway's pinned-check read for `context` pinned to `appId` on `head`, with every other read faked. */ +async function pinnedRead(repository: string, pullRequest: number, head: string, context: string, appId: number): Promise { + const calls: Call[] = []; + const notProtected = '{"message":"Branch not protected","documentation_url":"https://docs.github.com/rest/branches/branch-protection#get-branch-protection","status":"404"}'; + const gateway = new GhMergeGateway({ repository, pullRequest, issue: 1 }, async (args, options) => { + const joined = args.join(' '); + if (joined.startsWith('pr view')) return JSON.stringify({ baseRefName: 'main', baseRefOid: '0'.repeat(40), headRefName: 'x', headRefOid: head, isDraft: false, mergeable: 'MERGEABLE', number: pullRequest, state: 'OPEN', statusCheckRollup: [] }); + if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context, integration_id: appId }] } }]]); + if (/branches\/main$/.test(joined)) return JSON.stringify({ name: 'main', protected: false, protection: { enabled: false, required_status_checks: { enforcement_level: 'off', contexts: [], checks: [] } } }); + if (joined.endsWith('/protection')) throw new CommandFailed(`gh failed (exit 1): gh: Branch not protected (HTTP 404)\n${notProtected}`, 'gh: Branch not protected (HTTP 404)', notProtected); + if (args[0] !== 'api' || args[1] !== 'graphql') throw new Error(`Unexpected gh call: ${joined}`); + const call = await real(args, options?.signal); + calls.push(call); + return call.stdout; + }, { repository, check: async () => ({ outcome: 'clear', baseHead: '0'.repeat(40) }) }); + const state = await gateway.inspect({ fresh: true }); + if (calls.length !== 1) throw new Error(`Expected one pinned-check read, got ${calls.length}.`); + console.log(JSON.stringify(state.requiredChecks)); + return calls[0]!; +} + +const rerunHead = 'e3697dd532643de0f831b678856b2d677338231d'; +save('pinned-rerun', 'codeabovelab/codeboost', 151, [ + await pinnedRead('codeabovelab/codeboost', 151, rerunHead, 'test', 15368), + await real(['api', `repos/codeabovelab/codeboost/commits/${rerunHead}/check-runs?filter=all&per_page=100`, + '--jq', '[.check_runs[] | {name, status, conclusion, started_at, completed_at, check_suite: {id: .check_suite.id}, app: {id: .app.id}}]']), +]); + +save('pinned-large-suite', 'microsoft/vscode', 343099, [ + await pinnedRead('microsoft/vscode', 343099, 'b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f', 'Detect Changes', 15368), + await real(['api', 'repos/microsoft/vscode/check-suites/107124344927/check-runs?per_page=1', + '--jq', '{total_count, check_suite: {id: .check_runs[0].check_suite.id, head_sha: .check_runs[0].head_sha}, app: {id: .check_runs[0].app.id}}']), +]); + +const svelteHead = '111cc6411c9c647f691a159862da2b74212791e3'; +save('private-app-suites', 'sveltejs/svelte', 19050, [ + await real(['api', `repos/sveltejs/svelte/commits/${svelteHead}/check-suites`, '--jq', '[.check_suites[] | {id, app: {id: .app.id, slug: .app.slug}}]']), + await real(['api', 'graphql', '-f', 'query=query($owner:String!,$repo:String!,$oid:GitObjectID!){repository(owner:$owner,name:$repo){object(oid:$oid){... on Commit{checkSuites(first:100){nodes{databaseId app{databaseId slug}}}}}}}', + '-f', 'owner=sveltejs', '-f', 'repo=svelte', '-f', `oid=${svelteHead}`]), +]); diff --git a/test/fixtures/gh/pinned-large-suite.json b/test/fixtures/gh/pinned-large-suite.json index 97ebb94b..06eda10d 100644 --- a/test/fixtures/gh/pinned-large-suite.json +++ b/test/fixtures/gh/pinned-large-suite.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "microsoft/vscode", "pullRequest": 343099, - "recordedAt": "2026-10-21T02:14:52.164Z", + "recordedAt": "2026-10-21T02:46:52.833Z", "calls": [ { "args": [ @@ -33,7 +33,7 @@ "{total_count, check_suite: {id: .check_runs[0].check_suite.id, head_sha: .check_runs[0].head_sha}, app: {id: .check_runs[0].app.id}}" ], "exitCode": 0, - "stdout": "{\"app\":{\"id\":15368},\"check_suite\":{\"head_sha\":\"b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f\",\"id\":107124344927},\"total_count\":101}\n", + "stdout": "{\"app\":{\"id\":15368},\"check_suite\":{\"head_sha\":\"b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f\",\"id\":107124344927},\"total_count\":116}\n", "stderr": "" } ] diff --git a/test/fixtures/gh/pinned-rerun.json b/test/fixtures/gh/pinned-rerun.json index 8afda3ec..7edff640 100644 --- a/test/fixtures/gh/pinned-rerun.json +++ b/test/fixtures/gh/pinned-rerun.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "codeabovelab/codeboost", "pullRequest": 151, - "recordedAt": "2026-10-21T02:11:40.539Z", + "recordedAt": "2026-10-21T02:46:51.628Z", "calls": [ { "args": [ diff --git a/test/fixtures/gh/private-app-suites.json b/test/fixtures/gh/private-app-suites.json index acc2242f..385ff6e6 100644 --- a/test/fixtures/gh/private-app-suites.json +++ b/test/fixtures/gh/private-app-suites.json @@ -2,7 +2,7 @@ "gh": "gh version 2.102.0 (2026-09-30)", "repository": "sveltejs/svelte", "pullRequest": 19050, - "recordedAt": "2026-10-21T02:15:18.764Z", + "recordedAt": "2026-10-21T02:46:54.004Z", "calls": [ { "args": [ diff --git a/test/fixtures/gh/recorded.ts b/test/fixtures/gh/recorded.ts index c1b024f3..6b7cfcf0 100644 --- a/test/fixtures/gh/recorded.ts +++ b/test/fixtures/gh/recorded.ts @@ -3,11 +3,16 @@ import { CommandFailed } from '../../../github/run-with-input.ts'; import type { RunGh } from '../../../github/merge.ts'; /** - * What `scripts/record-merge-gh.ts` saves: every `gh` call of one real merge inspection, as gh answered it and - * `runWithInput` (github/run-with-input.ts) reports it to the gateway: stdout in full on exit 0, where stderr is not kept - * and is saved as ''; on another exit, stderr and stdout with surrounding whitespace trimmed, as runWithInput keeps them. - * One part is left out: a branch read (`api repos//branches/`) keeps only `name`, `protected` and - * `protection` of GitHub's answer, since the gateway reads only the last two. + * Real `gh` calls, as gh answered them and `runWithInput` (github/run-with-input.ts) reports them to the gateway: stdout + * in full on exit 0, where stderr is not kept and is saved as ''; on another exit, stderr and stdout with surrounding + * whitespace trimmed, as runWithInput keeps them. Two scripts write these files: + * - `scripts/record-merge-gh.ts` (zero-checks, checks-pending, checks-passing, nonadmin-*): every `gh` call of one real + * merge inspection. One part is left out: a branch read (`api repos//branches/`) keeps only `name`, + * `protected` and `protection` of GitHub's answer, since the gateway reads only the last two. + * - `scripts/record-pinned-gh.ts` (pinned-rerun, pinned-large-suite, private-app-suites): not a whole inspection. The + * pinned-* files hold the gateway's pinned-check read from an inspection whose other reads were faked, plus one REST + * read that `--jq` filters to a few fields; private-app-suites holds a REST read and a GraphQL read of one commit's + * check suites. The script's header says which. */ export interface RecordedGh { gh: string; diff --git a/test/merge-recorded.test.ts b/test/merge-recorded.test.ts index 1978579b..8a7a7fe7 100644 --- a/test/merge-recorded.test.ts +++ b/test/merge-recorded.test.ts @@ -7,6 +7,7 @@ import type { ReviewService } from '../runner/review.ts'; import { Store } from '../runner/store.ts'; import { commandDigest } from '../runner/checks.ts'; import type { Plan, PlanContext } from '../core/plan.ts'; +import { CommandFailed } from '../github/run-with-input.ts'; import { loadRecording, replayGh, type RecordedGh } from './fixtures/gh/recorded.ts'; // T12: required checks read from GitHub's real answers, recorded by scripts/record-merge-gh.ts with gh 2.102.0 from the @@ -123,23 +124,32 @@ it('reads a check both a ruleset and classic protection require once, with a tok expect(state).toMatchObject({ rulesKnown: true, atomicBaseGuard: false, requiredChecks: [{ context: 'vite-ecosystem-ci', appId: 2935623, state: 'success' }] }); }); -// Pinned-check reads on public commits whose rules pin no check, recorded 2026-10-21 with gh 2.102.0 by a script that -// ran the gateway with the fake pull request and ruleset of `inspectPinned` below and sent only the pinned-check read to -// the real gh. Each file also holds a REST read that shows what makes the commit a case worth recording. +// Pinned-check reads on public commits whose rules pin no check, recorded 2026-10-21 with gh 2.102.0 by +// scripts/record-pinned-gh.ts, which runs the gateway with the same fake pull request and rules as `inspectPinned` below +// and sends only the pinned-check read to the real gh. Each file also holds a REST read that shows what makes the +// commit a case worth recording. // - pinned-rerun: codeabovelab/codeboost commit e3697dd (PR 151's head). REST's list of every run (`filter=all`) shows // suite 102552054061 ran `test` twice: failed at 21:54, passed when re-run at 22:02. -// - pinned-large-suite: microsoft/vscode commit b3aa5cb (PR 343099's head). REST counts 101 runs in GitHub Actions -// suite 107124344927, more than one page of runs holds. +// - pinned-large-suite: microsoft/vscode commit b3aa5cb (PR 343099's head). REST counts more than 100 runs in GitHub +// Actions suite 107124344927, more than one page of runs holds. const pinnedRecordings = { rerun: loadRecording('pinned-rerun'), largeSuite: loadRecording('pinned-large-suite') }; -/** Inspects `recording`'s head with `context` pinned to `appId`; only the pinned-check read is answered from the recording. */ +/** + * Inspects `recording`'s head with `context` pinned to `appId`; only the pinned-check read is answered from the recording. + * The other answers follow the shapes gh 2.102.0 gave in zero-checks.json for a branch no classic protection covers: + * the branch read with `name`, `protected` and `protection`, and the protection read's 404 "Branch not protected", as + * runWithInput reports it. The pull request read leaves out `url` and gives an empty rollup: the gateway reads `url` + * only for display, and a pinned check is never matched from the rollup. The ruleset leaves out each rule's + * `ruleset_source_type`, `ruleset_source`, `ruleset_id` and `do_not_enforce_on_create`, which the gateway does not read. + */ async function inspectPinned(recording: RecordedGh, head: string, context: string, appId: number) { const replay = replayGh(recording); + const notProtected = '{"message":"Branch not protected","documentation_url":"https://docs.github.com/rest/branches/branch-protection#get-branch-protection","status":"404"}'; const run: RunGh = async (args, options) => { const joined = args.join(' '); - if (joined.startsWith('pr view')) return JSON.stringify({ number: recording.pullRequest, isDraft: false, baseRefName: 'main', baseRefOid: '0'.repeat(40), headRefName: 'x', headRefOid: head, state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [] }); + if (joined.startsWith('pr view')) return JSON.stringify({ baseRefName: 'main', baseRefOid: '0'.repeat(40), headRefName: 'x', headRefOid: head, isDraft: false, mergeable: 'MERGEABLE', number: recording.pullRequest, state: 'OPEN', statusCheckRollup: [] }); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context, integration_id: appId }] } }]]); - if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); - if (joined.endsWith('/protection')) throw new Error('HTTP 404: Not Found'); + if (/branches\/main$/.test(joined)) return JSON.stringify({ name: 'main', protected: false, protection: { enabled: false, required_status_checks: { enforcement_level: 'off', contexts: [], checks: [] } } }); + if (joined.endsWith('/protection')) throw new CommandFailed(`gh failed (exit 1): gh: Branch not protected (HTTP 404)\n${notProtected}`, 'gh: Branch not protected (HTTP 404)', notProtected); return replay(args, options); }; const state = await new GhMergeGateway({ repository: recording.repository, pullRequest: recording.pullRequest, issue: 1 }, run, clearCheck(recording.repository)).inspect({ fresh: true }); @@ -164,7 +174,8 @@ it('reads a pinned check from an app whose suite has more than 100 runs', async const { largeSuite } = pinnedRecordings; expect(largeSuite.gh).toBe('gh version 2.102.0 (2026-09-30)'); const count = JSON.parse(largeSuite.calls.find(call => call.args[1]!.includes('/check-suites/'))!.stdout) as { total_count: number; app: { id: number }; check_suite: { id: number; head_sha: string } }; - expect(count).toEqual({ total_count: 101, app: { id: 15368 }, check_suite: { id: 107124344927, head_sha: 'b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f' } }); + // The suite had 101 runs when the review found it and 116 when this was recorded, later the same day. + expect(count).toEqual({ total_count: 116, app: { id: 15368 }, check_suite: { id: 107124344927, head_sha: 'b3aa5cb6d1d0267f1b60cbb0bffaba898c83ad3f' } }); // GitHub filters the runs by name, so that suite answers with only the one run asked for. const suites = pinnedAnswer(largeSuite).data.repository.object.c0.nodes; expect(suites.map(suite => suite.checkRuns.totalCount).sort()).toEqual([0, 0, 0, 0, 1]); diff --git a/test/merge-stop-reasons.test.ts b/test/merge-stop-reasons.test.ts index 1ab49d95..56f80858 100644 --- a/test/merge-stop-reasons.test.ts +++ b/test/merge-stop-reasons.test.ts @@ -489,7 +489,12 @@ it('merge-state inspection reports the first stage\'s gh exit when a stopped pin // stage ends with the pinned-check read (#176). The list's gh exit comes first and stops that read; gh then exits on // its own with another error while it is stopping, which runWithInput reports instead of the abort (#48). const first = new CommandFailed('gh failed (exit 1): gh: Bad credentials (HTTP 401)', 'gh: Bad credentials (HTTP 401)', '{"message":"Bad credentials","status":"401"}'); - const later = new CommandFailed('gh failed (exit 1): gh: Something went wrong while executing your query.', 'gh: Something went wrong while executing your query.', ''); + // gh 2.102.0 exits 1 on a GraphQL error, with `gh: ` on stderr and GitHub's response on stdout. This error's + // text is GitHub's usual one; it was not reproduced here (GitHub gives it only on its own failures), and the gateway + // reads only that this is gh's own exit, not its text. + const wentWrong = 'Something went wrong while executing your query. This may be the result of a timeout, or it could be a GitHub bug. Please include `0000:0000:0000000:0000000:00000000` when reporting this issue.'; + const laterBody = JSON.stringify({ data: null, errors: [{ message: wentWrong }] }); + const later = new CommandFailed(`gh failed (exit 1): gh: ${wentWrong}\n${laterBody}`, `gh: ${wentWrong}`, laterBody); const pinnedStarted = deferred(); let pinnedStopReason: unknown; const run: Run = async (args, options) => { diff --git a/test/merge.test.ts b/test/merge.test.ts index fb44db4e..867d476d 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1091,6 +1091,9 @@ it('ignores required reviews in both rule sources and reads only their required * Inspects a pull request whose one required check is `test`, from any app or, with `appId`, from that app, and whose head * has these observed runs and statuses in gh's rollup. `checkRuns` answers the read of a pinned check's runs: the head's * check suites, or a function of the call's arguments that answers or throws; without it that read is unexpected. + * The other answers have gh 2.102.0's shapes (test/fixtures/gh/) and leave out fields the gateway does not read on this + * path: the pull request's `url`; each rule's source fields and `do_not_enforce_on_create`; and the branch's `name`, + * `commit`, links and `protection` (read only when the branch is protected). */ async function requiredTestState(statusCheckRollup: unknown[], appId: number | null = null, checkRuns?: readonly Suite[] | ((args: readonly string[]) => string | Promise)) { const run = async (args: readonly string[]) => { @@ -1099,7 +1102,8 @@ async function requiredTestState(statusCheckRollup: unknown[], appId: number | n if (checkRuns !== undefined && isCheckRunRead(args)) return typeof checkRuns === 'function' ? checkRuns(args) : checkRunsAnswer(args, checkRuns); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: appId }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); - if (joined.endsWith('/protection')) throw new Error('gh: Branch not protected (HTTP 404)'); + // gh 2.102.0's 404 for a branch no classic protection covers (zero-checks.json), as runWithInput reports it. + if (joined.endsWith('/protection')) throw NOT_PROTECTED; { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } throw new Error(`Unexpected gh call: ${joined}`); }; @@ -1355,7 +1359,8 @@ it('does not read pinned checks when the rules are unknown, so the rules blocker const run = async (args: readonly string[]) => { const joined = args.join(' '); if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: appLessPass }); - if (isCheckRunRead(args)) { reads++; throw ghExit('gh: Something went wrong while executing your query. This may be the result of a timeout, or it could be a GitHub bug.', ''); } + // Never answered: the read must not run. Shaped as gh 2.102.0 exits on a GraphQL error (stdout: GitHub's response). + if (isCheckRunRead(args)) { reads++; throw ghExit('gh: Something went wrong while executing your query.', '{"data":null,"errors":[{"message":"Something went wrong while executing your query."}]}'); } if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); // Classic protection whose `strict` is malformed: the rules are unknown, after the ruleset's pinned check was read. From 40717e1031332e12803faf99f4776dd6f43d6232 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:01:38 -0700 Subject: [PATCH 4/9] Describe the stop order runWithInput gives, and name what fakes leave out (#176 review) Round 3 of the independent review of PR #195 found that the code comment, the stop-reason test and guarded-merge.md described a stopped read that exits with its own error after the stop. runWithInput cannot give that: once it sees the abort before gh exits, it rejects with the abort's reason. The real case is a gh that had already exited while its pipes closed after the stop. All three now say so, and the test's fake models that order. It still fails without the first-failure line. The new fakes now use gh 2.102.0's shapes: the 401 and 404 bodies with documentation_url and runWithInput's stdout suffix, the unprotected branch read with its protection (UNPROTECTED_BRANCH), and the abort's reason for a stopped read. Notes name what the fakes leave out. guarded-merge.md says the fresh inspection's deadline fits a few pages and the 12 s display inspection about nine. Co-Authored-By: Claude Opus 5.5 --- docs/implementation/guarded-merge.md | 4 ++-- github/merge.ts | 8 +++---- test/merge-stop-reasons.test.ts | 34 ++++++++++++++++------------ test/merge.test.ts | 21 +++++++++++++---- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index a545b73e..60c84897 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -56,7 +56,7 @@ What this covers, each checked against GitHub: - A suite with many other runs. Only the pinned name is asked for, so a suite with more than 100 runs (microsoft/vscode) does not matter. - A re-run inside one suite. `LATEST` leaves out the older run (codeabovelab/codeboost PR 151: `test` failed, then passed when re-run). Runs of the name in several suites are ordered as the rollup's are. -One call holds up to 25 pinned checks, so most inspections make one call of about one second. A check whose app has more than 100 suites on the head is read in pages of 100, up to 10 pages. In practice the inspection's deadline ends the read first. Each call takes about a second, so a fresh six-second inspection fits only a few pages after its other reads. A head with more suites than that fails closed with “GitHub merge-state inspection timed out.” +One call holds up to 25 pinned checks, so most inspections make one call of about one second. A check whose app has more than 100 suites on the head is read in pages of 100, up to 10 pages. For a fresh inspection, the deadline ends the read first: each call takes about a second, so its six seconds fit only a few pages after the other reads. The display inspection has 12 seconds, which fit about nine. Either way, a head with more suites than fit fails closed with “GitHub merge-state inspection timed out.” Checks that any app may report are still read from the rollup alone. @@ -68,7 +68,7 @@ The read fails closed: - These fail the inspection with codeboost's own reason: an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs of the pinned name, more than 10 pages of suites, and suites that change between pages. - The merge status shows the reason after “Could not read GitHub merge state.” - A deadline that stops the read keeps the inspection's timeout as the reason. -- When the inspection's stages stop each other, the first stage's failure is reported, even if a stopped read then ends with a gh exit of its own. +- When the inspection's stages stop each other, the first stage's failure is reported. This holds even when a stopped read then settles with its own gh error: its gh had already exited, and its output closed after the stop. Recordings in `test/fixtures/gh/` back this: diff --git a/github/merge.ts b/github/merge.ts index 5fe0d545..76339968 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -334,8 +334,8 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { ]); const fixed = await alreadyFixed; // The first stage to fail stopped the others, and its error is `failed`'s reason. A stopped stage can still settle - // with an error of its own (a gh exit that came while gh was stopping), so the first failure is reported, not the - // first stage in this list that failed. Every rejection above aborts `failed`, so the loop after it is a backstop. + // with an error of its own (its gh had already exited, and its pipes closed after the stop), so the first failure is + // reported, not the first stage in this list that failed. Every rejection above aborts `failed`, so the loop after it is a backstop. if (failed.signal.aborted) throw failed.signal.reason; for (const read of [ruleRead, listRead]) if (read.status === 'rejected') throw read.reason; if (fixed instanceof Error) throw fixed; @@ -381,8 +381,8 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { * Only each suite's latest run of a name is asked for (`checkType: LATEST`): a run re-run inside its suite is left out. * Each check is one alias in the query, with up to PINNED_CHECKS_PER_READ checks per call; a check whose app has more * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. That cap only bounds the work: at about a second - * per call, the inspection's deadline (six seconds for a fresh one) usually stops the read after a few pages, and the - * inspection then fails with its timeout. + * per call, the inspection's deadline usually stops the read first (after a few pages in a fresh six-second inspection, + * about nine in a 12-second display one), and the inspection then fails with its timeout. * * The read fails closed. A gh failure keeps gh's error. GitHub's own errors in an answer, a malformed field, an answer * for another commit, a suite with more runs of the name than one page holds, more suites than the page limit, and diff --git a/test/merge-stop-reasons.test.ts b/test/merge-stop-reasons.test.ts index 56f80858..caed9e81 100644 --- a/test/merge-stop-reasons.test.ts +++ b/test/merge-stop-reasons.test.ts @@ -486,35 +486,41 @@ it('merge-state inspection answers a caller abort, not the already-fixed check\' it('merge-state inspection reports the first stage\'s gh exit when a stopped pinned-check read then settles with its own gh exit', async () => { // A published target, so the branch's PR list runs beside the rule reads. The rules pin `test` to app 10, so the rule - // stage ends with the pinned-check read (#176). The list's gh exit comes first and stops that read; gh then exits on - // its own with another error while it is stopping, which runWithInput reports instead of the abort (#48). - const first = new CommandFailed('gh failed (exit 1): gh: Bad credentials (HTTP 401)', 'gh: Bad credentials (HTTP 401)', '{"message":"Bad credentials","status":"401"}'); - // gh 2.102.0 exits 1 on a GraphQL error, with `gh: ` on stderr and GitHub's response on stdout. This error's - // text is GitHub's usual one; it was not reproduced here (GitHub gives it only on its own failures), and the gateway - // reads only that this is gh's own exit, not its text. + // stage ends with the pinned-check read (#176). That read's gh has already exited with an error, but its output pipes + // stay open (a process it started holds them). Then the list's gh exit stops the other stages. runWithInput reports + // an exit it saw before the abort, so the pinned-check read settles with its own error once its pipes close (#48). + // Every answer has gh 2.102.0's shape, as runWithInput reports it (`ghExit`: stderr, then stdout after a newline). + const ghExit = (stderr: string, stdout: string) => new CommandFailed(`gh failed (exit 1): ${stderr}${stdout ? `\n${stdout}` : ''}`, stderr, stdout); + const first = ghExit('gh: Bad credentials (HTTP 401)', '{"message":"Bad credentials","documentation_url":"https://docs.github.com/rest","status":"401"}'); + // GitHub's usual text for its own failure while running a query; not reproduced here (GitHub gives it only when it + // fails), and the gateway reads only that this is gh's own exit, not its text. const wentWrong = 'Something went wrong while executing your query. This may be the result of a timeout, or it could be a GitHub bug. Please include `0000:0000:0000000:0000000:00000000` when reporting this issue.'; - const laterBody = JSON.stringify({ data: null, errors: [{ message: wentWrong }] }); - const later = new CommandFailed(`gh failed (exit 1): gh: ${wentWrong}\n${laterBody}`, `gh: ${wentWrong}`, laterBody); - const pinnedStarted = deferred(); + const later = ghExit(`gh: ${wentWrong}`, JSON.stringify({ data: null, errors: [{ message: wentWrong }] })); + const notProtected = ghExit('gh: Branch not protected (HTTP 404)', + '{"message":"Branch not protected","documentation_url":"https://docs.github.com/rest/branches/branch-protection#get-branch-protection","status":"404"}'); + const pinnedExited = deferred(); let pinnedStopReason: unknown; const run: Run = async (args, options) => { const joined = args.join(' '); + // The pull request read leaves out `url`, which the gateway reads only for display; the ruleset leaves out each + // rule's source fields and `do_not_enforce_on_create`, which it does not read. if (joined.startsWith('pr view')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'task', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [], body: '', isCrossRepository: false }); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); - if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); - if (joined.endsWith('/protection')) throw new CommandFailed('gh failed (exit 1): gh: Branch not protected (HTTP 404)', 'gh: Branch not protected (HTTP 404)', '{"message":"Branch not protected","status":"404"}'); + if (/branches\/main$/.test(joined)) return JSON.stringify({ name: 'main', protected: false, protection: { enabled: false, required_status_checks: { enforcement_level: 'off', contexts: [], checks: [] } } }); + if (joined.endsWith('/protection')) throw notProtected; if (args[1] === 'graphql' && joined.includes('checkSuites(')) return new Promise((_, reject) => { - pinnedStarted.resolve(); + pinnedExited.resolve(); // gh exited; runWithInput now waits for its pipes and ignores a later abort. options?.signal?.addEventListener('abort', () => { pinnedStopReason = options.signal!.reason; setTimeout(() => reject(later), 20); }, { once: true }); }); - if (joined.includes('/pulls?')) { await pinnedStarted.promise; throw first; } + if (joined.includes('/pulls?')) { await pinnedExited.promise; throw first; } throw new Error(`Unexpected gh call: ${joined}`); }; const checks: AlreadyFixedGateway = { repository: 'owner/repo', check: async () => ({ outcome: 'clear', baseHead: sha('9') }) }; const error = await new GhMergeGateway(config, run, checks).inspect({ fresh: true, target: { pullRequest: 7, headBranch: 'task' } }) .then(() => null, (reason: unknown) => reason); - expect(pinnedStopReason).toBe(first); // The interleaving: the list's exit stopped the pinned-check read. + // The interleaving: the pinned-check read's gh had exited, then the list's exit stopped that read, which settled after. + expect(pinnedStopReason).toBe(first); expect(error).toBe(first); }); diff --git a/test/merge.test.ts b/test/merge.test.ts index 867d476d..19fd815f 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1251,6 +1251,15 @@ it.each([ expect(await requiredTestState([...rollup])).toEqual([expected]); }); +/** + * The branch read gh 2.102.0 gives for a branch no classic protection covers (zero-checks.json, less the branch's + * `commit` and links, which the gateway does not read). + */ +const UNPROTECTED_BRANCH = JSON.stringify({ name: 'main', protected: false, protection: { enabled: false, required_status_checks: { enforcement_level: 'off', contexts: [], checks: [] } } }); +// The pinned-check tests below fake gh 2.102.0's other answers in the shapes of test/fixtures/gh/: the protection read's +// 404 is NOT_PROTECTED and the branch read UNPROTECTED_BRANCH, unless a test says otherwise. They leave out what the +// gateway does not read on these paths: the pull request's `url` (read only for display) and each rule's +// `ruleset_source_type`, `ruleset_source`, `ruleset_id` and `do_not_enforce_on_create`. // #176: gh's rollup never carries a check run's app (test/fixtures/gh/, gh 2.102.0), so a requirement pinned to an app is // matched from the runs GitHub lists for that app and name on the head. Each rollup below holds an app-less passing // `test` run, as gh reports one; before #176 every requirement here read as missing. @@ -1325,7 +1334,7 @@ it('reads several pinned checks in one call, and pages only the check whose app } if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: 10 }, { context: 'lint', integration_id: 11 }] } }]]); - if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (/branches\/main$/.test(joined)) return UNPROTECTED_BRANCH; if (joined.endsWith('/protection')) throw NOT_PROTECTED; { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } throw new Error(`Unexpected gh call: ${joined}`); @@ -1344,7 +1353,7 @@ it('reads more pinned checks than one call holds in several calls', async () => if (isCheckRunRead(args)) { calls.push(pinnedVariables(args).length); return checkRunsAnswer(args, [{ app: 10, runs: contexts.map(context => checkRun(context, 'COMPLETED', 'SUCCESS')) }]); } if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: contexts.map(context => ({ context, integration_id: 10 })) } }]]); - if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (/branches\/main$/.test(joined)) return UNPROTECTED_BRANCH; if (joined.endsWith('/protection')) throw NOT_PROTECTED; { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } throw new Error(`Unexpected gh call: ${joined}`); @@ -1362,8 +1371,9 @@ it('does not read pinned checks when the rules are unknown, so the rules blocker // Never answered: the read must not run. Shaped as gh 2.102.0 exits on a GraphQL error (stdout: GitHub's response). if (isCheckRunRead(args)) { reads++; throw ghExit('gh: Something went wrong while executing your query.', '{"data":null,"errors":[{"message":"Something went wrong while executing your query."}]}'); } if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: false, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); + // GitHub's branch read also gives `protection`, which the gateway reads only after the protection read answers 404. if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); - // Classic protection whose `strict` is malformed: the rules are unknown, after the ruleset's pinned check was read. + // Classic protection whose `strict` is malformed: the rules are unknown, after the ruleset's pinned check was parsed. if (joined.endsWith('/protection')) return JSON.stringify({ required_status_checks: { strict: 'yes', checks: [] } }); { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } throw new Error(`Unexpected gh call: ${joined}`); @@ -1385,7 +1395,7 @@ function pinnedGateway(answer: (args: readonly string[], signal?: AbortSignal) = if (joined.startsWith('pr view 7')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: appLessPass }); if (isCheckRunRead(args)) return answer(args, runOptions?.signal); if (joined.includes('/rules/branches/')) return JSON.stringify([[{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: 10 }] } }]]); - if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: false }); + if (/branches\/main$/.test(joined)) return UNPROTECTED_BRANCH; if (joined.endsWith('/protection')) throw NOT_PROTECTED; { const fixed = alreadyFixedReads(args); if (fixed !== null) return fixed; } throw new Error(`Unexpected gh call: ${joined}`); @@ -1433,7 +1443,8 @@ it.each(failedCheckRunReads)('blocks the merge with the reason when the read of it('keeps the inspection\'s timeout as the reason when the read of the pinned check outlasts it', async () => { let stopped: unknown = null; const gateway = pinnedGateway((_args, signal) => new Promise((_resolve, reject) => { - signal!.addEventListener('abort', () => { stopped = signal!.reason; reject(new Error('gh was stopped')); }, { once: true }); + // runWithInput rejects with the abort's reason when the abort stops gh before it exits. + signal!.addEventListener('abort', () => { stopped = signal!.reason; reject(signal!.reason); }, { once: true }); })); await expect(gateway.inspect({ fresh: true, timeoutMs: 50 })).rejects.toThrow('GitHub merge-state inspection timed out.'); // The deadline stopped this read: it was in flight when the deadline fired. From 673b9d132740603feaa0c4d44c8a4cbae26024cf Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:15:11 -0700 Subject: [PATCH 5/9] Note svelte's changed mergeable, the re-record date and the Checks-permission gap (#176 review round 4) Co-Authored-By: Claude Opus 5.5 --- docs/implementation/guarded-merge.md | 2 +- test/merge-recorded.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index 60c84897..602b85bd 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -33,7 +33,7 @@ Malformed pagination, rules without a type, malformed protection objects, malfor GitHub reports a branch as `protected` when only a ruleset applies. Classic protection is read from `branches//protection`, which only a token with admin rights may read; it gives the required checks and `strict`. A token without admin rights got 404 "Not Found" there on every protected branch checked, so on that answer the adapter reads classic checks from the branch read (`branches/`), which a token with read access got on the public repositories checked: `protection.enabled`, and `protection.required_status_checks` with `enforcement_level`, `contexts` and `checks` (with `app_id`). On a branch GitHub reports as protected, absence of classic protection is confirmed only by `protection.enabled: false` with `enforcement_level` `off` and empty lists, together with a 404 "Branch not protected" (admin rights) or "Not Found" (no admin rights). With `protection.enabled: true`, a "Not Found" answer takes the classic checks from the branch read: `enforcement_level` `off` with empty lists means no classic checks; `non_admins` or `everyone` means the listed checks are required, held to the same rules as the protection read's (`contexts` and `checks` both present and naming the same checks, every check with an `app_id`). The branch read has no `strict`, so classic protection read this way never supplies the atomic base guard, and codeboost's merge into a branch that only classic `strict` guards stays blocked (the `base-guard` blocker) for a token without admin rights. Any other answer on a protected branch blocks merging as unknown rules: another 404 message, a 404 that gh did not answer, `enabled` that is not a boolean, an unknown `enforcement_level`, checks listed with enforcement `off`, enforced checks while `enabled` is false, missing or disagreeing lists, and "Branch not protected" with `enabled: true`. -`test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a private repository read without admin rights; and GitHub Enterprise Server. +`test/merge-recorded.test.ts` replays GitHub's recorded answers (T12): read with an admin token, a ruleset check with no `integration_id` and the "Branch not protected" 404; read with a token without admin rights (2026-10-20, gh 2.102.0, public repositories; vite and svelte were recorded again on 2026-10-21, see below): a ruleset-only branch (vitejs/vite `main`), a classic-protected branch (cli/cli `trunk`), and a branch where both require one check (sveltejs/svelte `main`). The malformed and contradictory answers are hand-written in `test/merge.test.ts`. Not checked: that the branch read's checks match the protection read's (no recording has both for one branch); a token with write but not admin access, the access codeboost needs to merge (all recordings and checks without admin rights used read-only access; if such a token gets another answer, such as 403, merging stays blocked as unknown rules); a fine-grained token or a GitHub App token without the Administration permission (it may get 403 instead, which blocks merging as unknown rules); a token that cannot read checks, such as a fine-grained or GitHub App token without the Checks permission (the read of runs pinned to an app may then find no suites, so a pinned check reads as missing: merging stays blocked, after the job's full wait); a private repository read without admin rights; and GitHub Enterprise Server. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging. diff --git a/test/merge-recorded.test.ts b/test/merge-recorded.test.ts index 8a7a7fe7..f6a84f88 100644 --- a/test/merge-recorded.test.ts +++ b/test/merge-recorded.test.ts @@ -56,7 +56,8 @@ it('reads the ruleset\'s required checks as pending, then as passing at the same // The same reads by a token without admin rights, recorded 2026-10-20 with gh 2.102.0 from public repositories this // token may only read. Each classic protection read answers 404 "Not Found", so classic checks come from the branch read. // nonadmin-ruleset and nonadmin-mixed were recorded again at the same heads on 2026-10-21 (UTC), when the inspection -// began to read the runs of checks pinned to an app from GitHub (#176); their other answers did not change. +// began to read the runs of checks pinned to an app from GitHub (#176). Their other answers did not change, except that +// svelte's pull request read now says `mergeable` MERGEABLE where it said UNKNOWN (no test reads it from this recording). // - nonadmin-ruleset: vitejs/vite PR 23837 into main, which only a ruleset covers (`protection.enabled` false); // - nonadmin-classic: cli/cli PR 14716 into trunk, with classic required checks and no ruleset checks; // - nonadmin-mixed: sveltejs/svelte PR 19050 into main, where a ruleset and classic protection both require one check. From 30c3c180e50ab9d22294b299bc039c88bae38d4d Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:19:19 -0700 Subject: [PATCH 6/9] Report a next page without a cursor as an incomplete answer (#176 review round 5) Co-Authored-By: Claude Opus 5.5 --- github/merge.ts | 4 +++- test/merge.test.ts | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/github/merge.ts b/github/merge.ts index 76339968..f142c88c 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -443,8 +443,10 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { check.done = true; continue; } - if (page === PINNED_SUITE_PAGES || typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) + if (page === PINNED_SUITE_PAGES) throw new Error(`The pull request head has more check suites from app ${check.appId} than codeboost can read.`); + // A next page without a cursor to it is a malformed answer, not a long one. + if (typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) throw new Error('GitHub returned incomplete check runs.'); check.cursor = connection.pageInfo.endCursor; } } diff --git a/test/merge.test.ts b/test/merge.test.ts index 19fd815f..79346988 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1422,6 +1422,9 @@ const failedCheckRunReads: Array<[string, (args: readonly string[]) => Promise ({ totalCount: 3 }) }) : checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2, hasNextPage: true, endCursor: 'PAGE_2' }) }), /changed while they were read/], ['the pages hold fewer suites than GitHub counts', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2 }) }), /changed while they were read/], + // Defensive: GitHub has not been seen to answer a next page without a cursor to it. + ['GitHub says there is a next page but gives no cursor to it (defensive)', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2, hasNextPage: true, endCursor: null }) }), + /^GitHub returned incomplete check runs\.$/], ['the app has more than 1,000 check suites on the head', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 1001, hasNextPage: true, endCursor: 'NEXT' }) }), /more check suites from app 10 than codeboost can read/], ]; From 4f88972192f5214697290709a1832429eed9967f Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:24:13 -0700 Subject: [PATCH 7/9] Check the cursor before the page limit, so a 10th page without one is incomplete too (#176 review round 6) Co-Authored-By: Claude Opus 5.5 --- github/merge.ts | 4 ++-- test/merge.test.ts | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/github/merge.ts b/github/merge.ts index f142c88c..ebb5b71e 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -443,10 +443,10 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { check.done = true; continue; } + // A next page without a cursor to it is a malformed answer, not a long one, on any page. + if (typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) throw new Error('GitHub returned incomplete check runs.'); if (page === PINNED_SUITE_PAGES) throw new Error(`The pull request head has more check suites from app ${check.appId} than codeboost can read.`); - // A next page without a cursor to it is a malformed answer, not a long one. - if (typeof connection.pageInfo.endCursor !== 'string' || !connection.pageInfo.endCursor) throw new Error('GitHub returned incomplete check runs.'); check.cursor = connection.pageInfo.endCursor; } } diff --git a/test/merge.test.ts b/test/merge.test.ts index 79346988..d3d81673 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1425,6 +1425,11 @@ const failedCheckRunReads: Array<[string, (args: readonly string[]) => Promise checkRunsAnswer(args, passing, { page: () => ({ totalCount: 2, hasNextPage: true, endCursor: null }) }), /^GitHub returned incomplete check runs\.$/], + // Defensive, as above, on the last page codeboost reads: the missing cursor, not the page limit, is the reason. + ['GitHub says there is a page after the 10th but gives no cursor to it (defensive)', async args => { + const after = pinnedVariables(args)[0]!.after, page = after === null ? 1 : Number(after.slice('PAGE_'.length)); + return checkRunsAnswer(args, passing, { page: () => ({ totalCount: 1001, hasNextPage: true, endCursor: page === 10 ? null : `PAGE_${page + 1}` }) }); + }, /^GitHub returned incomplete check runs\.$/], ['the app has more than 1,000 check suites on the head', async args => checkRunsAnswer(args, passing, { page: () => ({ totalCount: 1001, hasNextPage: true, endCursor: 'NEXT' }) }), /more check suites from app 10 than codeboost can read/], ]; From aa238398ae19bc1fa7ebe436a3b9e446b8a650a0 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:30:17 -0700 Subject: [PATCH 8/9] Drop the last rollup app field gh never sends, and scope the first-failure claim (#176 review round 7) Co-Authored-By: Claude Opus 5.5 --- docs/implementation/guarded-merge.md | 2 +- test/merge.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index 602b85bd..d0d6844f 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -68,7 +68,7 @@ The read fails closed: - These fail the inspection with codeboost's own reason: an answer with errors or a malformed field, an answer for another commit, a suite with more than 100 runs of the pinned name, more than 10 pages of suites, and suites that change between pages. - The merge status shows the reason after “Could not read GitHub merge state.” - A deadline that stops the read keeps the inspection's timeout as the reason. -- When the inspection's stages stop each other, the first stage's failure is reported. This holds even when a stopped read then settles with its own gh error: its gh had already exited, and its output closed after the stop. +- When the inspection's stages stop each other, the first stage's failure is reported, unless the inspection's deadline fires while the stopped stages settle and that failure was not gh's own exit: then the timeout is reported. Either way the read fails closed. The first failure is kept even when a stopped read then settles with its own gh error: its gh had already exited, and its output closed after the stop. Recordings in `test/fixtures/gh/` back this: diff --git a/test/merge.test.ts b/test/merge.test.ts index d3d81673..fd576acb 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1678,7 +1678,7 @@ it.each([ ] as const)('fails closed for an invalid %s check app identity', async (_source, rules, requiredStatusChecks) => { const run = async (args: readonly string[]) => { const joined = args.join(' '); - if (joined.startsWith('pr view')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [{ name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS', app: { databaseId: 10 } }] }); + if (joined.startsWith('pr view')) return JSON.stringify({ number: 7, isDraft: false, baseRefName: 'main', baseRefOid: sha('a'), headRefName: 'feature', headRefOid: sha('b'), state: 'OPEN', mergeable: 'MERGEABLE', statusCheckRollup: [{ name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS' }] }); if (joined.includes('/rules/branches/')) return JSON.stringify([rules]); if (/branches\/main$/.test(joined)) return JSON.stringify({ protected: true }); if (joined.endsWith('/protection')) return JSON.stringify({ required_status_checks: requiredStatusChecks }); From f622c5ecf3cc9950e16d3a0834ecf998e3340623 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 20:37:51 -0700 Subject: [PATCH 9/9] Give the measured per-call time of the pinned-check read (#176 review round 8) Co-Authored-By: Claude Opus 5.5 --- docs/implementation/guarded-merge.md | 2 +- github/merge.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/implementation/guarded-merge.md b/docs/implementation/guarded-merge.md index d0d6844f..cc889364 100644 --- a/docs/implementation/guarded-merge.md +++ b/docs/implementation/guarded-merge.md @@ -56,7 +56,7 @@ What this covers, each checked against GitHub: - A suite with many other runs. Only the pinned name is asked for, so a suite with more than 100 runs (microsoft/vscode) does not matter. - A re-run inside one suite. `LATEST` leaves out the older run (codeabovelab/codeboost PR 151: `test` failed, then passed when re-run). Runs of the name in several suites are ordered as the rollup's are. -One call holds up to 25 pinned checks, so most inspections make one call of about one second. A check whose app has more than 100 suites on the head is read in pages of 100, up to 10 pages. For a fresh inspection, the deadline ends the read first: each call takes about a second, so its six seconds fit only a few pages after the other reads. The display inspection has 12 seconds, which fit about nine. Either way, a head with more suites than fit fails closed with “GitHub merge-state inspection timed out.” +One call holds up to 25 pinned checks, so most inspections make one call. A call takes about one second when the head has a few suites from the app, and two to three seconds for a full page of 100 (measured with gh 2.102.0 on vite, cli/cli and next.js). A check whose app has more than 100 suites on the head is read in pages of 100, up to 10 pages, but the deadline ends the read first. A fresh inspection's six seconds fit about one full page after the other reads; the display inspection's 12 seconds fit three or four. Either way, a head with more suites than fit fails closed with “GitHub merge-state inspection timed out.” Checks that any app may report are still read from the rollup alone. diff --git a/github/merge.ts b/github/merge.ts index ebb5b71e..6d7de50b 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -380,9 +380,10 @@ export class GhMergeGateway implements MergeGateway, MergeQueueGateway { * suite whose `app` GitHub hides from this reader (a private app), which an app read from the answer would not name. * Only each suite's latest run of a name is asked for (`checkType: LATEST`): a run re-run inside its suite is left out. * Each check is one alias in the query, with up to PINNED_CHECKS_PER_READ checks per call; a check whose app has more - * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. That cap only bounds the work: at about a second - * per call, the inspection's deadline usually stops the read first (after a few pages in a fresh six-second inspection, - * about nine in a 12-second display one), and the inspection then fails with its timeout. + * than 100 suites on the head is paged, up to PINNED_SUITE_PAGES pages. That cap only bounds the work: a call takes about + * a second for a few suites and two to three seconds for a full page of 100 (measured with gh 2.102.0), so the + * inspection's deadline stops the read first (after about one full page in a fresh six-second inspection, three or four + * in a 12-second display one), and the inspection then fails with its timeout. * * The read fails closed. A gh failure keeps gh's error. GitHub's own errors in an answer, a malformed field, an answer * for another commit, a suite with more runs of the name than one page holds, more suites than the page limit, and