Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- `enable-auto-merge` no longer leaves every lanes App pull request unarmed, and disarms the armed ones, when the run's post-merge report is one the gate would not reuse (for example `demotion-changed`, which any issue the lanes App files and triage then labels sets). The lane's room now checks the report again as the gate does and arms from that answer.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- `babysit_verify_evidence.py lane-room` no longer reports the lane's room unreadable when the gate would not reuse the run's post-merge answer for a reason other than `unverified-stale`: too old, from the future, a demotion record filed or changed since (`demotion-changed`), or a lane merge it does not count (`new-merge:#<n>`). As the gate does, it reads the answer again (`_recompute_shared`, demotion included), so a demotion filed since still holds the rung off and a failure found since still refuses. Only an `unverified-stale` answer may still be kept unchanged when nothing it was read from has moved. With no check to run, or a check that fails or cannot be read, the room stays unreadable.
8 changes: 4 additions & 4 deletions .github/scripts/pr-merge/enable-auto-merge.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
// lane's unverified merges and the armed pull requests not yet queued, it
// arms; at the cap it arms nothing more. A room that cannot be read (no
// post-merge report, a failed read) holds as a refusal does: nothing is
// armed and every armed lanes App pull request is disarmed. A report past
// its reuse age only for a lane merge still unverified is kept while
// nothing it was read from has moved, else checked again as the gate
// does. A held room raises one warning annotation and step-summary line
// armed and every armed lanes App pull request is disarmed. A report the
// gate would not reuse is checked again as the gate does; one past its
// reuse age only for a lane merge still unverified is first kept while
// nothing it was read from has moved. A held room raises one warning annotation and step-summary line
// naming why and each lane pull request left unarmed or, report-only,
// that a disarm would have turned off, and one error and summary line
// naming each one still armed because its disarm failed.
Expand Down
7 changes: 4 additions & 3 deletions docs/conventions/pr-pipeline/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1134,9 +1134,10 @@ gate's `<!-- babysit-merge-queue-ejections -->` comment and no label, since a pu
count; an unread count leaves it unarmed with no comment, and the next run reads it again. An
armed one is not disarmed on the count. Each such run also
waits for the same run's `post-merge-dispatch` job and reads the lane's room from the post-merge
report that job keeps as an artifact (`babysit_verify_evidence.py lane-room`; with no report, or
one the gate would not reuse (`shared_refusal`: too old, a demotion record changed since, or a lane
merge landed since that it does not count), it holds as a refusal does; one past the gate's
report that job keeps as an artifact (`babysit_verify_evidence.py lane-room`; with no report it
holds as a refusal does; one the gate would not reuse (`shared_refusal`: too old, a demotion record
changed since, or a lane merge landed since that it does not count) is checked again as the gate
does; one past the gate's
two-minute reuse age only because a lane merge is still unverified is kept while main's head, its
config commit, the demotion records and the unverified merges' full and canary runs are as they
were (no run reached a conclusion since, no merge is past the six-hour dispatch deadline, and no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5094,11 +5094,12 @@ def lane_room(
`assess` applies to every lane candidate, read once at the default branch
head, in its order. `shared` is this run's `babysit_postmerge.py` answer
(`babysit_postmerge.read_shared`), as the gate's `--postmerge-result`.
`post_merge` is `assess`'s: an answer refused only as `unverified-stale`
is kept when nothing it was read from has moved (`_still_current`, with
`runs_unchanged(shared, branch)` for its unverified merges' runs), else
read again as the gate reads it (`_recompute_shared`), and a readable
one stands in for `shared`.
`post_merge` is `assess`'s: an answer the gate would not reuse
(`shared_refusal`) is read again as the gate reads it
(`_recompute_shared`, with the room's self logins, none), and a readable
one stands in for `shared`; one refused only as `unverified-stale` is
first kept when nothing it was read from has moved (`_still_current`,
with `runs_unchanged(shared, branch)` for its unverified merges' runs).

- A config naming another authority stops (`enabled_authority`).
- A post-merge refusal stops (`post-merge:<refusal>`).
Expand All @@ -5110,12 +5111,9 @@ def lane_room(

Any read failure, a missing `shared` and an in-flight count that could
not be read included, is `unreadable`. So is a `shared` the gate would
not reuse (`shared_refusal`): too old (a re-run job downloads the run's
first report, up to a day old), a demotion record filed or changed
since, or a lane merge landed since that it does not count. An
`unverified-stale` one is kept or read again instead, since a lane
merge's verifying run may have passed or failed since; a read that fails
or is not readable leaves it `unreadable`."""
not reuse (too old, a demotion record filed or changed since, a lane
merge landed since that it does not count) when there is no `post_merge`
to read it again, or that read fails or is not readable."""
if shared is None:
return LaneRoom(unreadable="post-merge answer missing or unreadable")
try:
Expand Down Expand Up @@ -5143,18 +5141,21 @@ def lane_room(
authority=PR_MERGE,
now=now,
)
if unusable == "unverified-stale" and post_merge is not None:
# Kept when nothing it was read from moved, else read again.
if unusable and post_merge is not None:
# As the gate, any refusal reads afresh; an unverified-stale answer
# is first kept when nothing it was read from moved.
try:
current = runs_unchanged is not None and _still_current(
repo, default, identity, shared, runs_unchanged
current = (
unusable == "unverified-stale"
and runs_unchanged is not None
and _still_current(repo, default, identity, shared, runs_unchanged)
)
except (RuntimeError, ValueError, OSError):
current = False
if not current:
try:
fresh = _recompute_shared(
repo, default, identity, shared.self_logins, post_merge, now
repo, default, identity, frozenset(), post_merge, now
)
except (RuntimeError, ValueError, OSError) as exc:
return LaneRoom(unreadable=f"post-merge answer {unusable}: {exc}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1992,8 +1992,9 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int:
(`merge_policy.lane_room`, `LaneRoom.report`), from this run's
`babysit_postmerge.py` report and the `CLAUDE_LANES_MERGE_RUNG_CAP` value
the workflow read (empty is no cap; missing holds the rung off). A report
refused only as `unverified-stale` is kept while nothing it was read from
has moved, else checked again."""
the gate would not reuse is checked again; one refused only as
`unverified-stale` is first kept while nothing it was read from has
moved."""
parser = argparse.ArgumentParser(
prog="babysit_verify_evidence.py", allow_abbrev=False
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,36 +239,90 @@ def test_at_the_cap_it_reports_the_cap_and_refuses_nothing(self) -> None:
(room.refusal, room.unreadable, room.count, room.cap), (None, None, 5, 5)
)

def test_an_answer_the_gate_would_not_reuse_is_unreadable(self) -> None:
# The gate reads afresh past SHARED_MAX_AGE or after a demotion record
# changed (shared_refusal); a re-run job reads the run's first report.
cases = {
"stale": dict(now=FLOOR + policy.SHARED_MAX_AGE + timedelta(seconds=1)),
"future": dict(now=FLOOR - timedelta(seconds=1)),
# Inside the two minutes: past them the answer is read again,
# demotion included.
"demotion changed": dict(
demotion_changed=True, now=FLOOR + timedelta(minutes=1)
),
}
recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None))
for name, kwargs in cases.items():
# The gate's reuse refusals other than unverified-stale (shared_refusal):
# past SHARED_MAX_AGE (a re-run job reads the run's first report), from
# the future, and a demotion record filed or changed since, inside the
# two minutes the gate reuses a pending answer for.
UNUSABLE = {
"stale": dict(now=FLOOR + policy.SHARED_MAX_AGE + timedelta(seconds=1)),
"future": dict(now=FLOOR - timedelta(seconds=1)),
"demotion changed": dict(
demotion_changed=True, now=FLOOR + timedelta(minutes=1)
),
}

def test_an_answer_the_gate_would_not_reuse_is_read_again(self) -> None:
# The gate reads afresh on any refusal (assess); so does the room,
# and a readable answer counts from its own floor. Run 40255059710
# left 12 lane pull requests unarmed on `demotion-changed`.
for name, kwargs in self.UNUSABLE.items():
with self.subTest(name):
fresh = policy.PostMergeGate(None, {"readable": True}, lambda m: True)
recheck = Recheck(fresh)
room = self.room(
shared(report=PENDING),
post_merge=recheck.post_merge,
**kwargs, # type: ignore[arg-type]
)
self.assertEqual(
(room.refusal, room.unreadable, room.count, room.cap),
(None, None, 2, 5),
)
self.assertEqual(recheck.asked, [(NEW_FLOOR, IDENTITY)])
self.assertIs(self.seen["verified"], fresh.verified)
self.assertEqual(self.seen["merged_since"], NEW_FLOOR)

def test_an_unusable_answer_is_never_kept_on_the_cheap_path(self) -> None:
# Only an unverified-stale answer may be kept unchanged; a stale one
# is read again even when nothing it was read from moved.
recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None))
runs = mock.Mock(return_value=True)
self.room(
shared(report=PENDING),
post_merge=recheck.post_merge,
runs_unchanged=runs,
head=MAIN,
**self.UNUSABLE["stale"], # type: ignore[arg-type]
)
self.assertEqual(len(recheck.asked), 1)
runs.assert_not_called()

def test_a_demotion_read_again_holds_the_room_off(self) -> None:
recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None))
room = self.room(
post_merge=recheck.post_merge,
fresh_demotion=policy.Demotion(True, "record-open", (8,)),
**self.UNUSABLE["demotion changed"], # type: ignore[arg-type]
)
self.assertEqual(room.refusal, "rung-off:demotion")

def test_an_unusable_answer_with_no_fresh_read_is_unreadable(self) -> None:
cases = {
"no check": None,
"check raises": Recheck(error=RuntimeError("runs 502")).post_merge,
"check unreadable": Recheck(
policy.PostMergeGate("runs-unreadable", {"readable": False}, None)
).post_merge,
}
for name, post_merge in cases.items():
with self.subTest(name):
room = self.room(
post_merge=post_merge,
**self.UNUSABLE["demotion changed"], # type: ignore[arg-type]
)
self.assertIsNone(room.refusal)
self.assertTrue(room.unreadable)
self.assertFalse(self.seen, "no in-flight read after an unusable answer")
# Only an unverified-stale answer is read again.
self.assertEqual(recheck.asked, [])
self.assertTrue(
str(room.unreadable).startswith(
"post-merge answer demotion-changed"
),
room.unreadable,
)
self.assertFalse(self.seen, "no in-flight read")

def test_a_lane_merge_the_answer_does_not_count_is_unreadable(self) -> None:
def test_a_lane_merge_the_answer_does_not_count_is_read_again(self) -> None:
# The gate's reuse rule (shared_refusal) reads afresh when a lane
# merge landed after the report was computed (`new-merge:#<n>`), so
# its failed full run is not missed; the room holds rather than arm.
# its failed full run is not missed; the room reads it again too.
seen: list[dict[str, object]] = []

def refusal(repo: str, answer: object, **kwargs: object) -> str:
Expand All @@ -278,12 +332,14 @@ def refusal(repo: str, answer: object, **kwargs: object) -> str:
recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None))
with mock.patch.object(policy, "shared_refusal", side_effect=refusal):
room = self.room(post_merge=recheck.post_merge)
self.assertEqual((room.refusal, room.unreadable), (None, None))
self.assertEqual(recheck.asked, [(NEW_FLOOR, IDENTITY)])
with mock.patch.object(policy, "shared_refusal", side_effect=refusal):
room = self.room()
self.assertEqual(
(room.refusal, room.unreadable),
(None, "post-merge answer new-merge:#7"),
)
self.assertFalse(self.seen, "no in-flight read after an unusable answer")
self.assertEqual(recheck.asked, [])
# Bound as the gate binds a candidate at the default branch head.
self.assertEqual(
{
Expand Down
Loading