[kyle-sexton]

Closes #9577

Summary

While any lane merge is unverified, lane_room refused the run's post-merge answer once it was older than SHARED_UNVERIFIED_MAX_AGE (two minutes). The post-merge step alone takes about 1m41s, and enable-auto-merge reads the answer at about 2:01. So every pending lane merge made the room unreadable, and enable-auto-merge.mjs disarmed every armed lane pull request, even below merge.max-in-flight. The only record of the hold was a console line.

Fix

• babysit_merge_policy.lane_room takes the gate's post_merge callable and a runs_unchanged callable. When shared_refusal returns unverified-stale, and only then, the room handles the answer in two steps:

  • Cheap freshness check (_still_current). The room keeps the answer and counts from it when all five of these hold:

    • (a) main's head equals its mainSha;
    • (b) the head's config commit equals its configSha, which with an unchanged demotion means the same floor;
    • (c) _demotion_changed finds no demotion record filed or changed since;
    • (d) babysit_postmerge.pending_runs_unchanged passes. It makes one actions-runs listing on the branch, covering runs created since the earliest unverified merge. No pr-require-checks.yml or post-merge-detect-silent-revert.yml run may have completed or been re-run since computedAt (less SINCE_CLOCK_MARGIN, now 60 s and covering clock skew only, since computedAt is stamped before the reads; run 40060427179's step started at 03:58:09.10Z and stamped 03:58:09.27Z), and no unverified merge may be past DISPATCH_DEADLINE, after which a missing dispatch or pin becomes a failure. A partial listing counts as moved.
    • (e) no quarantine that excused a failing suite has expired. The post-merge report now carries excusesExpireAt, the earliest expiry (issue creation plus merge.quarantine-days) of a quarantine that excused a failing suite, whether out of the affected set (demoting) or already failing on the baseline (expired). Once that time passes, or for an answer without the field, the room runs the full check.

    The time-based excuses _judge applies: the missing-pin and missing-dispatch grace (DISPATCH_DEADLINE, covered by (d)), and quarantine expiry (covered by (e)). Two other timers change no verdict: the lane-record expiry only sends a record back to an API read, and lane_rerun_lookback only selects re-runs.

    A run started since that is still running stays pending, so it does not count as moved. The pr-merge run can start a full run after it writes the report, so requiring the same run set would send nearly every read to the full check.

  • Full check. When any of (a)-(d) has moved, the room reads the answer again as assess does for a pr-merge candidate (_recompute_shared): computed_at first, then the head, its config commit, the demotion, the floor (PR_MERGE) and the post-merge check. It uses the fresh answer for the refusal, the rung and the in-flight count. A failure found since computedAt still refuses as post-merge:<reason>, and a demotion record filed since holds the rung off. A check that raises or cannot be read leaves the room unreadable.

  Every other refusal (stale, binding, demotion changed, new merge, floor differs) behaves as before. The room writes nothing back: nothing in the job reads the file again. The gate keeps its own write-back.

• pr-merge.yml enable-auto-merge job: it now reads the lane records the last run kept, using the same lane-records.mjs read step post-merge-dispatch uses. That step uses ACTIONS_TOKEN: github.token, within the job's existing actions: read, and is limited to reader runs. The job sets BABYSIT_POSTMERGE_LANE_RECORDS on the arm step, so the room's full check answers known records without an artifact read and a run read each. VERIFY_TIMEOUT_MS is unchanged.

• babysit_postmerge.gate_reader is the gate's post_merge(floor, identity). babysit_merge.assess_unattended now uses it in place of its inline copy. babysit_verify_evidence.py lane-room wires it and pending_runs_unchanged in (no self logins, lane merges only, as the room binds).

• enable-auto-merge.mjs: when the room is refused or unreadable, the run records what became of each held lanes App pull request. One ::warning:: annotation and $GITHUB_STEP_SUMMARY line name the reason and the pull requests left unarmed (never armed, or disarmed), and in report-only mode the ones a disarm would have turned off. One ::error:: annotation and summary line name any pull request still armed because its disarm failed. It posts no comment on the pull requests.

• The lane_room and lane-room docstrings, the enable-auto-merge.mjs header and docs/conventions/pr-pipeline/README.md describe this. There is a changelog fragment for source-control and one for pr-pipeline.

Verification

• New tests in test_babysit_lane_room.py, all failing on origin/main and on this PR's first head (6fe7ac3), checked by running the new test file against archives of each:
  • With (a)-(e) unchanged, the answer is kept and counted from its own floor and verified, and the fake full check is never called.
  • One subtest for each of (a)-(d) moving: each runs the full check exactly once.
  • A pending dispatch run that completed red, read through the real pending_runs_unchanged with a faked listing, gives post-merge:failure:dispatch#5.
  • A quarantine excuse that expired after computedAt (excusesExpireAt one minute after it, read three minutes on) runs the full check once.
  • pending_runs_unchanged: runs still running, or finished before computedAt, hold, read from one listing call. A full or canary run that completed since, a partial listing, a merge past DISPATCH_DEADLINE, an expired or missing excusesExpireAt count as moved.
  • test_babysit_postmerge_quarantine.py: the report's excusesExpireAt is the issue's creation plus 7 days, both for an out-of-set excuse and for a baseline failure under a live quarantine, and it is null when no quarantine issue excuses anything.
  • workflow.test.mjs: the enable-auto-merge job reads the lane records before the arm step and sets BABYSIT_POSTMERGE_LANE_RECORDS on it.
  • From the first head: a full check that finds a failure refuses; one that raises, is not readable, or cannot read the head leaves the room unreadable; a demotion read since holds the rung off; and the stale, future, demotion-changed and new-merge cases run no check.
• enable-auto-merge emits exactly one warning and one summary line, naming chore: initialize marketplace scaffold #1 and fix: correct plugin-system inaccuracies and harden references #2, for an unreadable room and for a refused one, and neither for a room with space. A failed disarm names its pull request on an error line as still armed, and report-only names it as one a disarm would have turned off. Both tests fail on 80f859f.
• Clock-margin edge: a run that completed 59 s before computedAt forces the full check, and one that completed 61 s before is reused. This fails on 80f859f.
• babysit-prs Python suite: 2794 tests OK. pr-merge node tests: 308 pass, 0 fail. actionlint .github/workflows/pr-merge.yml is clean. scripts/run-ruff.sh check is clean. scripts/check-affected.sh passed in the pre-push hook.
• Reads on the cheap path are counted from the code, not measured: (a) 1 commits/<branch> read, (b) 1 commits-by-path read (0 when a local clone holds the head), (c) 1 issues read, (d) 1 runs listing. That is at most 4 GitHub API calls; (e) reads nothing.
• Measured wall time of one full post-merge check: 4m20s, against VERIFY_TIMEOUT_MS of 4m00s.
  • How: a local run of babysit_postmerge.py --repo melodic-software/claude-code-plugins --agent-identity "melodic-automation-lanes[bot]" --lane-merges-only, read-only (no --remediate), on 2026-10-24 from 05:39:18Z to 05:43:39Z.
  • Result: 465 gh calls, including 180 artifact listings and 180 run reads. That is the cost of having no BABYSIT_POSTMERGE_LANE_RECORDS file, which the enable-auto-merge job does not carry either.
  • The CI figure from run 40060427179 is 1m41s, with the lane-records file.
  • Consequence: the enable-auto-merge job now carries the lane-records file, so its full check should run close to the 1m41s CI figure. That is UNVERIFIED until a scheduled run that needs the full check shows it. Without the file, the check could exceed pythonRoom's timeout and leave the room unreadable, which fails closed.

Related

• feat(pr-pipeline): merge lanes App pull requests through native auto-merge #9508

🤖 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	✅ Completed 2026-10-24T05:38:01.539584Z	6fe7ac3	PR opened

ℹ️ 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 d6fbf9bcb72b2c760c5d469b7e26dbe535f57535: 0 finding(s), 0 as inline comments.

Security review: no findings

I read the full 13-file diff and the PR context. I also read .github/scripts/pr-merge/lane-records.mjs (unchanged) to check the trust of the new workflow step's input.

critical-findings: 0

What I checked

• Fail-open risk in the "keep the stale answer" path (lane_room, _still_current, pending_runs_unchanged). The answer is kept only when all of these hold:
  • main's head and its config commit are unchanged;
  • no demotion record changed;
  • no full or canary run completed since computedAt minus the margin;
  • no unverified merge is past DISPATCH_DEADLINE;
  • no quarantine excuse has expired.
• Errors fail closed. Any RuntimeError, ValueError or OSError falls to the recompute. A failed or unreadable recompute returns unreadable, and enable-auto-merge.mjs disarms on that.
• A new lane merge changes main's head. That sends the room to the recompute, so it cannot be missed.
• An answer with no excusesExpireAt, a partial run listing (total_count != len), or an entry with no merge time counts as moved. Each of these forces the full check.
• Recompute binding. It uses the same identity and self_logins as the room's existing binding, with lane_merges_only=True. A fresh failure still refuses as post-merge:<reason>, and a fresh demotion holds the rung off.
• New workflow step ("Read the lane records the last run kept").
  • It runs node .base/..., so the code comes from the base checkout, not the PR head.
  • It is limited to schedule and workflow_dispatch, and it uses the job's own github.token within the existing actions: read.
  • lane-records.mjs only keeps a record whose artifact was uploaded by a trusted run (artifact.workflow_run.id === entry.sender.id, trustedRun), so a pull-request-triggered run can't plant a "verified" record.
  • The job gains no new permissions or secrets.
• Annotation and summary output (enable-auto-merge.mjs).
  • The reason text goes through escapeCommand, which encodes newlines, so a gh error message can't start a second workflow command. I did not re-read escapeCommand itself.
  • The PR numbers are integers.
  • The summary write is wrapped in try/catch and is non-fatal.
  • No comment is posted on pull requests.
• gate_reader refactor in babysit_merge.assess_unattended. The inline closure moves into babysit_postmerge.gate_reader with the same arguments, so behavior is unchanged.

Manifests: the diff touches no plugin.json or marketplace.json, so there are no manifest <field> lines.

Notes (not findings)

• SINCE_CLOCK_MARGIN drops from 5 minutes to 60 seconds. This narrows the demotion re-check window to cover clock skew only, and the gate's reuse check uses it too. The PR's reasoning is sound: computedAt is stamped before the reads. It would only miss a demotion record if the runner clock were more than 60 seconds ahead of GitHub's, which is unusual on hosted runners.
• I could not read docs/conventions/instruction-exception-register/README.md, so the protected-class list was not consulted. The diff deletes no standing instruction anyway. The docs edits only describe new behavior.
• GitHub Actions hardening is left to zizmor.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fe7ac3cf3

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

[github-actions]

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

No blocking or advisory findings on this head (d6fbf9b). I only read the code; I ran no tests.

What I checked

• Closing issue fix(pr-merge): lane_room reads its post-merge answer past the 2-minute unverified age and disarms the lane #9577
  • Recompute on unverified-stale. lane_room keeps or recomputes the answer only for unverified-stale. Other refusals are handled as before (shared_refusal / _shared_age_refusal). The issue's "a verification failure after computedAt must still refuse" is met, because the full check re-reads and refuses as post-merge:<reason>.
  • Test for the counted room. A test for "pending merge older than 2 minutes yields a counted room" exists: test_a_pending_merge_past_two_minutes_is_read_again_and_counted.
  • Scope-addition comment. One ::warning:: and one summary line naming the reason and each pull request left unarmed. They are implemented in enable-auto-merge.mjs and tested, with no per-pull-request comment.
• Skipped checks on the kept path. The kept path skips shared_refusal's demotion-changed and new-merge checks, since _shared_age_refusal returns first. They are covered by _still_current: head equals main_sha, config commit equals config_sha, and _demotion_changed. I found no gap.
• Clock margin. SINCE_CLOCK_MARGIN drops from 5 minutes to 60 seconds. The computedAt stamp does come before the reads (babysit_postmerge.py main, computed_at = datetime.now(UTC) before default_floor), so the narrower margin is sound. _recompute_shared stamps it first too.
• Error handling. pending_runs_unchanged turns every _time and _read failure into PostMergeReadError, which it catches, so a bad or partial listing fails to "moved" and runs the full check. lane_room catches RuntimeError/ValueError/OSError, which covers PolicyReadError, so a failed recompute leaves the room unreadable and fails closed.
• enable-auto-merge.mjs.
  • Outcome tracking. roomHeld outcomes (unarmed, armed after a failed disarm, report-only) follow every path that reaches the disarm or report-only branches.
  • Variable scoping. The loop-local held and the later held(state) helper live in separate scopes, so there is no TDZ problem.
• Manifests. The diff changes no plugin.json or marketplace.json, so no manifest <field> lines apply. The changelog fragments use bump: patch, which fits a fix with an additive report field and optional parameters.
• Hold classes. None apply. The diff adds no model spend, touches no trusted-actors, CODEOWNERS or .gitmodules files, and raises no critical-security finding. No hold is set.

Author-claimed and unverified by me: the PR's test counts (2794 Python, 308 node), the actionlint and ruff results, and the 4m20s versus 1m41s timing. The PR itself marks the "enable-auto-merge's full check runs close to 1m41s with the lane-records file" consequence as unverified until a scheduled run needs it. The path fails closed if it is slow, because the room stays unreadable.

Nits, not counted as findings:

• In enable-auto-merge.mjs, the new laneHeld constant sits between the roomHold doc comment and roomHold, so that comment now describes the wrong declaration.
• The two step-summary lines are written without a blank line between them, so Markdown renders them as one paragraph when both a warning and an error are raised.

[github-actions]

The pr-verify verifier failed head d6fbf9bcb72b2c760c5d469b7e26dbe535f57535. What it wrote, as data:

Summary: Issue #9577: the PR delivers what the issue asks for. On `unverified-stale`, `lane_room` now recomputes the post-merge answer as the gate does, and a failure found since `computedAt` still refuses. A new test shows a pending merge more than 2 minutes old giving a counted room. The scope-addition comment is also met: one warning annotation plus a step-summary line naming the reason and each pull request left unarmed. It fails for one reason. To make its cheap reuse path work, the PR cuts the shared `SINCE_CLOCK_MARGIN` from 5 minutes to 60 seconds. That constant also drives the gate's demotion re-check, so a merge-safety guard is weakened outside the issue's scope.
- PR body: its claims (tests failing on earlier heads, measured timings, a full suite run) cannot be checked from the diff, and the body itself labels the post-change wall time of the enable-auto-merge full check UNVERIFIED. I found no text trying to steer a reviewer.
- .github/scripts/pr-merge/enable-auto-merge.mjs:794: the per-pull-request `const held` inside the loop shadows the later `const held = (state) => ...` helper in the enclosing block. It works, but renaming one of them (for example `heldList`) would make it clearer.

[github-actions]

The pr-verify verifier failed head d6fbf9bcb72b2c760c5d469b7e26dbe535f57535. What it wrote about this file, as data:

- plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py:3884: `SINCE_CLOCK_MARGIN` drops from 5 minutes to 60 seconds. The constant is shared with the gate's demotion re-check (`_demotion_changed` / `shared_refusal`), so this narrows a guard that pr-merge's gate applies to every candidate. Issue #9577 does not ask for that. The claim that `computedAt` is always stamped before the reads it covers is asserted but not shown for the gate's own answers. Fix: leave `SINCE_CLOCK_MARGIN` at 5 minutes and add a separate constant (for example `RUNS_SINCE_CLOCK_MARGIN = timedelta(seconds=60)`) used only in `babysit_postmerge.pending_runs_unchanged`. Then update the changelog fragment in `.changes/source-control/` that says the 60-second margin applies to the demotion re-check.
- plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py:4635: the issue says to recompute on `unverified-stale` and not to ignore the age. The new `_still_current` path keeps an answer past its 2-minute reuse age without recomputing, as long as main's head, the config commit, the demotion records, the full/canary run listing and the quarantine expiry are unchanged. That goes beyond the spec and extends the gate's reuse rule for the room. It looks reasoned (the full check measured 4m20s against a 4m00s `VERIFY_TIMEOUT_MS`), but a human should confirm that the check's verdict has no other inputs (pins, lane records, timeline events) that can change without a full or canary run completing.

[github-actions]

The pr-verify verifier failed head d6fbf9bcb72b2c760c5d469b7e26dbe535f57535. What it wrote about this file, as data:

- plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py:3150: `pending_runs_unchanged` lists runs only with `created>=` the earliest unverified merge's `mergedAt`. A re-run of a run created earlier is missed. That seems harmless for runs that verify the merge, but it rests on verifying runs never being created before `mergedAt`.