[kyle-sexton]

No related issue: follow-up to #10154 (Automation Lanes review I1, S1-S3)

Summary

After #10154, lane_room runs the full post-merge recompute on any shared_refusal. _demotion_changed fires on any issue the lanes App opened that triage then edits, so most scheduled runs paid a recompute of about 5 minutes (run 40255059710's post-merge step took 4m56s) against LANE_ROOM_TIMEOUT_MS (8 minutes). In a slow API window that could time out and disarm every armed lane PR again.

Fix

• I1: when the only refusal is demotion-changed, lane_room calls read_demotion alone (no self logins, as the room binds them). It keeps the shared answer when the fresh demotion equals the shared one, the same way the unverified-stale path keeps an answer. Otherwise it runs the full _recompute_shared, so a demotion that turned active still gives rung-off:demotion. A demotion that is inactive but has other records or another clear time also recomputes, because those feed the floor (merged_since_floor).
• shared_refusal used to return demotion-changed before the head and new-merge:#<n> check ran, so "the only refusal" was not knowable. The demotion check now runs last. demotion-changed means nothing else refused, and an uncounted lane merge reports new-merge:#<n> first. For assess only the postMergeShared reason string changes, since any refusal reads afresh there. The cost is one commits read, plus a merges read when the head moved, on the demotion-changed path.
• S2: the lane_room docstring, the lane-room CLI docstring, the enable-auto-merge.mjs header and docs/conventions/pr-pipeline/README.md now say "any shared_refusal" instead of listing kinds, and they describe the demotion keep path.
• S3: restored assertFalse(self.seen) after the second room() call in the new-merge test.
• Added changelog fragments for source-control and pr-pipeline.

Verification

• Fail-first, against origin/main (a34fbce) before the fix:
  • test_a_no_op_demotion_edit_keeps_the_answer failed: the recheck was asked once, expected never.
  • test_demotion_changed_is_reported_only_when_nothing_else_refuses failed: it got unused:demotion-changed where unused:new-merge:#7 was expected.
  • S1 test_an_answer_bound_to_other_self_logins_is_read_with_none failed with line ~5158 temporarily reverted to shared.self_logins: read_demotion got {'lane-self[bot]'}. The line was then restored.
• Guard tests that pass before and after: test_a_demotion_that_moved_reads_the_answer_again (an active demotion gives rung-off:demotion with one recompute; another record gives one recompute).
• Intentional test changes: the demotion changed case moved out of UNUSABLE into DEMOTION_CHANGED, because it now keeps the answer instead of recomputing. test_an_unusable_answer_with_no_fresh_read_is_unreadable runs its raising and unreadable check cases on stale, since a demotion-changed answer no longer reaches that check. A no check, demotion changed case keeps the old trigger.
• babysit-prs Python suite: 2976 tests OK. node --test .github/scripts/pr-merge/enable-auto-merge.test.mjs: 99 pass, 0 fail. Lane-room tests: 29 OK. Pinned ruff check is clean. check-changelog-fragments.sh --check: all valid. Pre-push check-affected passed.
• Left out: no live pr-merge run; the effect on room time shows on the next scheduled runs.

Related

• fix(pr-merge): read the post-merge answer afresh in lane_room on any reuse refusal #10154, which made any refusal read afresh. This PR answers its Automation Lanes review.

🤖 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-26T03:34:03.934751Z	e5a90a7	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-review reviewed e5a90a77d6c62477d8c33500f590e2f57cbca9cc: 0 finding(s), 0 as inline comments.

No findings. I read the full diff (.claude-lane/pr.diff), the PR context, and the surrounding code in babysit_merge_policy.py. I ran no tests myself. The test results in the PR description are the author's claims and are unverified by me.

What I checked

