From 4f2fa87c6c44c004771f108dcd109e48e6080559 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 19:12:09 -0400 Subject: [PATCH 1/2] A closed PR's head can be superseded again The LOST row says to record why a closed PR's commits are dropped, and judge superseded is that record, but the verdict read required an open PR, so every row refused with "no longer an open source". Each verdict now declares the read it stands on: superseded reads any state, everything else still requires open. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XbFMSpSd8zQxyozgEV2Re7 --- src/agent_comms/head_review.py | 23 +++++++++++++++++++---- src/agent_comms/order_tools.py | 2 +- src/agent_comms/todos.py | 8 ++++++++ tests/test_head_review.py | 13 +++++++++++++ 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/agent_comms/head_review.py b/src/agent_comms/head_review.py index 250ecda460..795c593688 100644 --- a/src/agent_comms/head_review.py +++ b/src/agent_comms/head_review.py @@ -85,9 +85,9 @@ def among(cls, repositories: tuple[RepositoryName, ...], pr: int, head: str) -> raise RecordProblems(problems) return cls(held[0].repo, pr, head) - @classmethod - def at(cls, repo: RepositoryName, pr: int) -> OpenPullRequestHead: - """Read GitHub's head, its base branch and draft state in one request.""" + @staticmethod + def forge_read(repo: RepositoryName, pr: int) -> str: + """GitHub's head, base branch, draft state and state in one request.""" read = repo.rest( f"pulls/{pr}", "--jq", @@ -95,7 +95,11 @@ def at(cls, repo: RepositoryName, pr: int) -> OpenPullRequestHead: ) if read.returncode: raise ToolRefusedError(read.stderr.strip() or f"#{pr} could not be read.") - return OpenPullRequestHead.from_forge(repo, pr, read.stdout) + return read.stdout + + @classmethod + def at(cls, repo: RepositoryName, pr: int) -> OpenPullRequestHead: + return OpenPullRequestHead.from_forge(repo, pr, cls.forge_read(repo, pr)) def change_identity(self, reader: WorkBranches, base: str) -> PullRequestChange: """Compare the PR's patch, never the unrelated changes in its base tree. @@ -124,6 +128,17 @@ def current(self) -> OpenPullRequestHead: ) return current + def closed_or_open(self) -> ReadPullRequestHead: + """GitHub's read in whatever state the PR is in, refused unless this is still its head: + a closed PR's head can only be dropped, so nothing here may arm or integrate it.""" + head, base, *_ = self.forge_read(self.repo, self.pr).split() + current = ReadPullRequestHead(self.repo, self.pr, head, base=base) + if not current.holds(self.head): + raise ToolRefusedError( + f"#{self.pr} moved to {current.head[:9]} since {self.head} was reviewed; review again." + ) + return current + class SourceCheckout(Checkout, ABC): """The nominal checkout capability a captured source needs to build its tree.""" diff --git a/src/agent_comms/order_tools.py b/src/agent_comms/order_tools.py index c89186f630..53d2582173 100644 --- a/src/agent_comms/order_tools.py +++ b/src/agent_comms/order_tools.py @@ -227,7 +227,7 @@ def apply(self, comms: Comms, caller: ThreadCaller) -> HeadReviewed: actors.append(caller.head_review_actor(comms, self.request, order)) heads = [] with RecordProblems.collecting(problems, '--head', 'supply the current PR head SHA with --head'): - heads.append(self.request.current()) + heads.append(self.operation.current_head(self.request)) preparations = [] for current in heads: with RecordProblems.collecting(problems, self.operation.preparation_field, diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index e9a044cf38..4c7285dc56 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -4245,6 +4245,10 @@ class HeadVerdict(HeadReview, declared_name=None): preparation_field: ClassVar[str] = '--head' preparation_fix: ClassVar[str] = 'supply the current PR head SHA with --head' + def current_head(self, request: PullRequestHead) -> ReadPullRequestHead: + """The read this decision stands on: an open PR at the reviewed head.""" + return request.current() + @classmethod def require_merge(cls, request: PullRequestHead, repositories: tuple[Repo, ...]) -> None: """Source review cannot authorize even the owner's direct merge path.""" @@ -4455,6 +4459,10 @@ class SupersededHeadVerdict(DecidedHeadVerdict): def supersedes(self) -> bool: return True + def current_head(self, request: PullRequestHead) -> ReadPullRequestHead: + """Dropping a head is how a closed PR's lost commits get their recorded reason.""" + return request.closed_or_open() + def description(self, reviewer: str, subject: str) -> str: return f"{super().description(reviewer, subject)} (replaced by {self.replacement})" diff --git a/tests/test_head_review.py b/tests/test_head_review.py index 558a904633..ec91c89a13 100644 --- a/tests/test_head_review.py +++ b/tests/test_head_review.py @@ -1412,3 +1412,16 @@ def test_another_commanders_linked_request_stays_off_the_orderless_board(head_bu held.generation).record(comms.goals, todo.id) assert ready.pr not in {rule.request.pr for rule in OrderlessPullRequest.scan_orderless(comms.goals, ProgressRead(()))} + + +def test_only_superseded_may_decide_a_closed_head(head_bus): + comms, github, request = head_bus + stamp = datetime.fromtimestamp(time.time() - 601, UTC).strftime('%Y-%m-%dT%H:%M:%SZ') + github.put(request.repo, ClosedListedPullRequest( + request.pr, 'feature', request.head, closed_at=stamp, updated=stamp, + base='main', reviewable_times=(stamp,))) + with pytest.raises(Exception, match='no longer an open source'): + invoke(comms, request, ReviewedHeadVerdict(ACCEPTED)) + invoke(comms, request, SupersededHeadVerdict(ACCEPTED, 'P2')) + reviewed, = StoredHeadReview.head_reviews(comms.goals.todos, request) + assert reviewed.verdict.supersedes() From 9b21032f8f849116f5e7594a934776447dbd867e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 31 Oct 2026 01:01:38 -0400 Subject: [PATCH 2/2] A prepared review holds the head as read, open or closed Superseded decides closed heads, so PreparedHeadReview cannot promise an open one. Its field now states what its consumers read. Fixes the debt-src finding at order_tools.py:235 and the long line at head_review.py:138. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XbFMSpSd8zQxyozgEV2Re7 --- src/agent_comms/head_review.py | 5 +++-- src/agent_comms/todos.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/head_review.py b/src/agent_comms/head_review.py index 861dceb526..3bda1a1f21 100644 --- a/src/agent_comms/head_review.py +++ b/src/agent_comms/head_review.py @@ -124,7 +124,8 @@ def current(self) -> OpenPullRequestHead: current = self.at(self.repo, self.pr) if not current.holds(self.head): raise ToolRefusedError( - f"#{self.pr} moved to {current.head[:9]} since {self.head} was reviewed; review again." + f"#{self.pr} moved to {current.head[:9]} since {self.head} was reviewed; " + "review again." ) return current @@ -783,7 +784,7 @@ def record_review( class PreparedHeadReview: """The current forge head and its checked status text, read once before recording.""" - current: OpenPullRequestHead + current: ReadPullRequestHead subject: str diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 77acf2f639..3219b44a07 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -4309,7 +4309,7 @@ def reviewed(self, goals: Goals, request: PullRequestHead, prepared: PreparedHea self.after_record(goals, recorded) return HeadReviewed(recorded, self.landing(prepared.current)) - def landing(self, current: OpenPullRequestHead) -> MergeArm: + def landing(self, current: ReadPullRequestHead) -> MergeArm: """Source verdicts never request merge; native explicit promotion owns enqueueing.""" return UntouchedMergeArm()