Skip to content

perf(pr-merge): start one full run inside the cap for uncovered lane merges - #10853

Merged
kyle-sexton merged 2 commits into
mainfrom
perf/pr-merge-cap-bypass-uncovered-lane-merges
Oct 29, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
perf/pr-merge-cap-bypass-uncovered-lane-merges

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: a lane-throughput fix the coordinating session decided under the owner's standing direction on 2026-10-29. No issue tracks it.

Hold path: .github/workflows/pr-merge.yml is a hold path, so the owner approves the merge. No permission, secret, token or trigger change.

Decision and its basis. The coordinating session relayed this decision under the owner's standing direction, re-pasted 2026-10-29 at about 03:05Z:

  • §1: decide from evidence.
  • §3: runner minutes on this public repository are not a cost.
  • §4: throughput comes first.

Quoted from the relay: "build the full-run cap bypass now ... #10491's basis was runner-minutes, which §3 no longer counts as cost, and the cap now limits lane throughput (arming stops at max-in-flight until a full run verifies the batch)." The 30-minute cap came from #10491 (efficiency plan item 5), which cut main's full runs to at most 48 a day to save runner minutes.

Reversible: set merge.full-run-cap-bypass: false in docs/conventions/pr-pipeline.yaml.

Summary

The lane arms nothing past merge.max-in-flight (5) until a full run of main verifies the merges in flight. post-merge-dispatch starts at most one full run per 30 minutes (CAP_MINUTES). A batch that lands just after a run started therefore waits out the cap, then the next 15-minute tick, then the run itself.

The coordinating session's measurement, UNVERIFIED by me: 5 merges at 00:51, and the room still read 5/5 at 01:23:38 (run 40799692656). That is about 55 minutes per 5 merges.

With this change, the first tick after the batch lands starts one run of main's tip inside the cap. The batch is verified when that run ends.

Fix

New step, Find lane merges no full run covers. It runs in post-merge-dispatch only when the cover read reports capped and the post-merge check judged (exit 0 or 5). It sets bypass=true when the post-merge report lists an unverified lane merge that meets both conditions:

  • no dispatch run is under way for it: its pending holds dispatch-run-pending, and no dispatch-run-in-progress: or attribution-pending: entry;
  • the cover listing has no non-cancelled run created at or after the merge on a commit that contains it. The job's full-history .base checkout answers containment.

It fails closed toward the cap:

  • A run at a commit the checkout lacks is newer than the checkout, so it counts as cover.
  • A merge the checkout lacks, or one older than every listed run, is skipped.

Once a run starts, the next tick finds it in the listing, so the same batch never starts a second run.

Changed condition on Start the full CI run on main: (capped != 'true' || bypass == 'true'), and bypass also counts as a reason to dispatch. Kill switch, report-only and rate-limit stops gate the dispatch exactly as before.

Cover step, in both jobs: it writes its listing to $RUNNER_TEMP/cover-runs.json. The two bodies stay identical, as their test requires.

Config: new key merge.full-run-cap-bypass in pr-pipeline.yaml and its schema, default true, absent read as true. It is placed beside report-only, away from max-in-flight, which a separate PR raises from 5 to 10.

Unchanged:

  • dispatch-full-run keeps the cap. It has no post-merge report to name lane merges, and its sparse checkout has no history to judge coverage. The post-merge-dispatch tick, 15 minutes at most, covers its batches.
  • The concurrency groups from perf(pr-merge): batch main's whole-suite runs and stop sleeping after the dispatch #10491 (pr-merge-verify-main and pr-require-checks' pr-require-checks-main-dispatch, neither cancelling in progress) still collapse a burst to one run going and one waiting.
  • No workflow_run trigger. A full run started by GITHUB_TOKEN raises no such event. Since 2026-10-28T06:00Z, 1 of 33 completed bot-dispatched full runs had a post-merge-demotion-alert workflow_run run within 3 minutes.

