Skip to content

perf(pr-merge): reuse the last tick's post-merge report on an idle tick - #10475

Merged
kyle-sexton merged 4 commits into
mainfrom
perf/postmerge-idle-tick
Oct 27, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
perf/postmerge-idle-tick

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: efficiency plan item 1 alternative 2 (idle-tick exit)

Summary

pr-merge's post-merge-dispatch job runs "Check earlier lane merges" in full on every scheduled tick. Over the 72 schedule runs from 2026-10-26 13:57Z to 2026-10-27 13:00Z it took a median 375 s (p90 509 s), and a sample tick made 114 gh calls. That includes ticks with no new lane merge since the tick before: 10:59, 11:23, 11:36, 11:47 and 11:58Z on 2026-10-27 all judged the same 178 merges at main 3c97a87086, each with exit 0.

With this PR, an idle tick reuses the report the last tick kept and skips the check. A live probe of the reuse decision on a real 184-merge report took about 15 s and 15 calls (settle 2 s and 3 calls, idle_refusal 13.3 s and 12 calls).

Fix

  • Reading the last report. .github/scripts/pr-merge/last-report.mjs downloads the pr-merge-post-merge-report artifact. It takes the artifact from the newest scheduled or hand-started run of main's pr-merge.yml, never this run, and only when that run's head commit is on main. This is the same newestTrustedRun + onMain rule the lane records use, so trusted provenance is unchanged. It shares one run listing with the lane-records read (PR_MERGE_READS_PATH). Any error leaves no report, and the check runs. An in-progress run already lists the artifact: run 40521502018 showed expired=false while its merge job was still running.
  • The reuse decision. babysit_verify_evidence.py settle --idle-tick applies the lane room's rule first (settle_shared). That rule covers identity, lane, floor, config, check code, age, a new lane merge at a moved head, and the demotion. The report must then also pass babysit_postmerge.idle_refusal, which covers the rest of what check reads that can change without a merge. See the input table below.
  • The workflow step. post-merge-dispatch reuses a report only when the last check passed (exit 0 or 5) and settle exits 0. It keeps the report's computedAt, code and code commit as they were, so a chain of reuses ends SHARED_MAX_AGE after the check that made the report. It writes this run's freshly read lane-records.json beside the report, not the last run's copy. Any other code, refusal or crash runs the check exactly as before.
  • Pins in the report. The post-merge report now records each pin it read (pins), so a later tick can read them again.
  • Run listings. pending_runs_unchanged and the idle rule share _verifying_runs_unchanged, which lists runs once for each verifying workflow. The old listing covered every workflow on main and held 152 runs in under two hours, more than one page. Behavior change: because the listing is now whole, the merge job and lane room keep more unverified-stale (code 5) reports.
  • Expired excuses. shared_refusal (_shared_age_refusal) now refuses a report once a quarantine excuse it carries has expired (excuse-expired). This tightens the merge job and lane room as well.
  • Risk floor. last-report.mjs is added as gate code on the risk floor (owner decision Q26: every pr-merge script is gate code). It is listed in merge-holds.mjs, babysit_risk.py, the pr-require-checks.yml hold patterns and both hosted review lanes. The floor count goes from 51 to 52. Nothing is loosened: no trusted-actor rule, token permission, SHARED_MAX_AGE or SHARED_UNVERIFIED_MAX_AGE changes.

Fingerprint inputs and where they are checked

