Skip to content

perf(pr-pipeline): carry a deferred lane run's job read in the reconciler - #12752

Merged
kyle-sexton merged 3 commits into
mainfrom
perf/reconcile-lost-runs-carry-lane-jobs
Nov 3, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
perf/reconcile-lost-runs-carry-lane-jobs

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: GITHUB_TOKEN trip 05:38Z attribution (reconcile-lost-runs catch-up)

Summary

GITHUB_TOKEN counter B tripped at 05:38Z on 2026-11-03. In that hour maintenance-reconcile-lost-runs spent 356 calls over 4 runs. The lanes kill switch was on 04:34-04:55Z, so the 04:15Z and 04:38Z runs stopped with 1 call each: run 41579246488 logs check-kill-switch: proceed=false reason=kill-switch and the lanes kill switch stops this run; nothing read or re-run. They were not paced.

  • Run 41580864978 (04:56Z, 163 calls) did the recovery work the switch had left. It made 13 rerun-failed-jobs calls for verdict runs (by design these take no place under the cap), 3 lane and review re-runs, and about 24 first reads of runs the switch had cancelled before they started, which it then settled. It also read 12 re-runnable pr-verify runs and deferred them with all 3 re-run places taken.
  • Run 41581313634 (05:02Z, 99 calls) was a workflow_dispatch by hand, so it did not go through pace.
  • Run 41585026094 (05:38Z, 105 calls) read the same deferred runs again: attempts/{n}/jobs=15, of which about 12 were pr-verify runs still waiting for a place (10 deferred, 2 re-run).

The defect: holdReason reads a lane run's attempt-1 jobs before placement. Only a held run is settled, so a re-runnable run that is deferred is read again on every pass until it gets a place. The file's own rule says attempt-1 jobs of a completed run never change.

Fix

holdReason now takes a lane run still at attempt 1 from the settled-runs file's existing job carry (jobs, keyed id:attempt) while the run listing shows the same head and updated_at. It carries a whole read whose every job's steps listed, using the same checks as the PR-head job carry.

  • A re-run changes the attempt and updated_at, which drops the carry.
  • Act still re-reads each run before it writes.
  • The settled-run rules are unchanged, so SETTLED_VERSION stays 3. The entries keep the existing shape, and a lane run's id never collides with a CI run's.

Call delta:

  • Per pass while a lane backlog drains: about -10 to -12 calls, one attempts/1/jobs read per deferred run, plus annotation reads for any cut job.
  • Trip hour: about -24 of 356 (around 7%), from the 05:02Z and 05:38Z passes.
  • The 04:56Z peak is unchanged, because first reads and verdict re-runs are needed work.
  • Steady state with no deferred lane runs: 0.

Verification

  • New test a lane run deferred for want of a place is not read again while it stays at attempt 1:
    • Pass 1 re-runs 3 of 5 lost pr-verify runs and reads all 5.
    • Pass 2 makes 0 attempts/1/jobs reads and re-runs the 2 deferred runs.
    • A changed updated_at reads the run again.
    • Without the change, pass 2 reads [201, 202] and the test fails.
  • node --test over reconcile.test.mjs, infra-failure.test.mjs and component-status.test.mjs: 177 pass, 0 fail.

Related

  • Per-run readings: the gh-job-calls monitor (local).
  • Not changed here: each pass has a fixed cost of about 21 workflows/{x}/runs listings (9 statuses x 2 lanes, plus intake, verdict and the carry lookup) and 22 head_sha listings. That is the larger steady-state cost. The status split is deliberate (LANE_LIST_STATUSES).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW

kyle-sexton and others added 2 commits November 3, 2026 01:02
…iler

A lost pr-verify or pr-review-main run that is re-runnable is never
settled, so while all 3 re-run places are taken each pass read its
attempt-1 jobs again. After the 2026-11-03 kill-switch window, 10-12
deferred pr-verify runs were re-read on every pass (05:02Z, 05:38Z) until
each got a place.

holdReason now reads a run still at attempt 1 from the settled-runs
file's job carry while its listing shows the same head and updated_at,
and carries a whole read whose every job's steps listed. A re-run
changes the attempt and updated_at, so the carry drops; act still
re-reads each run before it writes.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 09:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 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-11-03T09:11:45.140569Z 3dcf9a4 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude-review: 1 inline finding(s) on 3dcf9a4deaa491e90493079dfe3d8b4abf3a29d4; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 3dba34543a1a52a39912c7d88657519e9fa67c9c: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12752 (incremental, 3dcf9a4d..3dba3454)