Verification

  • node --test .github/scripts/pr-merge/workflow.test.mjs .github/scripts/pr-merge/merge-job.test.mjs passes: 172 tests, 0 failures. The new step's script runs against a real git history (BEFORE, MERGED, TIP, plus a SHA the checkout lacks) and a report file. The expected values come from the rule above:
    • inside the cap, an uncovered batch sets bypass once;
    • a run created after the merge at MERGED, TIP or an unknown SHA covers it;
    • a run after the merge on a commit without it, or a run at TIP created before the merge, covers nothing;
    • the second tick in the window finds the first tick's run and starts none;
    • no lane merge, a verified merge, a run in progress, an attribution pending, or only the canary pending keeps the cap;
    • the key set to false keeps the cap, and an absent key means on;
    • a merge older than every listed run is skipped;
    • the dispatch condition fires on capped plus bypass, and not under report-only or a rate-limited read.
  • Each of six mutants fails a test: no ancestry check; no created-after check; no in-progress filter; no config read; no oldest-run skip; the dispatch if without the bypass.
  • actionlint is clean, scripts/check-convention-yaml.sh validates all 11 convention YAML files, and scripts/check-changelog-fragments.sh --check passes.

Cost:

  • Full runs: at most one extra per 15-minute tick, so at most +48 a day over the cap's 48. At the 84.1 runner-minute median from perf(pr-merge): batch main's whole-suite runs and stop sleeping after the dispatch #10491 that is at most +4,040 runner-minutes a day. Under §3 that is not a cost here, but each full run does occupy org job concurrency while it runs.
  • GITHUB_TOKEN: about 0 per extra full run. The 4 pr-require-checks workflow_dispatch runs in the 2026-10-28 ledger logged github-token=0.
  • Per extra run: one dispatch POST and one record upload. The new step itself makes no API call; it reads files and git.
  • post-merge-dispatch: no extra runs, since no trigger is added. At most about +2 calls per hour, against the ~32 per hour that perf(pr-merge): carry completed run attempts across post-merge-dispatch ticks #10834 (the attempt carry) already saved in this job.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 28, 2026 23:06
…merges

The lane arms nothing past merge.max-in-flight until a full run of main
verifies the merges in flight, and main's full runs are capped at one per
30 minutes, so a batch that landed inside the cap waited it out. A new
post-merge-dispatch step lets the cap give way when the post-merge report
lists an unverified lane merge that no started full run covers: one run
of main's tip for the whole batch. merge.full-run-cap-bypass (default
true) turns it off. dispatch-full-run keeps the cap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 29, 2026 03:22
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T03:24:06.960191Z aa77200 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed aa77200baf4469f9ac34bdce74a80f29d464b91e: 0 finding(s), 0 as inline comments.

No security findings. critical-findings: 0

