[kyle-sexton]

No related issue: pr-merge gate slowdown in scheduled run 40072153191 (per-candidate deadline hit, 4 of 6 candidates judged)

Summary

In scheduled pr-merge run 40072153191, main moved at 07:06:05Z while the gate was running. From then on, every _lane_sender check compared a lane record's run head against a live tip the .base clone did not have. The babysit-gh-calls lines in that run's log show two candidates at 190 and 211 gh calls in total, 146 and 162 of them compares. Each call took about 1.25 s, so two candidates hit the 240 s per-candidate deadline and only 4 of 6 were judged inside the 10-minute budget. One of the two, #9618, had already merged, yet it still ran the full post-merge check before the gate's not-open blocker.

Compare calls per candidate, measured in the new tests (20 records on the old tip, one between the old and the new tip, one off the branch):

Case	Before	After
Tip moved after the clone was made (22 records)	22	3 (one bridge compare, plus the two records not on the clone's copy)
Merged pull request in the lane	full post-merge check	no demotion, floor or post-merge read (the identity and queue readers still run)

On real runs, "after" is one bridge compare per candidate process, because each candidate reads the tip once. It is not one per run.

Fix

• Compare storm (babysit_postmerge.py). When contains(ancestor, tip) finds that the clone lacks the live branch tip, _tip_bridge reads the clone's copy of the branch (refs/remotes/origin/<branch>, else refs/heads/<branch>) and compares that copy with the tip once per tip.
  • When the status is ahead or identical, any record whose head is a local ancestor of that copy is an ancestor of the tip, with no further call.
  • A record off the copy, a diverged or force-moved tip, or a bridge compare that fails each fall back to the existing per-record compare. The answer is therefore identical to a per-record compare.
  • A failed per-record compare still raises PostMergeReadError. A failed bridge compare does not raise; it only turns the bridge off, because the per-record compare it falls back to fails closed on its own.
  • I chose this approach over fetching the tip into the clone. Both .base checkouts set persist-credentials: false (.github/workflows/pr-merge.yml), _history_checkouts also returns the user's working clone (a fetch would modify it), and HISTORY_GIT_ENV sets GIT_NO_LAZY_FETCH, so these history reads never touch the network.
• Early stop (babysit_merge_policy.assess). With the new stop_when_closed flag, a pull request whose REST record reads state: closed, merged: true refuses once the identity, credential, kill-switch, default-branch and permission reads are done, and before the demotion, floor and post-merge reads. The refusal reuses the gate's own blocker text, now in not_open_refusal: state=MERGED (not OPEN). A closed pull request that was not merged is read in full, because it can reopen before the gate's own GraphQL state read, and stopping it then would refuse a pull request the full read could pass.
  • The gate passes the flag only when there is no --state-dir, which is how the pr-merge lane runs. A state directory can hold a recorded merge request whose read-back for a merged pull request dispatches the full run and needs the whole policy, so with one the old path runs.
  • The verdict is unchanged: the gate's GraphQL state check already held that pull request. The new reason does not start with any STOP_REFUSALS prefix, so it dequeues nothing.
  • The unattended_writes queue sweep still runs for that candidate, because the stop comes after the readers it needs. The prefilter keeps one candidate alive so that some gate sweeps the queue (babysit_merge_prefilter.py), and that candidate may be the merged one.

Verification

• New tests, which failed on the old code:
  • TipMoved in test_babysit_postmerge_contains.py counted 22 compares where 3 were expected.
  • test_a_merged_pr_with_no_state_dir_skips_the_post_merge_check in test_babysit_merge_unattended.py found the post-merge check called once.
  • The Assess tests in test_babysit_merge_policy.py hit a TypeError on the new flag.
• What those tests check:
  • The tip-moved verdicts are identical to a compare-only context.
  • A force-moved tip and an unreadable bridge each cost one compare per record.
  • A failed record compare raises.
  • A merged pull request makes no demotion, in-flight, eligibility or post-merge read, still dequeues a held queued entry under --merge, and a closed unmerged one is read in full.
  • With a state directory, a recorded merged request still dispatches the full run.
• Full babysit-prs suite after merging main: uv run --no-project --with pytest python -I -m pytest -q plugins/source-control/skills/babysit-prs/scripts/tests gave 2834 passed.
• bash scripts/run-ruff.sh format and check pass on the changed files.
• Left out: a live pr-merge run, since only a scheduled run on main exercises the moved-tip case end to end.

Related

• Scheduled run 40072153191
• fix(pr-merge): recompute an unverified-stale post-merge answer in lane_room and annotate room holds #9602 (the lane_room recompute, merged into this branch from main)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-24T08:04:27.188781Z	14da5c5	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]

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

No security issues found in this pull request (head 14da5c5). critical-findings: 0

The change touches the babysit-prs merge gate, so I checked both changes for ways they could loosen a merge decision.

• Tip-bridge shortcut in babysit_postmerge.py (_tip_bridge and contains): this is sound.
  • A record is accepted as an ancestor of the live tip only when two things hold. The clone's copy L of the branch must be read by the GitHub compare API as ahead or identical to the live tip T. The record's commit must also be a local ancestor of L. Together these mean the record is an ancestor of T, so the verdict matches a per-record compare.
  • A local branch ref with unpushed or planted commits cannot widen trust. GitHub must confirm L is an ancestor of T, and a diverged, force-moved or unknown L returns diverged, behind or a 404 and turns the bridge off.
  • A failed bridge compare falls back to the per-record compare. That compare still raises PostMergeReadError, so reads still fail closed.
  • The branch name goes into git rev-parse only as refs/remotes/origin/<branch> or refs/heads/<branch>^{commit}, with --verify. The output must also match a full SHA, so there is no argument or option injection.
  • The bridge applies only when sha == self.branch_tip, so it cannot be used for arbitrary shas.
• stop_when_closed early refusal in babysit_merge_policy.assess: this only moves a refusal earlier.
  • It fires only when the REST record reads state == "closed" and merged is True. It returns a stopped policy (enabled false) with the gate's existing blocker text, so it can only refuse. It cannot enable a merge.
  • It is passed only when pending_path is None (no --state-dir). The recorded-merge read-back path that needs the full policy keeps the old behaviour.
  • A closed PR that was not merged is still read in full, and the identity, credential, kill-switch and permission reads still run before the stop.
  • The new reason does not match a STOP_REFUSALS prefix, so it dequeues nothing. The queue sweep still runs, as the PR describes.
• evaluate in babysit_merge.py: it now calls merge_policy.not_open_refusal, which produces the same string as before, so the blocker is unchanged.
• Tests and the .changes entry: the remaining changes are tests, formatting-only edits and a changelog entry.
• Manifests and instruction surfaces: no plugin.json or marketplace.json is touched, so there are no manifest <field> lines. No standing instruction (CLAUDE.md, rules file, skill or agent body, or hook allowlist) is deleted or softened, and no trust-gate path is changed.
• PR context: the PR description contains no embedded instructions directed at the reviewer.

[github-actions]

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

Code review: no findings

I read the full diff (.claude-lane/pr.diff, 7 files) and the PR context. I also read the changed code in babysit_postmerge.py, babysit_merge_policy.py and babysit_merge.py around each edit. I ran no tests, so everything the PR says about test results (the 2834 passes, ruff) is author-claimed and unverified by me.

Compare-storm fix (babysit_postmerge.py, contains and _tip_bridge)

• The bridge shortcut is sound. It answers True only when the record's commit is a local ancestor of the clone's copy of the branch, and the compare API has read that copy as an ancestor of the live tip. Ancestry is transitive, so True is always correct.
• Every other case still goes to the per-record _compare: a record off the copy, a diverged or force-moved tip, a missing clone or ref, or an unreadable bridge compare.
• A failed record compare still raises. PostMergeReadError is swallowed only in the bridge compare, and the per-record compare it falls back to still fails closed.
• bridge_cache is keyed by tip, and contains only uses the bridge when sha == self.branch_tip, so no other pair can be answered from it.
• The TipMoved tests compare verdicts against a compare-only context on a real git repo. They cover the force-moved, unreadable-bridge, failed-compare, record-on-live-tip, local-tip and no-clone cases, so a regression in the verdict would fail them.

Early stop (babysit_merge_policy.assess, stop_when_closed)

• The stop is narrow. It fires only when the REST record reads state == "closed" and merged is True.
• It fires after the identity, kill-switch, default-branch, config and permission reads. Those are the inputs unattended_writes needs, so the queue sweep still runs.
• A closed-but-unmerged pull request, which can reopen, is still read in full.
• _main passes the flag only when pending_path is None, which keeps the recorded-merge read-back path on the full policy. A test covers that case.
• not_open_refusal is now the single source of the state=… (not OPEN) text, used by both evaluate and the stop, so the blocker string cannot drift.
• The one side effect I noticed is that a merged candidate no longer computes or shares the run's post-merge answer, so a later candidate computes it itself. That costs a little speed and does not change any verdict.

Closing issue: none. The PR context says "No related issue", so there are no acceptance criteria to check.

Manifests: no plugin.json or marketplace.json change, so there are no manifest <field> lines. The changelog fragment says bump: patch, which fits an internal performance change with unchanged verdicts.

Hold: none. The PR adds no model or API spend, trigger or schedule, and touches no trust-gate path. It touches no CODEOWNERS, .gitmodules or trusted-actors file. The security lane covers security.

Not flagged:

• The reformatting-only hunks in test_babysit_merge_unattended.py are unrelated to the change.
• The docstring paragraph in assess has one over-long line after the edit.