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

- `babysit-prs`: the pr-merge merge job's queued-entry sweep no longer dequeues a lanes App entry just because this run could not read its pr-verify verdict. The merge queue's ci-status reads a lanes App pull request's verdict itself and refuses it fail-closed, so the reader answers `verify:queue-reads` and the entry stays, not counted as kept. Before, when the merge job's whole artifact listing failed, the sweep removed lanes App entries that enable-auto-merge had just armed on a pass, and they cycled in and out of the queue. Every other unreadable entry still leaves, adopted pull requests included, since the queue reads no verdict for them.
Original file line number Diff line number Diff line change
Expand Up @@ -4316,7 +4316,9 @@ def unattended_writes(
else:
queued, _ = postmerge.identity_queue(repo, branch, identity)
# In the pr-merge lane a queued entry also leaves without a
# passing verifier verdict at its head now.
# passing verifier verdict at its head now; a lanes App entry
# whose verdict cannot be read stays for the merge queue's own
# reader (`verify_evidence.QUEUE_READS`).
config = inputs.get("config")
verifier = inputs.get("verify_reader") or (
verify_evidence.Reader(
Expand Down Expand Up @@ -4350,6 +4352,11 @@ def judge(n: int) -> str | None:
if verifier is not None and n != number
else None
)
if hold == verify_evidence.QUEUE_READS:
# A lanes App entry whose verdict this run could not read:
# the merge queue's own reader refuses it fail-closed, so
# it stays, neither dequeued nor counted as kept.
return None
kept_at[n] = min([began, *taken])
return hold

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,9 @@ def _overcount_refusal(outcome: str) -> str:
REPEATED_FAIL = "verify:repeated-fail"
MISSING = "verify:missing"
UNREADABLE = "verify:unreadable"
# `Reader.queued_hold`: a lanes App entry whose verdict this run could not
# read. The merge queue's own reader judges it fail-closed, so it is no hold.
QUEUE_READS = "verify:queue-reads"
TOO_OLD = "verify:too-old"
UNTRUSTED_SPEC = "verify:untrusted-spec"
SPEC_CHANGED = "verify:spec-changed"
Expand Down Expand Up @@ -3953,7 +3956,10 @@ def _pass_refusal(

def queued_hold(self, number: int) -> str | None:
"""Why a queued entry must leave: no allowing verdict at its live
head now. An unreadable pull request is a hold. A dependency
head now. An unreadable pull request is a hold, but an unreadable
verdict on a lanes App entry is `QUEUE_READS`: the merge queue's
ci-status reads that entry's verdict itself and refuses it
fail-closed, while it reads none for any other author. A dependency
manager's entry is the lane's Dependabot class, which pr-verify never
judges, so it is not held here."""
try:
Expand All @@ -3963,7 +3969,21 @@ def queued_hold(self, number: int) -> str | None:
if dependency_bot(pr.get("user")):
return None
head = str(json_object(pr.get("head")).get("sha"))
return self.evidence(number, head, pr).refusal
refusal = self.evidence(number, head, pr).refusal
if refusal == UNREADABLE and self._queue_reads(pr):
return QUEUE_READS
return refusal

def _queue_reads(self, pr: dict[str, Any]) -> bool:
"""Whether the merge queue's ci-status reads this pull request's
verdict: the lanes App's, matched by id and type as ci-status
matches it. A lanes App id that cannot be read is a no."""
user = json_object(pr.get("user"))
try:
app_id = self._lane_app_id()
except Exception: # noqa: BLE001 -- any failure keeps the hold
return False
return app_id is not None and user.get("id") == app_id and user.get("type") == "Bot"


def lane_risk_refusal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5511,6 +5511,62 @@ def tearDownModule() -> None:
_role_label.stop()


class AnUnreadableVerdictKeepsAQueuedEntry(unittest.TestCase):
"""The queued-entry sweep dequeues an entry its verdict holds, but keeps
a lanes App entry whose verdict it could not read (`QUEUE_READS`): the
merge queue's own reader refuses that fail-closed. Any other unreadable
entry still leaves, since the queue reads no verdict for it. Run
41496022715 dequeued #12432 and #12435 on an unreadable listing while
enable-auto-merge re-armed them on a pass."""

REPO = "owner/repo"

def test_only_a_read_hold_dequeues(self) -> None:
holds = {
5: "verify:fail",
6: merge.verify_evidence.QUEUE_READS,
8: None,
9: merge.verify_evidence.UNREADABLE,
}
reader = mock.Mock()
reader.queued_hold.side_effect = holds.get
dequeued: list[set[int] | None] = []
kept: list[str] = []
with (
mock.patch.object(
postmerge, "identity_queue", return_value=([5, 6, 8, 9], 0)
),
mock.patch.object(
postmerge,
"dequeue_identity_entries",
side_effect=lambda *a, numbers, **k: dequeued.append(numbers) or 0,
),
mock.patch.object(policy, "entry_hold", return_value=None),
mock.patch.object(
run_cache, "put", side_effect=lambda ns, key, at: kept.append(key)
),
):
out = merge.unattended_writes(
self.REPO,
7,
policy.UnattendedPolicy(
"ineligible:x",
"agent-lanes[bot]",
{},
inputs={
"identity": "agent-lanes[bot]",
"branch": "main",
"verify_reader": reader,
},
),
)
self.assertEqual(
out["holds"], {5: "verify:fail", 9: merge.verify_evidence.UNREADABLE}
)
self.assertEqual(dequeued, [{5, 9}])
self.assertEqual([k.rsplit("|", 1)[1] for k in kept], ["8"])


class DependabotMajorEscalation(unittest.TestCase):
"""A Dependabot major held under `merge.dependabot.majors: hold` gets the
escalation contract: one marked comment, then the tracker's human-gated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2884,6 +2884,45 @@ def test_a_record_whose_issues_are_not_a_list_refuses(self) -> None:
self.assertEqual(read(fake).refusal, "verify:unreadable")


class AnUnreadableQueuedLanesEntryIsLeftToTheQueue(unittest.TestCase):
"""An unreadable verdict on a queued lanes App entry is `QUEUE_READS`,
since the merge queue's ci-status reads that entry's verdict itself; on
any other author, or when the App's id cannot be read, it stays a hold."""

APP = 4242

def hold(self, user: dict[str, Any], app: Any = APP) -> str | None:
unreadable = ve.Evidence(ve.UNREADABLE, HEAD, (), 0)
lane_id = (
mock.patch.object(ve.Reader, "_lane_app_id", side_effect=app)
if isinstance(app, BaseException)
else mock.patch.object(ve.Reader, "_lane_app_id", return_value=app)
)
with (
mock.patch.object(policy, "pull_request", return_value=pull(user=user)),
mock.patch.object(ve.Reader, "evidence", return_value=unreadable),
lane_id,
):
return ve.Reader(REPO, "main", now=NOW).queued_hold(PR)

def test_only_the_lanes_apps_entry_is_left_to_the_queue(self) -> None:
cases: dict[str, tuple[dict[str, Any], Any, str]] = {
"lanes App": ({"id": self.APP, "type": "Bot"}, self.APP, ve.QUEUE_READS),
"owner": ({"id": 7, "type": "User"}, self.APP, ve.UNREADABLE),
"same id, not a Bot": ({"id": self.APP, "type": "User"}, self.APP, ve.UNREADABLE),
"other App": ({"id": 99, "type": "Bot"}, self.APP, ve.UNREADABLE),
"no App id": ({"id": self.APP, "type": "Bot"}, None, ve.UNREADABLE),
"App id unreadable": (
{"id": self.APP, "type": "Bot"},
RuntimeError("trusted list"),
ve.UNREADABLE,
),
}
for label, (user, app, want) in cases.items():
with self.subTest(label):
self.assertEqual(self.hold(user, app), want)


class AQueuedDependabotEntryNeedsNoVerdict(unittest.TestCase):
"""The pr-merge lane enqueues Dependabot pull requests in their own class
(babysit_dependabot), which pr-verify never judges."""
Expand Down
Loading