Skip to content

A gate added to main never applies to a PR that was already green — it is absent from the PR's workflow and skipped in the queue #3508

Closed

Description

@rbuergi

A gate added to main never applies to any pull request that was already green

Measured tonight on a concrete instance, and it is a general property of how the pair-gate family
is triggered rather than a bug in any one gate.

The two exits, and why they close on the same PR

cross-repo-pair (and its siblings) carry, deliberately and with the reasoning written down:

if: ${{ github.event_name == 'pull_request' && … }}

"A merge_group build has no single pull request, so it is omitted there and the weaker rule
applies; nothing reaches the queue without the pull_request run being green."

That assumption is about the gate's verdict. It does not hold when the gate did not yet exist:

  1. On the pull_request run — the run executes the workflow from the PR's merge ref at the time
    it ran
    . A job added to main afterwards is simply not in that file, so it never runs.
  2. On the merge_group run — the job is skipped by the if: above, and collect-results'
    guard carries the identical predicate (correctly — so the two can never disagree about who is
    exempt), so a skip is not a failure.

Both exits close on the same pull request, and the result is that a gate lands and every PR
already green at that moment is permanently exempt from it — silently, and invisibly from the PR.

The instance, measured 2026-09-07

gate This diff does not shrink what the surface detector can see (#3492, merged as #3500 1340e351c)
the PR it should have covered #3503 — the one pull request that night that actually changes the parser
its pull_request run job Cross-repo pair (public surface) ran with steps 6–10 only; the new steps 8 and 12 are absent — the branch was cut before #3500 landed
its merge_group run (gh-readonly-queue/main/pr-3503-…, run 34067302142) the whole job is completed/skipped
net the gate ran on neither

No harm in this instance, and that is the point: the author had run the comparison by hand and put
the table in the PR body (all four denominators unchanged, two new). That is authorship luck, not a
property of the system. The gate is live and non-vacuous — confirmed on #3505's run, which executed
parser-delta self-test: 13 case(s), 0 failed and the delta step's byte-identical proof path — so
this is purely about which pull requests it reaches.

Why this is the sibling of #3504, not a duplicate

#3504: a repo that hand-rolled a lane opts out of every guard that lane grows later.
This: a pull request already green opts out of every gate the workflow grows later.

Same shape one level down — a guard added later does not retroactively cover what was already in
flight
— and both are invisible from the side that is uncovered.

A cheap fix for at least one of them

🚨 check-parser-delta.py needs the pull-request body only to accept a declared decrease. It
therefore CAN run on merge_group, without --pr-body-file, in its strict form: refuse any
decrease, accept any increase.
That is the conservative direction and the right one for a queue
build — nothing should shrink the detector at the moment it lands, declaration or not, and a PR that
legitimately declared a decrease has already been read by a human on the pull_request run.

The same test applies to the other gates in the family, per gate: does this gate need the PR body,
or only the diff?
Those that need only the diff can run in the queue and close exit (2). Those that
genuinely need the body — cross-repo-pair reading Pairs-with:, interface-addition reading
Implementers: — cannot, and for those the honest answer is exit (1): when a gate lands, the pull
requests open at that moment have not been checked by it
, and someone has to decide whether that
matters enough to ask them to re-sync.

Related: #3504 (the same shape across repos), #3492 / #3500 (the gate in question), #2689
(the pair gate's design and the sentence quoted above).

Activity

  1. rbuergi commented on Sep 6, 2026

    @rbuergi
    ContributorAuthor

    Not tonight — the fix is a workflow structural change and the queue is landing a release

    Filing the finding is safe; acting on it is not, at this hour.

    Closing the merge_group exit means adding the parser-delta invocation to a job that actually runs on that event — a change to the shape of dotnet-test.yml, not a change inside a script. The merge queue is currently landing the two preconditions the 3.0.0 tag is gated on, and a workflow edit that goes wrong there does not fail politely: it ejects every queued entry and every one rebuilds.

    Same reasoning as the hold on #3378. This is a good, bounded piece of work with an obvious acceptance test — watch a merge_group build actually execute the step, and watch it RED on a planted decrease — and it should be done on a quiet main, by someone who can afford for the first attempt to be wrong.

    🚨 One thing to carry forward when it is picked up: the acceptance test above is the whole point. This issue exists because a gate was added and then ran on neither of a pull request's two builds, and nobody noticed until the runs were read step by step. A fix for that which is verified by "the workflow file now mentions it" would reproduce the defect it closes.

  2. rbuergi commented on Sep 7, 2026

    @rbuergi
    ContributorAuthor

    Fixed and closing — the parser-delta control now runs in the merge queue (#3549, merged 6f47c295f)

    Done on a quiet main, as the hold asked, and verified by the acceptance test this issue named
    rather than by "the workflow file now mentions it".

    The change. check-parser-delta.py moved out of cross-repo-pair (pull_request only) and
    into record-signatures — the job whose if: is already
    github.event_name == 'pull_request' || github.event_name == 'merge_group'. The strict/declaring
    split is written on the event, the same shape the two gates beside it already use for --pr:

    event form
    pull_request --pr-body-file supplied — a declared decrease is honoured
    merge_group no body — STRICT: any decrease is refused, declared or not

    Every build gets exactly one of the two, so there is no path on which neither runs. The move also
    closes two exits nobody had counted: record-signatures carries no fork and no dependabot
    clause
    , and cross-repo-pair carries both.

    ① It runs on merge_group — observed, on this pull request's own queue build

    Run 34097613066,
    gh-readonly-queue/main/pr-3549-…, job Public surface (binary compatibility):

    7. Prove the parser-delta control is not vacuous (self-test)   [completed/success]
    8. This diff does not shrink what the surface detector can see [completed/success]
    

    and from that job's log, the strict arm identifying itself:

    Event is 'merge_group', which carries no single pull request:
      the STRICT rule applies — any decrease is refused, declared or not.
    The public-surface detector is byte-identical at the merge base and in this diff:
      scripts/check-type-forwards.py   sha256:864c23442e47ec1b
      scripts/transitional_allow.py    sha256:956c537e907c9f7b
    

    For the control arm — the old placement, on the same queue events — Cross-repo pair (public surface) reads completed/skipped, here and on #3546's queue build
    (34093021863), which is
    exit (2) measured directly.

    ② It can still FAIL — planted, watched red, then watched green again

    Each row executed the step's own run: block extracted from dotnet-test.yml, not a
    restatement of it:

    arm result
    clean tree, EVENT=merge_group exit 0 on the byte-identical proof, both digests printed
    a planted blindness in the detector's git-tree reader (MeshWeaver.Layout/ skipped), merge_group exit 1 — publicTypesAtBase 1975 → 1710 (−265), plus −1817 members / −11 interfaces / −46 obligations
    the same plant, pull_request, empty body exit 1, identical verdict
    the same plant, pull_request, body declaring all four counters exit 0, ↓ DECLARED in the pull-request body — the declaring arm is not lost
    plant reverted exit 0 again

    Cost: 3 s when the detector changed, and a printed SHA-256 proof when it did not.


    Exit (1) is untouched, and cannot be closed. A pull_request run executes the workflow file as
    it stood on the merge ref when it ran, and re-running re-runs the same file. So the honest answer
    stands: when a gate lands, the pull requests open at that moment have not been checked by it — the
    queue build is the one that now catches them.

    🚦 One trade, stated rather than hidden: a declared decrease is honoured on the pull request and
    refused in the queue, so a genuine parser tightening lands only if it does not shrink a
    published counter. That is deliberate — the queue is where the change becomes main, and a
    declaration read there would have to be attributed across a multi-entry group carrying several pull
    requests' diffs at once.

    The other gates in the family were tested against your question — does this gate need the PR body,
    or only the diff?
    — and cross-repo-pair (Pairs-with:), package-pin-removal (Satellite-pins:)
    and interface-addition (Implementers:) all genuinely need the body, so exit (2) stays open for
    them by construction.

    Written up in
    Doc/Architecture/CrossRepoPairGate
    → "A gate added to main reaches NEITHER build of a pull request already green". #3504 (the same
    shape across repos) is untouched and still open.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions