Redeliver an interrupted review comment, and close the issue on completion
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m16s
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m16s
Fixes counterexamples 6 and 7 from specs/orchestrator.qnt, the last two. 6. A crash while the executor answered a review comment lost it: its cursor is saved before the work (at most once), and a restart in PR_OPEN resumed quietly. Now pr.reviewing marks the comment until the executor returns (every way it returns, but not a crash), and boot resumes the watch just before it, with a notice. The mark stays until it is answered, so a second crash cannot lose it either. 7. 'project complete' left the plan issue open. It now closes it with a note, after IDLE is saved, as abandon does (shared helper). The model marks and redelivers, and closes the issue; scenarios 6 and 7 now expect their properties to hold. A mutant without redelivery fails scenario 6. All eight properties held together over 300,000 traces. No xfail reproductions remain. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE
This commit is contained in:
1 parent
83a4bd72b5
commit
848bd9f122
4 files changed
+153
-52
No files matched your search
@@ -351,10 +351,16 @@ class Orchestrator:
|
||||
# 8): resume, and watch the PR again from its cursor.
|
||||
pr = self._data.get("pr") or {}
|
||||
self._watch_pr(pr)
|
||||
self._transport.send_message(
|
||||
f"restarted with PR #{pr.get('number')} open; still watching it "
|
||||
"for review comments"
|
||||
)
|
||||
if pr.get("reviewing"):
|
||||
self._transport.send_message(
|
||||
f"restarted while answering review comment {pr['reviewing']} "
|
||||
f"on PR #{pr.get('number')}; answering it again"
|
||||
)
|
||||
else:
|
||||
self._transport.send_message(
|
||||
f"restarted with PR #{pr.get('number')} open; still watching "
|
||||
"it for review comments"
|
||||
)
|
||||
elif state.state == IDLE or (
|
||||
state.state in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF) and gate
|
||||
):
|
||||
@@ -532,7 +538,7 @@ class Orchestrator:
|
||||
return
|
||||
self._handle_gate_reply(msg, text, about)
|
||||
elif current == PR_OPEN:
|
||||
self._handle_pr_open(text)
|
||||
self._handle_pr_open(text, msg.sender)
|
||||
elif current == AWAIT_INPUT:
|
||||
self._handle_answer(text)
|
||||
else:
|
||||
@@ -869,13 +875,23 @@ class Orchestrator:
|
||||
|
||||
pr = self._data.get("pr") or {}
|
||||
milestone = int(pr.get("milestone", self._data.get("milestone", 1)))
|
||||
# Taken at most once, but answered at least once: until the executor
|
||||
# returns, the comment is marked, and a restart delivers it again.
|
||||
pr["reviewing"] = comment.id
|
||||
self._save(f"answering review comment {comment.id}")
|
||||
# The comment enters the transcript, so the executor sees review
|
||||
# feedback as conversation rather than as an out-of-band instruction.
|
||||
self._store.append_event(
|
||||
UserMessage(text=comment.body, source=f"gitea:comment:{comment.id}")
|
||||
)
|
||||
with self._long_work():
|
||||
outcome = self._executor.run(milestone)
|
||||
try:
|
||||
with self._long_work():
|
||||
outcome = self._executor.run(milestone)
|
||||
except Exception:
|
||||
# Reported to the operator by _settled: answered, if not well.
|
||||
self._done_reviewing(pr)
|
||||
raise
|
||||
self._done_reviewing(pr)
|
||||
if outcome.status == "stopped":
|
||||
self._halt_for_stop(
|
||||
what=f"the response to review comment {comment.id}",
|
||||
@@ -913,6 +929,13 @@ class Orchestrator:
|
||||
if pr.get("number"):
|
||||
self._publisher.comment(int(pr["number"]), note)
|
||||
|
||||
def _done_reviewing(self, pr: dict[str, Any]) -> None:
|
||||
"""The executor returned: the comment is being answered one way or
|
||||
another. Deliberately not in a `finally`: if the process dies
|
||||
mid-response, the mark must survive so the comment is redelivered."""
|
||||
if pr.pop("reviewing", None) is not None:
|
||||
self._save("review comment answered")
|
||||
|
||||
def _publish_milestone(self, milestone: int) -> None:
|
||||
if self._publisher is None:
|
||||
# No PR to wait on, but PR_OPEN's commands still lead on.
|
||||
@@ -988,11 +1011,8 @@ class Orchestrator:
|
||||
for intent in self._store.pending_intents():
|
||||
self._store.abort_intent(intent.id, cause=note)
|
||||
problems: list[str] = []
|
||||
if self._board is not None and board_state.get("issue"):
|
||||
try:
|
||||
self._board.close(int(board_state["issue"]), note)
|
||||
except GiteaError as exc:
|
||||
problems.append(f"plan issue #{board_state['issue']}: {exc}")
|
||||
if problem := self._close_plan_issue(board_state, note):
|
||||
problems.append(problem)
|
||||
if self._publisher is not None and pr.get("number"):
|
||||
try:
|
||||
self._publisher.close_pull_request(int(pr["number"]), note)
|
||||
@@ -1017,6 +1037,17 @@ class Orchestrator:
|
||||
except GiteaError:
|
||||
return None
|
||||
|
||||
def _close_plan_issue(self, board_state: dict[str, Any], note: str) -> str | None:
|
||||
"""Close a finished task's plan issue with a note; what went wrong,
|
||||
if anything, for the operator's message."""
|
||||
if self._board is None or not board_state.get("issue"):
|
||||
return None
|
||||
try:
|
||||
self._board.close(int(board_state["issue"]), note)
|
||||
except GiteaError as exc:
|
||||
return f"plan issue #{board_state['issue']}: {exc}"
|
||||
return None
|
||||
|
||||
def _return_to_pr_open(self, cause: str) -> None:
|
||||
self._transition(PR_OPEN, cause=cause)
|
||||
pr = self._data.get("pr") or {}
|
||||
@@ -1030,14 +1061,18 @@ class Orchestrator:
|
||||
"""Watch the open PR from its recorded cursor, catching up on what
|
||||
arrived while it was not watched."""
|
||||
if pr.get("number"):
|
||||
after = pr.get("last_comment")
|
||||
reviewing = pr.get("reviewing")
|
||||
if reviewing:
|
||||
# Its answer never finished: resume just before it.
|
||||
after = min(
|
||||
int(after if after is not None else reviewing), reviewing - 1
|
||||
)
|
||||
self._watch(
|
||||
"review",
|
||||
int(pr["number"]),
|
||||
self.handle_review_comment,
|
||||
after=pr.get("last_comment"),
|
||||
"review", int(pr["number"]), self.handle_review_comment, after=after
|
||||
)
|
||||
|
||||
def _handle_pr_open(self, text: str) -> None:
|
||||
def _handle_pr_open(self, text: str, sender: str = "the operator") -> None:
|
||||
command = command_form(text)
|
||||
if command == "next milestone":
|
||||
self._unwatch("review")
|
||||
@@ -1047,13 +1082,18 @@ class Orchestrator:
|
||||
self._transition(MILESTONE_PLANNING, cause="operator: next milestone")
|
||||
self._run_planning_turn(f"milestone-{self._data['milestone']}")
|
||||
elif command == "project complete":
|
||||
board_state = self._data.get("board") or {}
|
||||
self.stop_watching()
|
||||
self._data = {**_fresh_data()}
|
||||
self._project = None
|
||||
self._store.oplog.task_id = None
|
||||
self._transition(IDLE, cause="operator: project complete")
|
||||
# IDLE first, as for abandon; the PR is the operator's to merge.
|
||||
problem = self._close_plan_issue(board_state, f"completed by {sender}")
|
||||
self._transport.send_message(
|
||||
"project closed; say 'use <project>' to start another"
|
||||
"project closed"
|
||||
+ (f"; could not close {problem}" if problem else "")
|
||||
+ ". Say 'use <project>' to start another"
|
||||
)
|
||||
else:
|
||||
pr = self._data.get("pr") or {}
|
||||
|
||||
Reference in new issue
Block a user