Input check reads Covered by
Merges judged (main head, new lane merges) settle_shared → shared_refusal (new-merge:#N, merged-unreadable)
Floor (config commit, demotion clear) shared_refusal floor-differs / config-unreadable; demotion-changed, kept only on an equal read_demotion
Check code check_code_unchanged (code-changed)
excusesExpireAt, quarantine excuses idle_refusal refuses any excuse (excused, a missing key too) and any quarantines; shared_refusal excuse-expired
Quarantine issues (_quarantine_issues) idle_refusal quarantine-changed: any issue with the marker touched since computedAt (a full page counts as touched)
Pending runs, lane re-run candidates, baselines idle_refusal runs-changed: no full or canary run completed since computedAt among runs created since the earliest unverified merge or the _lane_reruns lookback plus one FULL_RUN_ALLOWANCE
Dispatch deadline (time) idle_refusal dispatch-deadline
Pins (read_pin): edits, new pins, deletions idle_refusal re-reads every recorded pin (pins-changed)
Side effects (dispatch, re-run, hold) idle_refusal refuses needsDispatch, reruns, hold, unattributed, a lane-rerun-unreadable warning, and a non-passing verdict
Downstream reuse (gate assess, lane room) IDLE_HEADROOM (10 min) short of SHARED_MAX_AGE (stale-soon)

Every read failure refuses (unreadable: …), and the step then runs the full check.

Verification

  • Fail-first: the new tests were copied onto origin/main (ae46394c7f). There, an idle tick reuses the last run's report… failed because the check ran again, and post-merge-dispatch reads the last run's report… and last-report.test.mjs failed. In Python, all IdleRefusal and idle SettleCommand cases failed, as did test_an_answer_past_its_quarantine_excuse_is_refused and test_the_report_records_each_pin_it_read_once.
  • Per input, in test_babysit_lane_room.py:
    • IdleRefusal covers each report field, headroom, a full or canary run completed since, a listing that is not whole, the pending-merge floor and deadline, a quarantine-marker issue or a full issue page, and a pin edited, deleted or unreadable.
    • The inputs settle_shared covers are tested in the existing SettleShared tests: test_a_moved_head_with_a_lane_merge_the_answer_does_not_count_is_refused, test_a_changed_demotion_is_refused_unless_it_reads_the_same, test_an_answer_from_other_check_code_is_refused_before_any_keep and test_a_stale_answer_is_refused_even_when_nothing_moved.
  • python3 -m unittest discover over the babysit-prs scripts: 3197 tests OK. node --test .github/scripts/pr-merge/*.test.mjs: 531 tests pass. scripts/ci-review-lanes.test.sh passes. actionlint is clean. Ruff runs through scripts/run-ruff.sh.
  • Live probe (local, CI arguments): the check exited 1 after 536 s and 427 calls. Local runs read artifact listings that CI carries in its lane records. The exit 1 comes from a real failure on perf(guardrails): read the neutral commit convention once without per-key grep spawns #10421 (attempt 1 failure; re-run cancelled), which CI's 14:00Z tick also reports with exit 1. settle --idle-tick refused it as not-passing in 2 s. Run with the failure and excuse cleared, idle_refusal passed in 13.3 s and 12 calls, and the 177 re-read pins matched the recorded ones.
  • Expected saving: a reused tick replaces the check (median 375 s, ~115 calls) with about 15 s and 15 calls. Replaying the 72 runs, about 10 ticks reuse, saving about 21 minutes of check time a day. This is a lower bound, because the replay counts only ticks at the same run head SHA, and settle also accepts a moved head with no new lane merge. Two of those 10 would end the merge gate at 30.6 and 31.4 minutes of report age, where the gate reads the check again. On 21 of the 72 ticks the gate already ends past 30 minutes today.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 27, 2026 10:41
post-merge-dispatch's "Check earlier lane merges" ran in full on every
tick (median 375 s over 72 ticks, ~115 gh calls), including ticks with no
new lane merge since the tick before. It now reads the report the last
trusted run on main kept (last-report.mjs, found as the lane records are)
and reuses it when `settle --idle-tick` keeps it: the lane room's rule
(settle_shared) plus idle_refusal, which covers the rest of what the
check reads that can move without a merge (verifying runs completed
since, quarantine issues, pins, quarantine excuses, the dispatch
deadline, anything to start). The report keeps its computedAt, code and
commit, so reuse ends SHARED_MAX_AGE after the check that made it, with
10 minutes left for the merge gate and lane room.

pending_runs_unchanged now lists runs per verifying workflow, and the
reuse rule refuses a report past an expired quarantine excuse.

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

chatgpt-codex-connector Bot commented Oct 27, 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-27T15:46:51.035708Z ddd88bd 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

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review of #10475 at fe29d06: no security findings.

critical-findings: 0

Plugin manifests: the diff changes no plugin.json or marketplace.json, so no manifest <field> lines apply.

I read the whole change for every trusted path the diff touches. These are pr-merge.yml, pr-require-checks.yml, last-report.mjs, merge-holds.mjs, babysit_risk.py, babysit_postmerge.py, babysit_merge_policy.py and babysit_verify_evidence.py. I did not review the other files, which are tests, docs and the review-lane path lists. I judged what each gates from the code alone, not from the PR text.

Is any merge hold weakened? I judged no.

  • The new report source is trusted. last-report.mjs takes the report artifact only from the newest scheduled or hand-started pr-merge.yml run on main, never the current run, and only when that run's head commit is on main (newestTrustedRun plus onMain). That is the rule the lane records already use, so no new writer can plant a report. gh run download goes to a .part directory that is renamed into place, and any failure leaves no report, so the full check runs.
  • A reused report has to pass more than before. settle --idle-tick applies settle_shared (identity, lane, floor, config, check code, age, new merges, demotion) and then idle_refusal. Each refusal leaves the full check to run, and check_code_unchanged rejects a code-sha mismatch before any read. The workflow reuses a report only for exit 0 or 5, and the original computedAt, code and commit are kept, so a chain of reuses cannot outlive SHARED_MAX_AGE.
  • The reuse rule covers what _judge reads that can change without a new merge. I traced _judge against it.
    • A pending full run in a merge's window leaves that merge unverified, and the earliest unverified merge sets the run-listing floor.
    • A canary still running in a verified merge's window is recorded as outstandingSince, and the listing reaches back to it.
    • Red-but-unattributed runs, reruns, quarantines and excuses, a missing dispatch past the deadline, and edited, new or deleted pins each refuse.
    • A full issue page or a full run listing refuses, and a listing whose total_count is not the number of runs returned raises.
  • Two behaviour changes go the other way.
    • shared_refusal now refuses an expired quarantine excuse (excuse-expired), which tightens the merge job and lane room.
    • pending_runs_unchanged now lists the full and canary workflows separately, so a busy main's second page no longer forces a re-check. The merge job and lane room therefore keep more code-5 reports, as the PR says. I found no gap in the per-workflow listing: it filters by branch, created floor and run path, and refuses when the page is not whole.
  • The step in pr-merge.yml passes nothing attacker-controlled to the shell. The step uses only github.token and the repository. The artifact's post-merge-code and post-merge-code-sha files are matched against ^[05]$ or passed as a quoted argument. Those files come from a trusted main run.
  • Risk floor. last-report.mjs is added consistently to merge-holds.mjs, babysit_risk.py, the pr-require-checks.yml hold patterns and both hosted review lanes, and the tests expect 52 floor classes. No trusted-actor list, token scope, kill switch, broker rule or pr-pipeline.yaml setting changes.

Residual risks, not findings:

  • Quarantine check may rarely pass. _quarantine_touched treats a full 100-item issue page as touched. On a busy repository it may nearly always refuse. That costs efficiency, not safety, because it fails closed.
  • Reuse is only as good as the field list in idle_refusal. A field a future check reads but idle_refusal does not would let a stale report through. The PR's input table is the contract to keep in step.

Cost: this change only shortens or skips existing Actions work and adds no paid plan or billing setting, so no cost hold.

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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

No findings. I read the full diff for the non-test files and the cross-file code they depend on. I did not run any tests: this session had no shell, so the test results in the PR description are author-claimed and unverified.

What I checked

  • last-report.mjs
    • It reuses newestTrustedRun with the onMain accept rule that lane-records.mjs uses, and it excludes the current run.
    • It deletes any leftover report directory before the read and downloads into ${dir}.part, then renames it, so a failed download leaves no report.
    • Every error is caught, a warning is logged, and the step exits 0, so the check runs as before.
    • GH_TOKEN is the job's actions: read token, and GITHUB_REPOSITORY is validated against a strict pattern.
  • Workflow step in pr-merge.yml
    • Reuse happens only when the last check's exit code matches ^[05]$ and settle --idle-tick exits 0.
    • Otherwise code stays empty and the check runs unchanged.
    • A reused report keeps its old computedAt and code commit, so a chain of reuses cannot outlive SHARED_MAX_AGE.
    • An empty --code-sha cannot pass check_code_unchanged, so a missing sha file also falls through to the check.
  • idle_refusal
    • A report missing excusesExpireAt, pins or outstandingSince is refused, so reports from before this PR never get reused.
    • read_pins returns a value or None for every requested number, and a missing pull request raises. The recorded-versus-reread comparison is therefore like for like.
    • The ValueError from int(n) is caught and refuses, and _time raises a caught error type.
    • A full issue page counts as touched, and a listing whose total_count differs from the run count refuses. Both fail closed.
  • _shared_age_refusal / excuse-expired
    • computed_at + age is the current time, so the check compares correctly, and an unreadable timestamp refuses.
    • This tightens the merge job and the lane room, as the PR says.
  • check() pins refactor
    • The eager pins dict is built under the same condition as the old inline expression, so the set of reads is unchanged.
  • Risk-floor sync
    • last-report.mjs is added to merge-holds.mjs, babysit_risk.py, the pr-require-checks.yml hold patterns, and both hosted review lane lists. Those lists match each other.

Closing issues: none (the PR states "No related issue"), so there are no acceptance criteria to check.

Manifests: no plugin.json or marketplace.json change. Both changelog fragments use bump: patch, which fits a behavior-preserving perf change that adds a report field and tightens a refusal.

Cost: the change reduces Actions minutes and gh calls on idle ticks. Nothing here can start a billed resource, so there is no hold.

Trusted paths: this PR touches merge-holds.mjs, pr-merge.yml, pr-require-checks.yml, babysit_risk.py, babysit_postmerge.py, babysit_merge_policy.py and babysit_verify_evidence.py. I read each of those changes in full. I found no loosened trusted-actor rule, token permission, or age limit. The new reuse path adds refusals on top of settle_shared and does not replace any.

Minor, not counted: the README sentence in docs/conventions/pr-pipeline/README.md around line 1229 now reads awkwardly, because the settle --idle-tick clause is wedged before lane-room. It is a wording issue only.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ddd88bdfed

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kyle-sexton and others added 2 commits October 27, 2026 15:36
…indow

An idle tick could reuse a passing report while an older canary in a
verified merge's window was still queued: `_judge` verifies on a later
green canary, and the queued one fails the merge once it turns red. Only
unverified merges moved the run listing's floor past the fixed lookback,
so a canary created before it was never listed and the reuse stood.

The post-merge report now records when the earliest canary still running
in a merge's window was created (`outstandingSince`). `idle_refusal` and
`pending_runs_unchanged` move their listing floor back to it and refuse a
report without the field. No extra API calls: the floor moves inside the
same two per-workflow listings; a listing that no longer fits one page
reads as not whole and runs the full check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit f70a780 Oct 27, 2026
49 checks passed
@kyle-sexton
kyle-sexton deleted the perf/postmerge-idle-tick branch October 27, 2026 20:32
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