I read the full diff (6 files) and the PR context. .github/workflows/pr-merge.yml, docs/conventions/pr-pipeline.yaml and the tests are in scope. I also checked the neighbouring post-merge step and the .base checkout in pr-merge.yml.

  • No merge hold is weakened. The new bypass step only makes post-merge-dispatch start a full CI run of main's tip inside the 30-minute cap. A full run is the verifier that releases the lane's in-flight room, so starting one earlier does not skip or narrow a hold. The merge-holds.mjs, enable-auto-merge.mjs and verifier-answer paths are untouched. A merge is still marked verified only by a completed full run.
  • The dispatch condition keeps its other gates. The changed if: on Start the full CI run on main keeps the kill switch (steps.kill.outputs.proceed), report-only (steps.mode.outputs.live) and the rate-limit stop (steps.cover.outputs.rate-limited != 'true'). It only loosens the capped clause to capped != 'true' || bypass == 'true'. merge.full-run-cap-bypass defaults to true, and false restores the old behaviour.
  • The new step's inputs are controlled. It reads post-merge.json (the lane's own report) and cover-runs.json (the listing of main's full runs). It also reads .base/docs/conventions/pr-pipeline.yaml, which comes from the base checkout (path: .base, fetch-depth: 0, pr-merge.yml lines 145-146), not from PR code. Report fields are regex-validated before use: PR number ^[1-9][0-9]*$, SHA ^[0-9a-f]{40}$, and a timestamp pattern. They are passed to git as arguments, never to eval or a shell string, so there is no injection path. A missing report, listing or commit makes the step fall through to the cap.
  • The workflow's trust surface is unchanged. No permissions:, token scope, trigger, secret or workflow_run change, and no change to the trusted-actor, kill-switch or token-broker controls.
  • The merge setting is a throughput toggle. full-run-cap-bypass is a merge setting in pr-pipeline.yaml. Read as a hold control, it widens nothing. It gates when a verifying run starts, and the verification itself stays.

Spend, not a hold: the change can start up to one extra full run of main per 15-minute tick (about +48 a day). That uses runner minutes and org job concurrency on a public repository, with no billed resource, and dispatch-full-run keeps the cap.

I did not run the tests or the workflow. This is a read-only review.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed aa77200baf4469f9ac34bdce74a80f29d464b91e: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and the PR context, and checked the new step against pr-merge.yml and babysit_postmerge.py. I did not run the tests. The PR's own claims (172 passing tests, mutant results, the 55-minute measurement) are author-claimed and unverified by me.

What I checked:

  • Report fields. The autoMerges fields the step reads (number, mergeCommit, mergedAt, verified, pending) are the ones babysit_postmerge.py emits at report(). The pending strings it filters on (dispatch-run-pending, attribution-pending:<id>, <signal>-run-in-progress:<id>) match what the code appends.
  • Inputs. code is a post-merge step output, and post-merge.json is written to $RUNNER_TEMP before the new step runs. The cover step now writes cover-runs.json after its failure paths, so the new step finds no stale or partial file there.
  • Dispatch gating. The dispatch step keeps its kill-switch, live-mode and rate-limited != 'true' guards. The bypass step's if requires capped == 'true', which only a cover step that ran and read successfully can set. A failed cover read therefore skips the step.
  • Repeat starts. After a bypass dispatch, the next tick's listing holds a non-cancelled run on main's tip, created after the merge. It covers the merge, so the same batch never starts a second run. A report reused from an earlier tick can list a merge as still pending, but the same listing covers it.
  • Cap-bypass-off path. The key is read with yq. An absent key prints null, not false, so absent means on, as documented. A bypass-step failure is not a concern: it exits 0 on missing files and on false.
  • Failure direction. A merge the checkout lacks, or one older than the oldest listed run, is skipped. A run at a commit the checkout lacks counts as cover. Both lean toward keeping the cap.

Spend: this adds at most one extra full run per 15-minute tick on a public repository, which is runner minutes only, and no paid plan or billing setting changes. This is not a hold.

Trusted path .github/workflows/pr-merge.yml and docs/conventions/pr-pipeline.yaml changed. I found no permission, token, secret or trigger change in either.

@kyle-sexton

Copy link
Copy Markdown
Member Author

Owner decision record (R19). In the poteto session on 2026-10-29 at about 03:05Z, the owner answered my hold on this cap with "REMEMBER:" and re-pasted the standing direction, including:

When the evidence settles a question, act on it. Do not ask me.

Signal me [...] only for: 1. A change to the original intent or vision. 2. A finding during implementation [...] that changes the intent, vision or primary design. 3. A real HIGH/CRITICAL security or cost risk

Not cost, so spend it freely: model usage under the Claude subscriptions [...]. Cost, which comes to me: [...] GitHub (larger runners, Actions minutes or storage on private repositories [...])

Aim for the highest throughput with the least friction that is safe

The session applied it to this cap. #10491's basis was runner minutes, which are not cost on this public repo. The bypass costs about 0 GITHUB_TOKEN calls (full runs measured github-token=0), so it touches no rate-budget safety limit. The independent security review of aa77200baf found no HIGH or CRITICAL. The change is reversible: set merge.full-run-cap-bypass: false.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 60be258 Oct 29, 2026
77 checks passed
@kyle-sexton
kyle-sexton deleted the perf/pr-merge-cap-bypass-uncovered-lane-merges branch October 29, 2026 03:37
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