Externalise plans to Gitea and accept approvals there (A4, Part H)

- approvals.py: PlanBoard opens one issue per project and posts each
  plan revision as a comment carrying its rev tag. The state store
  keeps only a pointer, so gate state can be re-derived from Gitea
  after a state-volume loss — external truth wins
- Gitea comments are a second approval source: revision binding, the
  typed word approve, and first-approval-wins work exactly as over
  transport, with a per-channel allowlist because a Gitea login is
  not a Matrix MXID
- Outcomes are echoed to the other channel, so the two records cannot
  silently disagree about what was approved
- A Gitea outage degrades durability of the record, not the ability
  to approve: planning proceeds and Matrix remains a complete channel
- Live test now asserts the server refuses the agent's own merge and
  its push to the protected branch. That property was previously
  true only because our code did not attempt it; branch protection
  makes it structural, and this test fails if the protection is ever
  relaxed
- Fixed a race in the process-group test: killing a group is
  asynchronous and the grandchild is an orphan, so its exit is not
  synchronised with our return; the test now waits for death

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nisse-dev/src/nisse_dev/approvals.py

@@ -0,0 +1,80 @@
+"""Plan externalisation and Gitea-backed approvals (spec A4, Part H).
+
+Plans are posted as comments on a per-project Gitea issue. The approval
+comment — its author, timestamp, and revision tag — is the durable off-box
+record, authenticated by Gitea's own auth. The state store keeps only a
+pointer to it, so boot reconciliation can re-derive gate state from Gitea
+even after the state volume is lost: external truth wins.
+
+Approvals arriving here bind to a revision exactly as they do over transport
+(A4). The first valid approval on any channel wins; the orchestrator echoes
+the outcome to the other channel so the records converge.
+"""
+
+from dataclasses import dataclass
+
+from nisse_dev.gitea import Comment, GiteaClient, GiteaError
+
+APPROVAL_HINT = (
+    "Reply `approve {tag}` to approve this revision, or comment with feedback "
+    "to reject it."
+)
+
+
+@dataclass(frozen=True)
+class PlanPost:
+    issue: int
+    comment_id: int
+
+
+class PlanBoard:
+    """The Gitea side of plan gating: one issue per project, one comment per
+    plan revision."""
+
+    def __init__(self, client: GiteaClient, repo: str) -> None:
+        self._client = client
+        self._repo = repo
+
+    @property
+    def repo(self) -> str:
+        return self._repo
+
+    def open_project_issue(self, project: str, task_id: str) -> int:
+        return self._client.create_issue(
+            self._repo,
+            title=f"nisse-dev: {project}",
+            body=(
+                f"Plan gate for project `{project}` (task `{task_id}`).\n\n"
+                "Each plan revision is posted here as a comment. Approving a "
+                "revision by comment is equivalent to approving it in Matrix; "
+                "the first valid approval on either channel wins."
+            ),
+        )
+
+    def post_plan(self, issue: int, gate: str, rev: int, plan: str) -> PlanPost:
+        tag = f"rev-{rev}"
+        body = "\n".join(
+            [
+                f"## Plan for `{gate}` ({tag})",
+                "",
+                plan,
+                "",
+                "---",
+                APPROVAL_HINT.format(tag=tag),
+            ]
+        )
+        comment = self._client.post_comment(self._repo, issue, body)
+        return PlanPost(issue=issue, comment_id=comment.id)
+
+    def echo(self, issue: int, text: str) -> None:
+        """Mirror an outcome decided on another channel, so the Gitea record
+        does not silently disagree with Matrix."""
+        try:
+            self._client.post_comment(self._repo, issue, text)
+        except GiteaError:
+            pass
+
+    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
+        ]

nisse-dev/src/nisse_dev/control.py

@@ -94,6 +94,17 @@ class MilestonePublisher(Protocol):
     def comment(self, index: int, body: str) -> None: ...
 
 
+class PlanBoard(Protocol):
+    """The Gitea side of plan gating; the concrete one is in
+    nisse_dev.approvals."""
+
+    def open_project_issue(self, project: str, task_id: str) -> int: ...
+
+    def post_plan(self, issue: int, gate: str, rev: int, plan: str) -> Any: ...
+
+    def echo(self, issue: int, text: str) -> None: ...
+
+
 class PlanningAgent(Protocol):
     def plan(
         self,
@@ -133,6 +144,10 @@ def _fresh_data() -> dict[str, Any]:
         "task_id": None,
         "roles": None,  # A11 mapping, pinned at task start
         "pr": None,  # the open PR for the current milestone, once published
+        # Pointer to the Gitea plan record (Part H): the state store holds
+        # only this, so gate state can be re-derived from Gitea after a
+        # state-volume loss.
+        "board": None,
     }
 
 
@@ -147,6 +162,7 @@ class Orchestrator:
         executor: "MilestoneExecutor | None" = None,
         publisher: "MilestonePublisher | None" = None,
         gitea_operators: Iterable[str] = (),
+        board: "PlanBoard | None" = None,
     ) -> None:
         self._store = store
         self._transport = transport
@@ -159,6 +175,7 @@ class Orchestrator:
         # are different names for the same person, and neither implies
         # the other.
         self._gitea_operators = set(gitea_operators)
+        self._board = board
         # Transport and the comment poller deliver on separate threads;
         # state transitions must not interleave.
         self._lock = threading.RLock()
@@ -260,6 +277,19 @@ class Orchestrator:
         self._data = {**_fresh_data(), "task_id": task_id, "roles": pinned}
         self._project = project
         self._store.oplog.task_id = task_id
+        if self._board is not None:
+            try:
+                self._data["board"] = {
+                    "issue": self._board.open_project_issue(project, task_id),
+                    "last_comment": 0,
+                }
+            except GiteaError as exc:
+                # Gitea being down must not block planning; Matrix remains a
+                # complete approval channel on its own.
+                self._transport.send_message(
+                    f"note: could not open the Gitea plan issue ({exc}); "
+                    "approvals will be Matrix-only for this task"
+                )
         self._transition(PLANNING_ARCH, cause=f"operator command: use {project}")
         self._run_planning_turn("arch")
 
@@ -308,6 +338,7 @@ class Orchestrator:
             f"{action.content}\n\nreply 'approve' to approve rev-{rev}; "
             "any other reply is rejection feedback"
         )
+        self._post_plan_to_board(gate_name, rev, action.content)
         self._store.oplog.emit(
             "gate.open",
             cause=f"plan submitted (rev-{rev})",
@@ -317,6 +348,12 @@ class Orchestrator:
         )
 
     def _handle_gate_reply(self, msg: InboundMessage, text: str) -> None:
+        self._apply_gate_reply(sender=msg.sender, text=text, channel="transport")
+
+    def _apply_gate_reply(self, *, sender: str, text: str, channel: str) -> None:
+        """Shared by every approval source (A4). The allowlist is per channel:
+        a Matrix MXID and a Gitea login are different names, and holding one
+        says nothing about holding the other."""
         gate_data = self._data["gate"]
         name, rev = gate_data["name"], gate_data["rev"]
         milestone = self._data["milestone"]
@@ -332,9 +369,8 @@ class Orchestrator:
                 )
             return
         revision = PlanRevision(gate=name, rev=rev, content=gate_data["plan"])
-        result = Gate(revision, self._operators).evaluate(
-            msg.sender, text, channel="transport"
-        )
+        allowlist = self._operators if channel == "transport" else self._gitea_operators
+        result = Gate(revision, allowlist).evaluate(sender, text, channel=channel)
         if isinstance(result, Ignored):
             if result.notice:
                 self._transport.send_message(result.notice)
@@ -347,12 +383,19 @@ class Orchestrator:
                 rev=f"rev-{rev}",
                 channel=result.channel,
             )
+            self._echo_outcome(
+                channel,
+                f"{name} rev-{rev} rejected by {result.approver}: {result.feedback}",
+            )
             self._data["gate"] = None
             back = PLANNING_ARCH if name == "arch" else MILESTONE_PLANNING
             self._transition(back, cause=f"plan rejected ({name} rev-{rev})")
             self._run_planning_turn(name, feedback=result.feedback)
             return
         assert isinstance(result, Approved)