Scope: 1 of 1 file in scope, read in full. The only change is in .github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs: one hunk at L2253-L2267 that adds a pass in which the deferred runs stay deferred.

Earlier finding: fixed. The new pass adds the case the earlier advisory said was missing (discussion_r4455949030): a run deferred twice. Checked against reconcile.mjs at head:

  • No room for re-runs. running(203..205) has status: "in_progress" and run_attempt: 2, so each one counts toward inFlight (reconcile.mjs#L1055). That leaves room at 0 (L1185), so 201 and 202 stay deferred and held.reruns is [].
  • The carry-hit branch now runs. lost(201) and lost(202) keep the same head_sha/updated_at at attempt 1, so the pass uses the carried read. The assertion on held.settled.jobs (ids 201 and 202 at attempt === 1) matches the { id, attempt: 1, ... } entry shape (L604). Removing keep.set(key, carried) would now fail that assertion.
  • A third pass reads nothing. The third pass is fed held.settled and asserts no job reads, which checks that the kept entry still works one pass later.
  • The new pass uses the right carry. It is fed first.settled, not second.settled, so it models pass 2 going the other way. The later "listing changed" check still uses first.settled, so it is not affected.

Findings: none.

Not verified: this lane has no shell, so I did not run the tests. The owner's reply says the new assertion fails when that line is commented out and passes with it. That is author-claimed and unverified.

Other checks:

  • Settled threads: the one settled thread is the fix-now reply to the finding above. This push answers it.
  • Plugin manifests: none changed.
  • Trusted paths: the changed file is not on the trusted-path list.
  • Spend: the change is test-only, so it adds no spend.

…carry

The carry test freed every re-run place on its second pass, so it never
reached the path that puts a carried read back for a run deferred again.
Add a pass with the three re-runs still in flight: 201 and 202 stay
deferred, are not read, keep their carried entries, and a third pass reads
nothing. Removing the carry-back line now fails the test.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 3dba34543a1a52a39912c7d88657519e9fa67c9c: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12752 (incremental since 3dcf9a4): no security issues found

Scope. Since 3dcf9a4deaa491e90493079dfe3d8b4abf3a29d4, this push changed one file, .github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs. I read its whole hunk (+15 lines at L2253). None of the trusted-path globs match this file. The only listed path in that directory is infra-failure.mjs.

What changed. The new lines are test code only. They extend the existing test a lane run deferred for want of a place is not read again while it stays at attempt 1 with a case where a run is deferred twice:

  • Three runs (203–205) are re-runs still in progress at attempt 2.
  • The deferred runs 201 and 202 make no attempts/1/jobs reads and get no re-runs.
  • 201 and 202 keep their carried attempt-1 entries in settled.jobs.
  • A third pass, fed that carry, also makes no reads.

Security surface. None. No production code, workflow, token request, kill-switch read, merge hold, trusted-actor list or instruction surface changed. There are no plugin manifest changes. The test only checks the carry and freshness behaviour that the prior round reviewed in reconcile.mjs, and it changes none of that behaviour. Skip gate 2 applies.

The prior-findings file holds only this lane's earlier no-findings summary, so nothing is repeated here. No settled threads apply.

Spend: none. The change is test-only and adds no API calls or lane runs.

critical-findings: 0

@kyle-sexton kyle-sexton added do-not-merge Hard merge gate: do not merge while applied. and removed do-not-merge Hard merge gate: do not merge while applied. labels Nov 3, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit daaefff Nov 3, 2026
88 of 91 checks passed
@kyle-sexton
kyle-sexton deleted the perf/reconcile-lost-runs-carry-lane-jobs branch November 3, 2026 14:50
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 5, survived 2, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 71%; mutation score 71%.

File Killed Survived Timeout No coverage Invalid Not run
.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs 5 2 0 0 0 0

Surviving mutants

Location Operator Disposition Cause
.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs:593 other equivalent none
.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs:603 ROR productive input-gap

The run's summary has the full report.

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