Skip to content

feat(github): revalidate stored check state at merge-queue admission - #977

Draft
aparajon wants to merge 2 commits into
armand/check-plan-time-holdfrom
armand/merge-group-admission
Draft

feat(github): revalidate stored check state at merge-queue admission#977
aparajon wants to merge 2 commits into
armand/check-plan-time-holdfrom
armand/merge-group-admission

Conversation

@aparajon

@aparajon aparajon commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Why this matters

Once a pull request enters a merge queue, branch protection stops evaluating required checks on the PR head and evaluates them on the synthetic merge-group commit instead. SchemaBot posted an unconditional pass on that commit, on the rationale that the PR-head check already gated queue entry — so anything that turned blocking after entry had no way to stop the merge: a preflight hold from a sibling apply, a failed apply, a fail-closed re-plan.

"Sibling apply", throughout: two open pull requests that declare changes to the same database in the same environment are siblings. When an apply starts from one of them, the preflight (#940, #941) flips every sibling's stored check to a hold — apply_in_flight_on_target — until the apply settles, because each sibling's plan was computed against a schema that is now changing underneath it. A sibling apply is that in-flight apply, seen from the held PR's perspective.

The same unconditional pass also launders a check that went stale before entry. Confirmed against a running deployment: a pull request whose aggregate check went green kept that green — same Check Run, same completion timestamp — while a second pull request applied a change to the same database, because every stored check update is keyed to its own pull request and nothing fans out to siblings. The queue then admits it.

#942 closed the last first-party gap on the PR head and left merge-time revalidation as future work. This is that work, and it is what makes the merge queue a real gate rather than a formality. Stack 8/8, on top of #942.

What it does

  • postMergeGroupAdmissionChecks replaces the unconditional pass with a re-fold of the queued pull request's stored check state. It identifies the pull request from the merge-group ref, loads its stored checks, and for each aggregate target this instance gates posts success only when nothing blocks for that environment.
  • When something does block, it posts action_required instead, and the queue removes the pull request. Because the queue never re-adds a pull request on its own — and the blocking Check Run lives on the synthetic merge-group commit, where the author won't find it — the block also posts a guidance comment on the ejected pull request (preview below). It is marker-deduped per queue attempt, keyed on the merge-group head SHA: a webhook redelivery never double-posts, while a later re-queue gets a fresh comment at the bottom of the timeline. A comment-post failure retries the delivery; the already-posted checks reconcile idempotently.
  • Fails closed on both uncertainties, for different reasons and with different handling: a merge-group ref that cannot be parsed posts a blocking check (we cannot prove the queued change is safe), while a storage read failure returns an error so the delivery retries rather than guessing a verdict.
  • postPassingAggregateChecks becomes postAggregateChecks(conclusion, onEach) plus a single-target postAggregateCheck. The default-branch push callers stay unconditional, which is still correct — those commits have already merged.
  • Metric: schemabot.merge_gate.merge_group_admission_blocked_total. Any sustained rate means pull requests are reaching the queue and then being ejected, which points at holds landing later than expected rather than at this check.

From webhook to verdict

The merge_group webhook names the queued pull request in its ref — pr-481-… → pull request 481 (a made-up example; parsed from the tail segment, so base branches containing slashes still work; a ref that doesn't fit posts a blocking check rather than guessing):

{
  "action": "checks_requested",
  "merge_group": {
    "head_sha": "0d4f0c9e2ab…",
    "head_ref": "refs/heads/gh-readonly-queue/main/pr-481-9f8e7d6c5b4…"
  },
  "repository": { "full_name": "acme/schema" }
}

Admission then reads that pull request's stored checks — the same rows every other gate reads — and posts the verdict on the merge-group commit:

stored checks for acme/schema#481:
  production/widgets: action_required
  "held: an apply in flight is changing widgets in production"

→ SchemaBot (production) on 0d4f0c9e2ab… : action_required

(nothing blocking → success instead)

Before / after

Two open pull requests change the same database:

  PR A ──► apply starts: a schema change is now running on `widgets`
  PR B ──► its check went green earlier; it enters the merge queue

Before — the queue's question gets an unconditional yes:

  merge queue: "can PR B merge?"
       │
       ▼
  SchemaBot: "yes" — always, no lookup
       │
       ▼
  PR B merges while PR A's apply is still changing `widgets`   ✗

After — the queue's question is answered from PR B's stored check state, where PR A's apply already left a hold:

  merge queue: "can PR B merge?"
       │
       ▼
  SchemaBot re-reads PR B's stored checks
       │
       ├─ PR A's apply still in flight ──► "no" (action_required)
       │       queue ejects PR B + a comment on PR B explains why
       │       and says: queue again once the check is green
       │
       └─ nothing blocking ──► "yes" (success) ──► merge proceeds   ✓

What the ejected pull request sees

The guidance comment posted on PR B, at the bottom of its timeline:

🚦 Removed From Merge Queue

This pull request's SchemaBot check state turned blocking after it entered the merge queue — most often because another change's apply is in flight on a database this pull request also changes, which invalidates the verdict it queued with. SchemaBot posted a blocking admission check on the merge group, so the queue removed this pull request instead of merging it on a stale verdict.

Blocking right now:

  • widgets in production

What happens next

  • Check this pull request's SchemaBot check for the reason. A held check clears on its own: when the in-flight apply settles, SchemaBot re-plans this pull request and refreshes the check.
  • The merge queue does not re-add pull requests on its own — once this pull request's checks are green again, add it to the merge queue again.

How it moves us toward the northstar

The gate's guarantee has been "a passing check means the schema change is applied and verified." Until now that held on the PR head and evaporated at the queue, which is precisely where the merge decision is made. With this, the stored state is authoritative at every point a merge can happen, and the check that gates the merge is the one derived from it.

Worth stating what remains outside this change: a hold landing after admission passes but before the queue merges is still uncovered, since the merge-group Check Run is already recorded by then. The natural follow-up is for the preflight render to flip recorded merge-group Check Runs alongside PR-head ones. Separately, whether a stale PR-head check can arise at all depends on the repository requiring branches to be up to date before merging — a code-host setting SchemaBot neither enforces nor verifies, and worth auditing independently of this change.

The chain: #867 (storage) → #868 (drive-tail recording) → #866 (settle re-plan processor) → #939 (request kinds + hold storage) → #940 (preflight hold fan-out) → #941 (apply-start gate) → #942 (plan-time holds) → this PR (merge-queue admission). Merges bottom-up; each PR retargets to main as its base merges.

🤖 Generated with Claude Code

aparajon and others added 2 commits August 8, 2026 07:42
A PR's required check is evaluated on the synthetic merge-group commit
after it enters the merge queue, but SchemaBot posted an unconditional
pass there — so a preflight hold landing on the PR head after queue
entry could not stop the merge. Re-fold the queued PR's stored check
state at admission instead: identify the PR from the merge-group ref,
post success only when nothing blocks, and post action_required when a
sibling apply's hold (or the PR's own in-flight apply) blocks, so the
queue ejects the PR until its checks turn green again.

Admission fails closed: an unidentifiable merge-group ref posts a
blocking check, and a storage read failure retries the delivery rather
than guessing a verdict.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When the admission check blocks a queued pull request, GitHub removes it
from the merge queue and never re-adds it on its own — and the blocking
Check Run lives on the synthetic merge-group commit, invisible from the
PR page. Without guidance the author watches their merge silently
vanish. Post a comment on the ejected pull request naming the blocked
databases, explaining that held checks re-plan on their own when the
in-flight apply settles, and spelling out the one manual step: queue
the pull request again once its checks are green.

The comment is idempotent per queue attempt — a hidden marker keyed on
the merge-group head SHA deduplicates webhook redeliveries, while a
later re-queue gets a fresh comment at the bottom of the timeline. A
posting failure retries the delivery; the already-posted checks
reconcile idempotently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant