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

Merged
dongar merged 2 commits from gitea-approvals into review-loop 2026-09-07 22:47:40 +00:00
6 changed files with 409 additions and 4 deletions
Showing only changes of commit d508e9a9b7 - Show all commits

No files matched your search

+80
View File
@@ -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
]
+92 -3
View File
@@ -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.
+6
View File
@@ -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"})
+210
View File
@@ -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"
+7
View File
@@ -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)
+14 -1
View File
@@ -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)