+        self._echo_outcome(
+            channel, f"{name} rev-{rev} approved by {result.approver} via {channel}"
+        )
         self._store.oplog.emit(
             "gate.approve",
             cause=f"approved by {result.approver}",
@@ -395,6 +438,52 @@ class Orchestrator:
             pending={"kind": "milestone_stopped", "milestone": milestone},
         )
 
+    def _echo_outcome(self, deciding_channel: str, text: str) -> None:
+        """A gate satisfied on one channel is echoed to the other, so neither
+        record silently disagrees about what was approved (A4)."""
+        board_state = self._data.get("board")
+        if deciding_channel != "gitea" and self._board is not None and board_state:
+            self._board.echo(board_state["issue"], text)
+        elif deciding_channel == "gitea":
+            self._transport.send_message(text)
+
+    def _post_plan_to_board(self, gate_name: str, rev: int, plan: str) -> None:
+        board_state = self._data.get("board")
+        if self._board is None or not board_state:
+            return
+        try:
+            post = self._board.post_plan(board_state["issue"], gate_name, rev, plan)
+        except GiteaError:
+            # Matrix already carries the plan; a Gitea outage degrades the
+            # durability of the record, not the ability to approve.
+            return
+        board_state["comment_id"] = post.comment_id
+
+    def handle_plan_comment(self, comment: Any) -> None:
+        """A comment on the project issue may satisfy an open plan gate (A4).
+
+        Approvals bind to a revision here exactly as over transport, and the
+        first valid approval on either channel wins.
+        """
+        with self._lock:
+            self._handle_plan_comment_locked(comment)
+
+    def _handle_plan_comment_locked(self, comment: Any) -> None:
+        if comment.author not in self._gitea_operators:
+            return
+        state = self._store.get_state()
+        if state is None or state.state not in (
+            AWAIT_ARCH_SIGNOFF,
+            AWAIT_MILESTONE_SIGNOFF,
+        ):
+            return
+        self._store.append_event(
+            UserMessage(text=comment.body, source=f"gitea:comment:{comment.id}")
+        )
+        self._apply_gate_reply(
+            sender=comment.author, text=comment.body.strip(), channel="gitea"
+        )
+
     def handle_review_comment(self, comment: Any) -> None:
         """React to a PR review comment (B2). Called from the poller thread.
 

nisse-dev/src/nisse_dev/gitea.py

@@ -151,6 +151,12 @@ class GiteaClient:
             created_at=data.get("created_at", ""),
         )
 
+    def create_issue(self, repo: str, title: str, body: str) -> int:
+        data = self._request(
+            "POST", f"/repos/{repo}/issues", json={"title": title, "body": body}
+        )
+        return int(data["number"])
+
     def close_pull_request(self, repo: str, index: int) -> None:
         self._request("PATCH", f"/repos/{repo}/pulls/{index}", json={"state": "closed"})
 

nisse-dev/tests/test_approvals.py

@@ -0,0 +1,210 @@
+"""Plan externalisation to Gitea and approvals arriving on either channel
+(spec A4, Part H)."""
+
+import io
+from pathlib import Path
+from typing import Any
+
+from nisse_dev.approvals import PlanBoard
+from nisse_dev.canonical import CanonicalEvent, ToolCall
+from nisse_dev.control import (
+    AWAIT_ARCH_SIGNOFF,
+    AWAIT_MILESTONE_SIGNOFF,
+    MILESTONE_PLANNING,
+    Orchestrator,
+)
+from nisse_dev.gitea import Comment, GiteaError
+from nisse_dev.llm import RoleMap
+from nisse_dev.oplog import OpLog
+from nisse_dev.planner import LLMPlanningAgent
+from nisse_dev.state import StateStore
+from nisse_dev.transport import InMemoryTransport
+
+ROLES = RoleMap(executor="x", planner="y", delegate="y", consultant="y")
+
+
+class FakeGitea:
+    def __init__(self, *, fail: bool = False) -> None:
+        self.issues: list[dict[str, str]] = []
+        self.comments: list[tuple[int, str]] = []
+        self._fail = fail
+        self._next_id = 100
+
+    def create_issue(self, repo: str, title: str, body: str) -> int:
+        if self._fail:
+            raise GiteaError("gitea is down")
+        self.issues.append({"title": title, "body": body})
+        return len(self.issues)
+
+    def post_comment(self, repo: str, index: int, body: str) -> Comment:
+        if self._fail:
+            raise GiteaError("gitea is down")
+        self.comments.append((index, body))
+        self._next_id += 1
+        return Comment(
+            id=self._next_id, author="nisse-dev-testing", body=body, created_at="t"
+        )
+
+    def list_comments(self, repo: str, index: int, since: str | None = None):
+        return []
+
+
+class ScriptedClient:
+    def __init__(self) -> None:
+        self.calls = 0
+
+    def complete(self, events, tools, model, system=None) -> list[CanonicalEvent]:
+        self.calls += 1
+        return [
+            ToolCall(
+                call_id=f"c{self.calls}",
+                tool="submit_plan",
+                arguments={"type": "x", "content": f"plan {self.calls}"},
+            )
+        ]
+
+
+def build(
+    tmp_path: Path, gitea: Any
+) -> tuple[Orchestrator, StateStore, InMemoryTransport, PlanBoard]:
+    store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()))
+    transport = InMemoryTransport()
+    board = PlanBoard(gitea, "o/sandbox")
+    orch = Orchestrator(
+        store,
+        transport,
+        LLMPlanningAgent(ScriptedClient(), store, ROLES),
+        operators={"don"},
+        roles=ROLES,
+        gitea_operators={"dongar"},
+        board=board,
+    )
+    orch.start()
+    return orch, store, transport, board
+
+
+def comment(body: str, author: str = "dongar", cid: int = 900) -> Comment:
+    return Comment(id=cid, author=author, body=body, created_at="t")
+
+
+def test_project_start_opens_an_issue_and_posts_the_plan(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    _, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+
+    assert gitea.issues[0]["title"] == "nisse-dev: demo"
+    issue, body = gitea.comments[0]
+    assert issue == 1
+    assert "plan 1" in body
+    assert "approve rev-1" in body  # the tag is printed for copy-paste (A4)
+    # The store keeps a pointer, not the plan record itself (Part H).
+    assert store.get_state().data["board"]["issue"] == 1
+
+
+def test_approval_by_gitea_comment_satisfies_the_gate(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, store, _, _ = build(tmp_path, gitea)
+    orch._transport.receive("don", "use demo")
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF
+
+    orch.handle_plan_comment(comment("approve rev-1"))
+
+    assert store.get_state().state == AWAIT_MILESTONE_SIGNOFF
+
+
+def test_stale_revision_from_gitea_is_refused(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+    transport.receive("don", "too vague")  # now at rev-2
+
+    orch.handle_plan_comment(comment("approve rev-1"))
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF  # not advanced
+
+    orch.handle_plan_comment(comment("approve rev-2", cid=901))
+    assert store.get_state().state == AWAIT_MILESTONE_SIGNOFF
+
+
+def test_non_operator_gitea_comment_cannot_approve(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+
+    orch.handle_plan_comment(comment("approve rev-1", author="drive-by"))
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF
+
+
+def test_agents_own_comment_cannot_approve_its_plan(tmp_path: Path) -> None:
+    # The agent posts plans as nisse-dev-testing; that login is not on the
+    # operator allowlist, so its own comments can never satisfy its gate (A4).
+    gitea = FakeGitea()
+    orch, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+
+    orch.handle_plan_comment(comment("approve rev-1", author="nisse-dev-testing"))
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF
+
+
+def test_rejection_by_gitea_comment_replans(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+
+    orch.handle_plan_comment(comment("needs more detail on the schema"))
+
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF
+    assert "plan 2" in transport.outbound[-1].text  # replanned
+
+
+def test_matrix_approval_is_echoed_to_gitea(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    _, _, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+    transport.receive("don", "approve")
+
+    echoes = [body for _, body in gitea.comments if "approved by don" in body]
+    assert echoes, "the Gitea record must not silently disagree with Matrix"
+
+
+def test_gitea_approval_is_echoed_to_matrix(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, _, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+    orch.handle_plan_comment(comment("approve rev-1"))
+
+    assert any("approved by dongar via gitea" in m.text for m in transport.outbound)
+
+
+def test_gitea_outage_leaves_matrix_a_complete_channel(tmp_path: Path) -> None:
+    gitea = FakeGitea(fail=True)
+    _, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+
+    # Planning proceeded and the gate is usable despite Gitea being down.
+    assert store.get_state().state == AWAIT_ARCH_SIGNOFF
+    assert any("Matrix-only" in m.text for m in transport.outbound)
+    transport.receive("don", "approve")
+    assert store.get_state().state == AWAIT_MILESTONE_SIGNOFF
+
+
+def test_comments_are_ignored_outside_a_gate(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    orch, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+    transport.receive("don", "approve")  # arch approved; now at milestone gate
+    transport.receive("don", "approve")  # milestone approved, awaiting start
+    before = store.get_state().state
+
+    orch.handle_plan_comment(comment("approve rev-1", cid=902))
+    assert store.get_state().state == before
+
+
+def test_board_records_the_milestone_plan_too(tmp_path: Path) -> None:
+    gitea = FakeGitea()
+    _, store, transport, _ = build(tmp_path, gitea)
+    transport.receive("don", "use demo")
+    transport.receive("don", "approve")
+
+    assert store.get_state().state in (MILESTONE_PLANNING, AWAIT_MILESTONE_SIGNOFF)
+    posted = [body for _, body in gitea.comments if "milestone-1" in body]
+    assert posted, "the milestone plan must be externalised as well"

nisse-dev/tests/test_tools.py

@@ -1,4 +1,5 @@
 import os
+import time
 from pathlib import Path
 
 import pytest
@@ -97,6 +98,12 @@ def test_timeout_kills_the_whole_process_group(tmp_path: Path) -> None:
     assert pidfile.exists()
     pid = int(pidfile.read_text().strip())
     assert _is_running(os.getpid()), "liveness check is broken, not the kill"
+    # SIGKILL to a process group is asynchronous, and the grandchild is an
+    # orphan by now, so its exit is not synchronised with our return. Wait
+    # for death rather than assuming it is instantaneous.
+    deadline = time.monotonic() + 5
+    while _is_running(pid) and time.monotonic() < deadline:
+        time.sleep(0.05)
     assert not _is_running(pid)
 
 

tests/integration/test_gitea_live.py

@@ -12,7 +12,7 @@ from pathlib import Path
 import pytest
 
 from nisse_dev.envfile import load_env_file
-from nisse_dev.git import GitRepo, authenticated_url
+from nisse_dev.git import GitError, GitRepo, authenticated_url
 from nisse_dev.gitea import GiteaClient, pull_request_reconciler
 from nisse_dev.state import Intent
 
@@ -88,6 +88,19 @@ def test_push_open_pr_and_reconcile(tmp_path: Path) -> None:
 
         # Now the same reconciler must see the effect and refuse to repeat it.
         assert reconcile(intent_for(branch)) is True
+
+        # A4 is only real if the server enforces it. The agent's own token
+        # must not be able to merge the PR it just opened, and must not be
+        # able to push to the protected default branch at all.
+        merged = client._http.post(
+            f"/repos/{repo_path}/pulls/{pr.number}/merge", json={"Do": "merge"}
+        )
+        assert merged.status_code >= 400, (
+            "the agent merged its own PR: branch protection is not enforcing "
+            "the human gate"
+        )
+        with pytest.raises(GitError):
+            repo.push(url, "main", token=token)
     finally:
         if pr is not None:
             client.close_pull_request(repo_path, pr.number)