diff --git a/docs/Architecture-Build-Spec.md b/docs/Architecture-Build-Spec.md index 6430cbc..42ffb9c 100644 --- a/docs/Architecture-Build-Spec.md +++ b/docs/Architecture-Build-Spec.md @@ -909,6 +909,9 @@ EXECUTING autonomous; no per-command approval PR_OPEN → open PR; react to review comments and commits (review loop) → on acceptance → next milestone, or project complete + +any waiting state (a sign-off gate, AWAIT_INPUT, PR_OPEN) + → [abandon] close the plan issue and any open PR, → IDLE ``` ### B2. Requirements @@ -917,8 +920,14 @@ PR_OPEN or the agent switches on its own under 0.2's rules; other projects stay parked at their last position and resume where they left off. - **Operator commands** (over transport): `use `, `approve`, - `reject `, `start milestone `, `status`, `stop` (A4), plus free - text during planning (forwarded to the agent as answers). + `reject `, `start milestone `, `status`, `stop` (A4), `abandon`, + plus free text during planning (forwarded to the agent as answers). + - `abandon` ends the current work item from any waiting state: the "abandon" + among A4's continue / re-plan / abandon, and a close in 0.2's terms. It closes + the plan issue and any open milestone PR with a note, keeps the branches, and + marks any unresolved intent aborted so boot reconciliation does not gate on + work dropped on purpose. While work is running, `stop` comes first. Like + `status`, it is a command in every state, never read as an answer or feedback. - **Plan gating** per A4: architecture plan and each milestone plan require sign-off before the next phase. Enforced structurally. Plans and approvals are externalized to Gitea issue comments (Part H, decided); Matrix and Gitea are both approval diff --git a/nisse-dev/src/nisse_dev/approvals.py b/nisse-dev/src/nisse_dev/approvals.py index 5b5edf1..c6d4c0e 100644 --- a/nisse-dev/src/nisse_dev/approvals.py +++ b/nisse-dev/src/nisse_dev/approvals.py @@ -74,6 +74,12 @@ class PlanBoard: except GiteaError: pass + def close(self, issue: int, note: str) -> None: + """Close the plan issue with a note saying why (e.g. abandoned). + Unlike echo, failures raise: the caller reports them.""" + self._client.post_comment(self._repo, issue, note) + self._client.close_issue(self._repo, issue) + def comments_since(self, issue: int, after_id: int) -> list[Comment]: return [ c for c in self._client.list_comments(self._repo, issue) if c.id > after_id diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index 00c5b12..fe9be26 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/nisse-dev/src/nisse_dev/control.py @@ -55,6 +55,10 @@ TRANSITIONS = { # than passing through a separate FINISHED state. (PR_OPEN, MILESTONE_PLANNING), (PR_OPEN, IDLE), + # abandon (A4, B2): the operator may end the task from any waiting state. + # (AWAIT_INPUT may go anywhere already.) + (AWAIT_ARCH_SIGNOFF, IDLE), + (AWAIT_MILESTONE_SIGNOFF, IDLE), } @@ -65,7 +69,7 @@ _WORKING = (PLANNING_ARCH, MILESTONE_PLANNING, EXECUTING) _BOOT_CHOICES = ( "reply 'continue' to pick up where the restart cut in, " - "or 're-plan' to plan the current step again" + "'re-plan' to plan the current step again, or 'abandon' to drop the task" ) @@ -117,6 +121,8 @@ class MilestonePublisher(Protocol): def comment(self, index: int, body: str) -> None: ... + def close_pull_request(self, index: int, note: str) -> None: ... + class PlanBoard(Protocol): """The Gitea side of plan gating; the concrete one is in @@ -128,6 +134,8 @@ class PlanBoard(Protocol): def echo(self, issue: int, text: str) -> None: ... + def close(self, issue: int, note: str) -> None: ... + class PlanningAgent(Protocol): def plan( @@ -428,6 +436,13 @@ class Orchestrator: text = msg.text.strip() if text == "status": self._send_status(current) + elif text == "abandon": + # Like status, a command in every waiting state: it takes + # precedence over reading the word as an answer or feedback. + if current == IDLE: + self._transport.send_message("no active task; nothing to abandon") + else: + self._abandon(msg.sender) elif current == IDLE: if text.startswith("use "): self._begin_project(text[4:].strip()) @@ -821,7 +836,7 @@ class Orchestrator: self._transition(PR_OPEN, cause=f"[stub] milestone {milestone} done") self._transport.send_message( f"[stub] milestone {milestone} done; no publisher, so no PR — " - "say 'next milestone' or 'project complete'" + "say 'next milestone', 'project complete' or 'abandon'" ) return gate = self._data.get("gate") or {} @@ -869,13 +884,49 @@ class Orchestrator: "'project complete'" ) + def _abandon(self, sender: str) -> None: + """End the task on the operator's word (A4: continue, re-plan, or + abandon), closing its Gitea records. Only reached from a waiting + state: while work runs the reply is "busy; say 'stop'" instead. + + The move to IDLE is made durable first, so a failure while tidying + Gitea can never leave a live task behind a closed plan issue.""" + note = f"abandoned by {sender}" + board_state = self._data.get("board") or {} + pr = self._data.get("pr") or {} + project = self._project + self.stop_watching() + self._data = {**_fresh_data()} + self._project = None + self._store.oplog.task_id = None + self._transition(IDLE, cause=f"operator: abandon ({sender})") + + # Dropped on purpose: boot reconciliation must not gate on it. + 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 self._publisher is not None and pr.get("number"): + try: + self._publisher.close_pull_request(int(pr["number"]), note) + except GiteaError as exc: + problems.append(f"PR #{pr['number']}: {exc}") + closed = "; could not close " + ", ".join(problems) if problems else "" + self._transport.send_message( + f"task on {project} abandoned{closed}. Say 'use ' to start another" + ) + def _return_to_pr_open(self, cause: str) -> None: self._transition(PR_OPEN, cause=cause) pr = self._data.get("pr") or {} self._watch_pr(pr) self._transport.send_message( f"PR #{pr.get('number')} is open and watched for review comments; " - "say 'next milestone' or 'project complete'" + "say 'next milestone', 'project complete' or 'abandon'" ) def _watch_pr(self, pr: dict[str, Any]) -> None: @@ -909,8 +960,8 @@ class Orchestrator: else: pr = self._data.get("pr") or {} self._transport.send_message( - f"PR #{pr.get('number')} is open; say 'next milestone' or " - "'project complete'" + f"PR #{pr.get('number')} is open; say 'next milestone', " + "'project complete' or 'abandon'" ) def _watch( diff --git a/nisse-dev/src/nisse_dev/gitea.py b/nisse-dev/src/nisse_dev/gitea.py index 6a0678f..0bdd3c6 100644 --- a/nisse-dev/src/nisse_dev/gitea.py +++ b/nisse-dev/src/nisse_dev/gitea.py @@ -150,6 +150,11 @@ class GiteaClient: def close_pull_request(self, repo: str, index: int) -> None: self._request("PATCH", f"/repos/{repo}/pulls/{index}", json={"state": "closed"}) + def close_issue(self, repo: str, index: int) -> None: + self._request( + "PATCH", f"/repos/{repo}/issues/{index}", json={"state": "closed"} + ) + def close(self) -> None: self._http.close() diff --git a/nisse-dev/src/nisse_dev/publish.py b/nisse-dev/src/nisse_dev/publish.py index c47152e..4ba7c6f 100644 --- a/nisse-dev/src/nisse_dev/publish.py +++ b/nisse-dev/src/nisse_dev/publish.py @@ -102,6 +102,12 @@ class MilestonePublisher: except GiteaError: pass + def close_pull_request(self, index: int, note: str) -> None: + """Close a milestone PR with a note saying why. The branch is kept, + so the work can be reopened in Gitea.""" + self._client.post_comment(self._repo, index, note) + self._client.close_pull_request(self._repo, index) + def push_followup( self, *, task_id: str, milestone: int, message: str ) -> str | None: diff --git a/nisse-dev/tests/test_abandon.py b/nisse-dev/tests/test_abandon.py new file mode 100644 index 0000000..08716a8 --- /dev/null +++ b/nisse-dev/tests/test_abandon.py @@ -0,0 +1,185 @@ +"""The `abandon` command: ending a task from any waiting state (A4, B2).""" + +import io +from pathlib import Path +from typing import Any + +import pytest + +from nisse_dev.control import ( + AWAIT_ARCH_SIGNOFF, + AWAIT_INPUT, + AWAIT_MILESTONE_SIGNOFF, + IDLE, + PR_OPEN, + AskHuman, + Orchestrator, + SubmitPlan, +) +from nisse_dev.executor import ExecutorOutcome +from nisse_dev.gitea import GiteaError, PullRequest +from nisse_dev.oplog import OpLog +from nisse_dev.publish import PublishResult +from nisse_dev.state import StateStore +from nisse_dev.transport import InMemoryTransport + + +class Planner: + def __init__(self, ask_first: bool = False) -> None: + self._ask = ask_first + + def plan( + self, + gate: str, + project: str, + feedback: str | None = None, + answer: str | None = None, + ) -> SubmitPlan | AskHuman: + if self._ask and answer is None: + return AskHuman("which language?") + return SubmitPlan(f"{gate} plan") + + +class Board: + def __init__(self, *, fail_close: bool = False) -> None: + self.closed: list[tuple[int, str]] = [] + self._fail_close = fail_close + + def open_project_issue(self, project: str, task_id: str) -> int: + return 10 + + def post_plan(self, issue: int, gate: str, rev: int, plan: str) -> Any: + class Posted: + comment_id = 1 + + return Posted() + + def echo(self, issue: int, text: str) -> None: + pass + + def close(self, issue: int, note: str) -> None: + if self._fail_close: + raise GiteaError("gitea is down") + self.closed.append((issue, note)) + + +class Executor: + def run(self, milestone: int) -> ExecutorOutcome: + return ExecutorOutcome("completed", "done") + + +class Publisher: + def __init__(self) -> None: + self.closed: list[tuple[int, str]] = [] + + def reconcilers(self) -> dict[str, Any]: + return {} + + def publish(self, **kwargs: Any) -> PublishResult: + pr = PullRequest(number=20, state="open", head="h", url="u") + return PublishResult(pr, "opened PR #20") + + def close_pull_request(self, index: int, note: str) -> None: + self.closed.append((index, note)) + + +class Harness: + def __init__( + self, tmp_path: Path, *, ask_first: bool = False, board: Board | None = None + ) -> None: + self.store = StateStore( + tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()) + ) + self.transport = InMemoryTransport() + self.board = board or Board() + self.publisher = Publisher() + Orchestrator( + self.store, + self.transport, + Planner(ask_first), + operators={"don"}, + board=self.board, + executor=Executor(), + publisher=self.publisher, + ).start() + + @property + def state(self) -> str: + state = self.store.get_state() + assert state is not None + return state.state + + def say(self, *replies: str) -> None: + for reply in replies: + self.transport.receive("don", reply) + + +WAITING = [ + ("arch sign-off", ["use demo"], AWAIT_ARCH_SIGNOFF), + ("milestone sign-off", ["use demo", "approve"], AWAIT_MILESTONE_SIGNOFF), + ( + "approved, not started", + ["use demo", "approve", "approve"], + AWAIT_MILESTONE_SIGNOFF, + ), + ("PR open", ["use demo", "approve", "approve", "start milestone 1"], PR_OPEN), +] + + +@pytest.mark.parametrize( + ("replies", "waiting"), + [(r, w) for _, r, w in WAITING], + ids=[n for n, _, _ in WAITING], +) +def test_abandon_ends_the_task_from_any_waiting_state( + tmp_path: Path, replies: list[str], waiting: str +) -> None: + h = Harness(tmp_path) + h.say(*replies) + assert h.state == waiting + h.say("abandon") + state = h.store.get_state() + assert state.state == IDLE + assert state.project is None + assert "abandoned" in h.transport.outbound[-1].text + assert h.board.closed == [(10, "abandoned by don")] + # A new task starts cleanly afterwards. + h.say("use other") + assert h.state == AWAIT_ARCH_SIGNOFF + + +def test_abandon_answers_a_question_by_ending_the_task(tmp_path: Path) -> None: + h = Harness(tmp_path, ask_first=True) + h.say("use demo") + assert h.state == AWAIT_INPUT + h.say("abandon") + assert h.state == IDLE + + +def test_abandon_in_pr_open_closes_the_pr(tmp_path: Path) -> None: + h = Harness(tmp_path) + h.say("use demo", "approve", "approve", "start milestone 1") + h.say("abandon") + assert h.publisher.closed == [(20, "abandoned by don")] + + +def test_abandon_aborts_a_pending_publish_intent(tmp_path: Path) -> None: + h = Harness(tmp_path) + h.say("use demo") + h.store.begin_intent("open_pr", {"branch": "b"}, cause="publish cut short") + h.say("abandon") + assert h.store.pending_intents() == [] + + +def test_abandon_still_ends_the_task_when_gitea_is_down(tmp_path: Path) -> None: + h = Harness(tmp_path, board=Board(fail_close=True)) + h.say("use demo", "abandon") + assert h.state == IDLE + assert "could not close plan issue #10" in h.transport.outbound[-1].text + + +def test_abandon_with_no_task_says_so(tmp_path: Path) -> None: + h = Harness(tmp_path) + h.say("abandon") + assert h.state == IDLE + assert "nothing to abandon" in h.transport.outbound[-1].text diff --git a/nisse-dev/tests/test_gitea.py b/nisse-dev/tests/test_gitea.py index 17a4d20..3a35362 100644 --- a/nisse-dev/tests/test_gitea.py +++ b/nisse-dev/tests/test_gitea.py @@ -1,4 +1,5 @@ import json +from pathlib import Path import httpx import pytest @@ -195,3 +196,40 @@ def test_intent_without_a_branch_is_unverifiable() -> None: resolved_at=None, ) assert pull_request_reconciler(client(handler), "o/r")(blank) is None + + +def test_closing_comments_first_then_closes_issue_and_pr() -> None: + from nisse_dev.approvals import PlanBoard + from nisse_dev.git import GitRepo + from nisse_dev.publish import MilestonePublisher + + seen: list[tuple[str, str, dict]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) if request.content else {} + seen.append((request.method, request.url.path, body)) + if request.method == "POST": + return httpx.Response( + 201, json={"id": 5, "user": {"login": "bot"}, "body": body["body"]} + ) + return httpx.Response(201, json={}) + + gitea = client(handler) + PlanBoard(gitea, "o/r").close(10, "abandoned by don") + publisher = MilestonePublisher( + None, # type: ignore[arg-type] -- closing touches neither store nor git + gitea, + "o/r", + GitRepo(Path(".")), + base_url="https://gitea.test", + account="bot", + token="tok", + ) + publisher.close_pull_request(20, "abandoned by don") + + assert seen == [ + ("POST", "/api/v1/repos/o/r/issues/10/comments", {"body": "abandoned by don"}), + ("PATCH", "/api/v1/repos/o/r/issues/10", {"state": "closed"}), + ("POST", "/api/v1/repos/o/r/issues/20/comments", {"body": "abandoned by don"}), + ("PATCH", "/api/v1/repos/o/r/pulls/20", {"state": "closed"}), + ]