• Keep path in lane_room (babysit_merge_policy.py:5152-5158). shared.demotion == read_demotion(...) compares the whole frozen Demotion dataclass (active, reason, records, newest_created, cleared_at). A newly filed record, a changed clear time, or a demotion that turned active all make it unequal. The code then falls through to _recompute_shared, so rung-off:demotion still holds the rung off. A failed read is caught by the existing except (RuntimeError, ValueError, OSError) and treated as not current.
• Self logins. shared_refusal binds self_logins and returns self-logins-differ before reaching the demotion check. So a kept answer always has empty self_logins, which matches self_logins=frozenset() in the fresh read_demotion and the shared.self_logins passed to in_flight.
• Reordering shared_refusal (:4542-4566). The head and new-merge checks now run before _demotion_changed. Only the refusal reason changes for assess (:4756-4767). It records the reason in details["postMergeShared"] and reads afresh on any refusal. It only branches on SHARED_BINDING_REFUSALS, whose binding checks still come first. I found no other consumer of the string demotion-changed outside tests and docs. The extra commits and merges reads on that path are acknowledged in the PR description and are small next to the roughly 5-minute recompute they avoid.
• Tests. The new tests cover the keep path with no recompute, an active demotion, a changed record set, the no-post_merge case, and new-merge winning over demotion-changed. I found no coverage gap.
• Docs and changelog. The docstrings, the enable-auto-merge.mjs header, and docs/conventions/pr-pipeline/README.md match the code. Both fragments use bump: patch, which fits a perf-only change.
• Manifests. The diff changes no plugin.json or marketplace.json, so no manifest <field> lines apply.
• Closing issue. The PR states "No related issue", so there are no acceptance criteria to check.
• Trusted paths. None of the changed files match the listed globs.
• Cost. This is not a hold. The change reduces API calls and does not start any billed resource.

[github-actions]

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

No security findings in PR #10162 at e5a90a7.

critical-findings: 0

What I read: the whole diff (8 files) and the PR context. I also read the code the change gates in babysit_merge_policy.py: Demotion, read_demotion, _record_hold, _edits_refusal, _demotion_changed, shared_refusal, _still_current, _recompute_shared, lane_room, and the assess call site.

The change lets lane_room keep a shared post-merge answer that was refused only as demotion-changed. It does this when a lone read_demotion returns a value equal to shared.demotion. This gates whether lane PRs are armed and whether a demotion holds the merge rung off, so I checked whether it weakens that control. It does not:

• Active demotion. A record that is open, reopened, closed by a non-human or by reference, or edited by a non-human makes read_demotion return active=True. That differs from the inactive shared value, so the room recomputes in full and reports rung-off:demotion. The new guard test covers this.
• Inactive demotion. Demotion equality covers records, newest_created and cleared_at. A new, changed or newly human-closed record changes one of them, so the answer is recomputed. Only a true no-op, such as a triage edit to a non-record issue, keeps the answer.
• Self logins. read_demotion in the room uses frozenset(). The binding check in shared_refusal runs before the demotion check and returns self-logins-differ unless shared.self_logins is also empty. So the comparison runs on identically bound reads.
• Reordered shared_refusal. The demotion check now runs last, so no refusal is dropped. Every non-None result still makes assess read afresh, and only the reason string changes. demotion-changed is not in SHARED_BINDING_REFUSALS, so the write-back behaviour is unchanged. An uncounted lane merge is now reported as new-merge:#<n> ahead of demotion-changed, which sends it down the full recompute path instead of the keep path.
• Overlap with unverified-stale. The age refusal is returned before the demotion check, so an answer refused as both goes down the existing _still_current path. That path still calls _demotion_changed, so nothing is relaxed there.
• Read failures. RuntimeError, ValueError and OSError from the demotion read set current = False and fall through to the full recompute. A PolicyReadError is already handled inside read_demotion as Demotion(True, ...), which differs from an inactive shared answer. Both fail closed.

Manifests and trusted paths: no plugin.json or marketplace.json change, and no file matches the trusted-path globs, so there are no manifest <field> lines. The enable-auto-merge.mjs change is a header comment only. The PR description contains no embedded directives. The instruction-exception register lookup did not apply, since no standing instruction was deleted or softened. The doc, docstring and changelog edits only describe the new behaviour.

Cost: the change reduces API time for the lane-room step and starts no billed resource, so there is no hold.