Skip to content

fix(pr-merge): settle a wake's fresh report on its contents, not on runs that moved while it ran - #13008

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/wake-fresh-report-runs-test
Nov 4, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/wake-fresh-report-runs-test

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #12445

Summary

pr-merge's PR-scoped wake throws away a post-merge report it computed seconds earlier. When the last sweep's report does not stand, the wake runs the post-merge check fresh. It then runs settle --idle-tick on that fresh report, and idle_runs_refusal refuses it if any verifying run on main completed since computedAt minus 60 s (babysit_postmerge.py:4575-4599, SINCE_CLOCK_MARGIN at babysit_merge_policy.py:6419). On a busy main some run nearly always completes while the check is running. Since #12863, 8 of 23 wakes ended skipped: settle --idle-tick refused the fresh post-merge report (idle tick runs-changed). Each spent 63-71 GITHUB_TOKEN calls and decided nothing (AFTER v3).

The sweep has no such rule. post-merge-dispatch keeps its own fresh report without a settle call (pr-merge.yml:303-314), and the sweep's arm reads the room through lane-room -> settle_shared (babysit_merge_policy.py:7347, 7452-7454), which applies no idle rule. So the wake was stricter than the sweep it stands in for.

Fix

  • babysit_verify_evidence.py settle --idle-tick --fresh skips only idle_runs_refusal. idle_report_refusal (no hold, needsDispatch or reruns: the wake's no-full-run, no-rerun contract), settle_shared (including its unverified-stale keep) and idle_marks_refusal (quarantine issues and pins) all still apply. --fresh without --idle-tick is a usage error.
  • pr-merge.yml keep-report passes --fresh only on the fresh report's settle. The kept-report settle and post-merge-dispatch are unchanged.

Call delta (R34). The fresh check's own calls are unchanged. A fresh report that used to be refused on runs-changed now skips 2 run listings, spends settle_shared's and the marks reads, then arms or refuses on its contents, so the arm step's calls follow. Estimated +23 to +87 GITHUB_TOKEN calls per such wake (between the measured 63-71 refused and 94-150 armed), about +650 to +2,500 a day (UNVERIFIED, linear). This trades wakes that discard what they spent for wakes that decide. The offset is the queued listing-carry fix for the pr-unblock and pr-feedback-dispatch readers (hub estimate ~120-150 calls/h). Both will be re-measured on #12445.

Verification

  • pytest test_babysit_lane_room.py: 2 failed before the fix, 79 passed after. New: test_a_fresh_report_skips_the_runs_its_own_check_just_read (the runs test is not called; needs-dispatch, pins-changed and a lane-room refusal still refuse), test_a_fresh_report_stands_though_a_canary_finished_as_it_ran, and test_fresh_needs_the_idle_tick (guard).
  • node --test workflow.test.mjs: 2 PR-scoped-run tests failed before, 106/106 after. SETTLE_FRESH pins --fresh on the fresh settle; the anchored kept-path SETTLE fails if --fresh ever reaches it. workflow + merge-job together: 199/199.
  • scripts/run-ruff.sh check clean on both Python files.
  • After merge: R21 on the next two wakes that take the fresh path, and an outcome=armed with report=fresh call count posted on pr-pipeline: wake pr-merge per PR on pr-verify completion and make the cron a backstop #12445.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWmASQQUMmnivZUuzgYfbe

…uns that moved while it ran

A PR-scoped wake whose kept report fails runs the post-merge check fresh,
then settled that fresh report with the same `settle --idle-tick` as a
kept one. Its verifying-runs test refuses any run completed since
computedAt less SINCE_CLOCK_MARGIN, so a run that finished while the check
ran discarded a report computed seconds earlier (`idle tick
runs-changed`). A sweep arms on the report post-merge-dispatch just made
with no idle rule at all.

`settle --fresh` (with `--idle-tick`) skips only idle_runs_refusal; the
report checks (no hold, no needsDispatch, no reruns), the lane room's rule
and the quarantine and pin reads still apply. The wake passes it on the
fresh path alone; the kept report's settle is unchanged.

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

chatgpt-codex-connector Bot commented Nov 4, 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-04T00:37:27.358004Z 3643002 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 Nov 4, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 3643002f602f446c6d523c932db2af6a30ee3fbb: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of all 5 changed files. For context I also read the _settle and _settle_reason functions in babysit_verify_evidence.py at head, the whole keep-report step in pr-merge.yml, and the test fixtures.

What changes. settle --idle-tick --fresh drops only idle_runs_refusal, the check of whether a verifying run on main finished after the report was computed. These checks still apply, in this order:

  • idle_report_refusal, before any read
  • the lane room's rule, settle_shared (its pending_runs_unchanged callback is untouched)
  • idle_marks_refusal (quarantine issues and pins)
  • idle_report_refusal a second time, last

Using --fresh without --idle-tick is rejected by parser.error. This matches the PR description.

Workflow. The settle() helper now passes --code-sha "$@":

  • The kept-report call settle "$last_sha" gives the same arguments as before.
  • The fresh call settle "$sha" --fresh gives --code-sha <sha> --fresh.
  • If git rev-parse fails and sha is empty, argparse still parses --code-sha '' --fresh correctly, and check_code_unchanged refuses the empty SHA as it did before.

Tests.

  • In workflow.test.mjs, the SETTLE regex ends in $. So the kept-report pattern SETTLE("c{40}") fails if --fresh ever leaks onto that call, and SETTLE_FRESH pins --fresh onto the fresh call. c{40} matches the fixture's "c".repeat(40).
  • The Python tests check that the runs test is skipped (idle_runs.assert_not_called() plus the call order), and that needs-dispatch, pins-changed and a lane-room refusal still refuse.
  • The refactored canary_settle helper mocks idle_marks_refusal and gives settle_shared a return value, so its fresh case reaches exit 0.

I ran none of the tests; the PR's pytest and node results are the author's claims and unverified.

Design note, not a finding. With --fresh, a run that goes red between the check's run listing and computedAt is no longer caught by the 60 s margin. The PR states this is deliberate: it gives the wake the same exposure as the sweep, which arms on post-merge-dispatch's fresh report without applying any idle rule.

Changelog. The fragment uses bump: patch under ### Fixed, which suits a new flag on an internal script CLI that fixes a refusal.

Closing issue. The PR only says Refs: #12445 and closes no issue, so there were no acceptance criteria to check. The PR context contained no embedded instructions.

Spend (not a hold). Per the PR, each fresh-path wake that used to be refused now makes an estimated +23 to +87 more GITHUB_TOKEN API calls, about +650 to +2,500 a day. That uses API rate-limit budget, not billed resources.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 3643002f602f446c6d523c932db2af6a30ee3fbb: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #13008. No security issues found.

I read every hunk of all 5 changed files. Two of them are on trusted paths, and I read their whole change and what they gate:

  • .github/workflows/pr-merge.yml, the keep-report step
  • plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py, _settle, _settle_reason and main

What changed. settle takes a new --fresh flag, and main rejects it unless --idle-tick is also set. With --fresh, the settle skips only the verifying-runs test (idle_runs_refusal). These checks still run:

  • idle_report_refusal: no hold, needsDispatch or reruns
  • settle_shared
  • the quarantine and pin reads in idle_marks_refusal

The workflow passes --fresh only on the settle of the report this job's own check just wrote, at pr-merge.yml#L2595. The settle of the last sweep's cached report, at L2570, still calls settle "$last_sha" with no flag. So a stale or carried report can never skip the runs test.

Merge-hold check. This narrows a merge hold, so I judged it under the merge-hold bullet:

  • What it holds: the post-merge health check of the default branch (verifying runs and canaries). It is not a hold on untrusted provenance or an untrusted spec, and the gate it feeds only arms auto-merge for the owner's lane pull requests.
  • What now gets through: a verifying run that finished while the fresh check was running no longer refuses the wake's report. Any hold, needsDispatch or rerun in the report still refuses it, and so do changed quarantine markers or pins and the lane room's rule.
  • Security impact: none. No outside attacker input reaches this path, and it causes no leak, compromise or charge. The sweep already arms on the same kind of fresh report with no idle rule.

Shell change. The function changed from --code-sha "$1" to --code-sha "$@". Both values passed in come from git rev-parse or from the job's own temp file, and both are quoted. A --fresh argument only gets in through the hard-coded call at L2595, so no injection path opens.

Spend. The PR estimates 23 to 87 more GITHUB_TOKEN API calls on each wake that used to be refused, about 650 to 2,500 more a day. These calls are covered by the API rate budget and cost no money, so I held nothing for them.

critical-findings: 0

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit f7da85d Nov 4, 2026
64 of 76 checks passed
@kyle-sexton
kyle-sexton deleted the fix/wake-fresh-report-runs-test branch November 4, 2026 01:05
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