M3.3: the trust dial (the agent merges approved PRs)
A project's trust.merge is operator (the default) or approved (L3, D5, #65). Set to approved, the harness, never the model, merges a PR in the PR watch once an approving review from a gitea.operators account is on its current commit, its checks pass (none at all counts; pending waits) and it's mergeable; the item then goes on as after any merge. Each reason to hold off (failed checks, conflicts, a refused merge) is said once per commit. Gitea enforces it: protection requires approvals from an operators-only whitelist and dismisses stale ones. At start, for each approved project, the agent checks by behaviour that its own unapproved merge is refused, with a throwaway branch and PR made through the API (never the workspace); if it isn't refused, the operator merges until a restart. - gitea: approvals, combined check status, a PR's head commit and mergeability, API-made branches and their deletion. - Test and bring-up repositories get the protection. - Tests: the decision (unit), the hard wall on a real Gitea (test_trust_live), an e2e test where the operator only approves, and a walk property, merges_only_approved, checked against a mutant that ignores which commit was approved. Closes #65 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
21cbf1682c
commit
ab469b60bb
17 files changed
+733
-11
No files matched your search
@@ -511,3 +511,10 @@ def test_capability_profiles_are_read() -> None:
|
||||
def test_a_bad_profile_is_reported() -> None:
|
||||
with pytest.raises(ConfigError, match="profiles.m.colour: unknown setting"):
|
||||
build(profiles={"m": {"colour": "red"}})
|
||||
|
||||
|
||||
def test_the_trust_dial_defaults_to_the_operator_merging() -> None:
|
||||
assert only(build()).merge == "operator"
|
||||
assert only(build(**{f"{P}trust__merge": "approved"})).merge == "approved"
|
||||
with pytest.raises(ConfigError, match="trust.merge: must be one of operator"):
|
||||
build(**{f"{P}trust__merge": "always"})
|
||||
@@ -248,3 +248,74 @@ def test_assigned_at_is_the_last_assignment_across_pages() -> None:
|
||||
|
||||
assert client(handler).assigned_at("o/r", 6, "dev-agent") == "t4"
|
||||
assert client(handler).assigned_at("o/r", 6, "nobody") is None
|
||||
|
||||
|
||||
def test_approvals_are_the_standing_approving_reviews() -> None:
|
||||
reviews = [
|
||||
{"state": "APPROVED", "user": {"login": "don"}, "commit_id": "abc"},
|
||||
{
|
||||
"state": "APPROVED",
|
||||
"user": {"login": "don"},
|
||||
"commit_id": "old",
|
||||
"stale": True,
|
||||
},
|
||||
{
|
||||
"state": "APPROVED",
|
||||
"user": {"login": "x"},
|
||||
"commit_id": "abc",
|
||||
"dismissed": True,
|
||||
},
|
||||
{"state": "REQUEST_CHANGES", "user": {"login": "don"}, "commit_id": "abc"},
|
||||
{"state": "COMMENT", "user": {"login": "don"}, "commit_id": "abc"},
|
||||
]
|
||||
handler = lambda request: httpx.Response(200, json=reviews)
|
||||
approvals = client(handler).approvals("o/r", 3)
|
||||
assert [(a.author, a.commit, a.stale) for a in approvals] == [
|
||||
("don", "abc", False),
|
||||
("don", "old", True),
|
||||
]
|
||||
|
||||
|
||||
def test_checks_are_the_combined_status_or_none() -> None:
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
assert request.url.path == "/api/v1/repos/o/r/commits/abc/status"
|
||||
return httpx.Response(200, json=answer)
|
||||
|
||||
answer: dict = {"state": "", "total_count": 0, "statuses": []}
|
||||
assert client(handler).checks("o/r", "abc") == "none"
|
||||
answer = {"state": "pending", "total_count": 1, "statuses": [{}]}
|
||||
assert client(handler).checks("o/r", "abc") == "pending"
|
||||
answer = {"state": "success", "total_count": 2, "statuses": [{}, {}]}
|
||||
assert client(handler).checks("o/r", "abc") == "success"
|
||||
|
||||
|
||||
def test_a_pull_request_carries_its_head_commit_and_mergeability() -> None:
|
||||
data = {
|
||||
**pr_json(5, "m-1"),
|
||||
"head": {"ref": "m-1", "sha": "f00"},
|
||||
"mergeable": True,
|
||||
}
|
||||
pr = client(lambda request: httpx.Response(200, json=data)).pull_request("o/r", 5)
|
||||
assert (pr.head_sha, pr.mergeable) == ("f00", True)
|
||||
|
||||
|
||||
def test_a_check_branch_is_made_through_the_api() -> None:
|
||||
seen: list = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
seen.append((request.method, request.url.path, request.content))
|
||||
return httpx.Response(201, json={})
|
||||
|
||||
gitea = client(handler)
|
||||
gitea.branch_with_file(
|
||||
"o/r", base="main", branch="chk", path="f", text="x", message="m"
|
||||
)
|
||||
gitea.delete_branch("o/r", "chk")
|
||||
body = json.loads(seen[0][2])
|
||||
assert (seen[0][0], seen[0][1]) == ("POST", "/api/v1/repos/o/r/contents/f")
|
||||
assert (body["branch"], body["new_branch"], body["content"]) == (
|
||||
"main",
|
||||
"chk",
|
||||
"eA==",
|
||||
)
|
||||
assert seen[1][:2] == ("DELETE", "/api/v1/repos/o/r/branches/chk")
|
||||
@@ -14,7 +14,14 @@ from nisse_dev.cancel import Cancel
|
||||
from nisse_dev.canonical import ToolCall
|
||||
from nisse_dev.execution import SSHHost
|
||||
from nisse_dev.git import GitRepo
|
||||
from nisse_dev.gitea import Comment, GiteaError, Issue, PullRequest, ReviewNote
|
||||
from nisse_dev.gitea import (
|
||||
Approval,
|
||||
Comment,
|
||||
GiteaError,
|
||||
Issue,
|
||||
PullRequest,
|
||||
ReviewNote,
|
||||
)
|
||||
from nisse_dev.loop.agent import LoopAgent
|
||||
from nisse_dev.loop.driver import Driver
|
||||
from nisse_dev.loop.items import ANSWER, MERGE, WAITING, loop_of
|
||||
@@ -62,6 +69,10 @@ class Forge:
|
||||
self.deps: dict[int, list[int]] = {} # issue -> the issues it depends on
|
||||
self.assigned: list[int] = [] # issues the agent assigned to itself
|
||||
self.reassigned: dict[int, str] = {} # issue -> when assigned again
|
||||
# The trust dial (M3.3): reviews, check statuses, merges by the agent.
|
||||
self.approved: dict[int, list[Approval]] = {}
|
||||
self.status: dict[str, str] = {} # commit -> combined check state
|
||||
self.agent_merged: list[int] = []
|
||||
|
||||
# intake
|
||||
def assigned_issues(self, repo: str, assignee: str) -> list[Issue]:
|
||||
@@ -91,7 +102,40 @@ class Forge:
|
||||
# PRs
|
||||
def _pr(self, n: int) -> PullRequest:
|
||||
p = self.prs[n]
|
||||
return PullRequest(n, p["state"], p["head"], f"https://g/pr/{n}", p["merged"])
|
||||
return PullRequest(
|
||||
n,
|
||||
p["state"],
|
||||
p["head"],
|
||||
f"https://g/pr/{n}",
|
||||
p["merged"],
|
||||
head_sha=self.head_sha(n),
|
||||
)
|
||||
|
||||
def head_sha(self, n: int) -> str:
|
||||
ref = f"refs/heads/{self.prs[n]['head']}"
|
||||
found = subprocess.run(
|
||||
["git", "--git-dir", str(self.bare), "rev-parse", "--verify", "-q", ref],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
return found.stdout.strip()
|
||||
|
||||
def approve(self, n: int, by: str = "don", sha: str = "") -> None:
|
||||
"""A review approving the PR as it is now (or at ``sha``)."""
|
||||
self.approved.setdefault(n, []).append(Approval(by, sha or self.head_sha(n)))
|
||||
|
||||
def approvals(self, repo: str, index: int) -> list[Approval]:
|
||||
return self.approved.get(index, [])
|
||||
|
||||
def checks(self, repo: str, sha: str) -> str:
|
||||
return self.status.get(sha, "none")
|
||||
|
||||
def branch_with_file(self, repo: str, *, base, branch, path, text, message) -> None:
|
||||
git("--git-dir", str(self.bare), "update-ref", f"refs/heads/{branch}", base)
|
||||
|
||||
def delete_branch(self, repo: str, name: str) -> None:
|
||||
git("--git-dir", str(self.bare), "update-ref", "-d", f"refs/heads/{name}")
|
||||
|
||||
def find_pull_request(self, repo: str, head: str) -> PullRequest | None:
|
||||
for n, p in self.prs.items():
|
||||
@@ -150,8 +194,15 @@ class Forge:
|
||||
return {"protected": self.protected, "user_can_push": not self.protected}
|
||||
|
||||
def merge_pull_request(self, repo: str, index: int) -> None:
|
||||
if self.protected:
|
||||
"""As protection would have it: an operator's approval of the
|
||||
PR's current commit, or no merge."""
|
||||
head = self.head_sha(index)
|
||||
if self.protected and not any(
|
||||
a.author == "don" and a.commit == head and not a.stale
|
||||
for a in self.approved.get(index, [])
|
||||
):
|
||||
raise GiteaError("gitea POST merge returned 405: approvals needed")
|
||||
self.agent_merged.append(index)
|
||||
self.merge(index)
|
||||
|
||||
# what the operator does in Gitea
|
||||
|
||||
@@ -243,8 +243,33 @@ class Walk:
|
||||
self.steps: list[str] = []
|
||||
self.shown_at_go: list[str] = [] # item:revision on screen at each go
|
||||
self.now = 1000.0
|
||||
# The trust dial (M3.3): half the walks have the agent merge approved
|
||||
# PRs, behind protection that refuses anything else.
|
||||
self.merge = "approved" if seed % 2 == 0 else "operator"
|
||||
self.agent_merges: list[tuple[int, bool]] = [] # (PR, approved at head)
|
||||
if self.merge == "approved":
|
||||
for w in worlds.values():
|
||||
self._watch_merges(w["forge"])
|
||||
self.agent, self.transport = self._agent()
|
||||
|
||||
def _watch_merges(self, forge: Forge) -> None:
|
||||
"""Protected, and each merge the agent asks for recorded with
|
||||
whether an operator had approved the PR as it was."""
|
||||
forge.protect()
|
||||
merge = forge.merge_pull_request
|
||||
|
||||
def recorded(repo: str, index: int) -> None:
|
||||
if not forge.prs[index]["head"].startswith("nisse-check-"):
|
||||
head = forge.head_sha(index)
|
||||
approved = any(
|
||||
a.author == "don" and a.commit == head and not a.stale
|
||||
for a in forge.approved.get(index, [])
|
||||
)
|
||||
self.agent_merges.append((index, approved))
|
||||
merge(repo, index)
|
||||
|
||||
forge.merge_pull_request = recorded # type: ignore[method-assign]
|
||||
|
||||
def reads_yes(self, proposal: str, reply: str) -> bool:
|
||||
"""The scripted go-ahead reader: a plain yes usually reads as one,
|
||||
and nothing else ever does."""
|
||||
@@ -270,6 +295,7 @@ class Walk:
|
||||
repo=w["repo_name"],
|
||||
workspace=Workspace(w["host"], w["repo"].root, timeout_s=30),
|
||||
milestones=w["milestones"],
|
||||
merge=self.merge,
|
||||
)
|
||||
for name, w in self.worlds.items()
|
||||
],
|
||||
@@ -342,6 +368,8 @@ class Walk:
|
||||
weights.update(
|
||||
{"comment": 4, "merge": 2, "close-pr": 1, "comment-merge": 2}
|
||||
)
|
||||
if self.merge == "approved":
|
||||
weights.update({"approve": 4})
|
||||
action = self.rng.choices(list(weights), list(weights.values()))[0]
|
||||
self.steps.append(f"{action}{self.name(item)}" if item else action)
|
||||
if action == "intake":
|
||||
@@ -455,6 +483,16 @@ class Walk:
|
||||
self.agent.watch()
|
||||
forge.merge(loop.pr)
|
||||
self.agent._schedule()
|
||||
elif action == "approve" and loop and forge.prs[loop.pr]["state"] == "open":
|
||||
# An operator's approval, or someone else's; of the PR as it is,
|
||||
# or of an earlier commit.
|
||||
forge.approve(
|
||||
loop.pr,
|
||||
by=self.rng.choice(["don", "don", "mallory"]),
|
||||
sha=self.rng.choice(["", "0" * 40]),
|
||||
)
|
||||
self.agent.watch()
|
||||
self.agent._schedule()
|
||||
elif action == "merge" and loop and forge.prs[loop.pr]["state"] == "open":
|
||||
forge.merge(loop.pr)
|
||||
self.agent.watch()
|
||||
@@ -497,6 +535,10 @@ class Walk:
|
||||
|
||||
def check(self) -> None:
|
||||
where = f"seed {self.seed}, steps {self.steps}"
|
||||
# D5: the agent asks Gitea to merge only what an operator approved
|
||||
# as it is (Gitea would refuse the rest; the harness mustn't ask).
|
||||
unapproved = [pr for pr, approved in self.agent_merges if not approved]
|
||||
assert not unapproved, f"merges_only_approved: {unapproved}: {where}"
|
||||
self._dangerous_only_when_allowed(where)
|
||||
store = self.world["store"]
|
||||
items = self.items()
|
||||
|
||||
@@ -0,0 +1,187 @@
|
||||
"""The trust dial (L3, D5; M3.3): a project set to `merge: approved` has its
|
||||
PRs merged by the harness once an operator approved the PR's current commit
|
||||
and its checks pass. Gitea enforces the approval (protection); the agent
|
||||
checks at start that it does, and otherwise leaves merging to the operator."""
|
||||
|
||||
import io
|
||||
from typing import Any
|
||||
|
||||
from test_loop_milestones import (
|
||||
DON,
|
||||
FINISH,
|
||||
RESTATE,
|
||||
WORK,
|
||||
Forge,
|
||||
ScriptedModel,
|
||||
call,
|
||||
say,
|
||||
)
|
||||
from test_loop_milestones import item as the_item
|
||||
from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture
|
||||
|
||||
from nisse_dev.cancel import Cancel
|
||||
from nisse_dev.loop.agent import LoopAgent, Project
|
||||
from nisse_dev.loop.driver import Driver
|
||||
from nisse_dev.loop.items import MERGE, loop_of
|
||||
from nisse_dev.oplog import OpLog
|
||||
from nisse_dev.tools import Workspace
|
||||
from nisse_dev.transport import InMemoryTransport
|
||||
from nisse_dev.worker import InlineWorker
|
||||
|
||||
CLOSE = [call("close_item", summary="added and merged")]
|
||||
|
||||
|
||||
def agent_for(
|
||||
world: dict[str, Any], model: ScriptedModel, merge: str = "approved"
|
||||
) -> tuple[LoopAgent, InMemoryTransport]:
|
||||
transport = InMemoryTransport()
|
||||
cancel = Cancel()
|
||||
project = Project(
|
||||
"sandbox",
|
||||
"don/sandbox",
|
||||
Workspace(world["host"], world["repo"].root, timeout_s=30),
|
||||
world["milestones"],
|
||||
merge=merge,
|
||||
)
|
||||
loop = LoopAgent(
|
||||
world["store"],
|
||||
transport,
|
||||
world["forge"],
|
||||
projects=[project],
|
||||
account="nisse-dev",
|
||||
operators={DON},
|
||||
gitea_operators={"don"},
|
||||
driver=Driver(model, "m", cancel=cancel), # type: ignore[arg-type]
|
||||
worker=InlineWorker(),
|
||||
cancel=cancel,
|
||||
oplog=OpLog("nisse-dev", io.StringIO()),
|
||||
)
|
||||
transport.on_message(loop.on_message)
|
||||
return loop, transport
|
||||
|
||||
|
||||
def to_review(world: dict[str, Any], merge: str = "approved", *more: Any) -> Any:
|
||||
"""An item whose PR is up, waiting for the merge."""
|
||||
world["forge"].protect()
|
||||
model = ScriptedModel(RESTATE, [*WORK, *FINISH], *more)
|
||||
agent, transport = agent_for(world, model, merge)
|
||||
agent.boot()
|
||||
agent.intake()
|
||||
say(transport, world, "go")
|
||||
item = the_item(world)
|
||||
assert loop_of(item).wait == MERGE, (item.position, loop_of(item), said(transport))
|
||||
return agent, transport
|
||||
|
||||
|
||||
def said(transport: InMemoryTransport) -> list[str]:
|
||||
return [m.text for m in transport.outbound]
|
||||
|
||||
|
||||
def test_an_approved_pr_is_merged_and_the_item_goes_on(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
agent, transport = to_review(world, "approved", CLOSE)
|
||||
pr = loop_of(the_item(world)).pr
|
||||
agent.watch()
|
||||
assert forge.agent_merged == [] # not approved yet
|
||||
forge.approve(pr)
|
||||
agent.watch()
|
||||
agent._schedule()
|
||||
assert forge.agent_merged == [pr]
|
||||
assert f"Merged PR #{pr} after don's approval." in said(transport)
|
||||
assert the_item(world).status == "closed"
|
||||
|
||||
|
||||
def test_the_operator_merges_by_default(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
agent, _ = to_review(world, "operator")
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr)
|
||||
agent.watch()
|
||||
assert forge.agent_merged == []
|
||||
assert loop_of(the_item(world)).wait == MERGE
|
||||
|
||||
|
||||
def test_an_approval_of_an_older_commit_doesnt_count(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
agent, _ = to_review(world)
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr, sha="0" * 40) # what the PR was before a fix
|
||||
agent.watch()
|
||||
assert forge.agent_merged == []
|
||||
|
||||
|
||||
def test_only_an_operators_approval_counts(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
agent, _ = to_review(world)
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr, by="mallory")
|
||||
agent.watch()
|
||||
assert forge.agent_merged == []
|
||||
|
||||
|
||||
def test_pending_checks_wait_and_failed_ones_are_said_once(
|
||||
world: dict[str, Any],
|
||||
) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
agent, transport = to_review(world)
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr)
|
||||
forge.status[forge.head_sha(pr)] = "pending"
|
||||
agent.watch()
|
||||
assert forge.agent_merged == []
|
||||
forge.status[forge.head_sha(pr)] = "failure"
|
||||
agent.watch()
|
||||
agent.watch()
|
||||
held = [t for t in said(transport) if "its checks failed" in t]
|
||||
assert len(held) == 1 and forge.agent_merged == []
|
||||
forge.status[forge.head_sha(pr)] = "success"
|
||||
agent.watch()
|
||||
assert forge.agent_merged == [pr]
|
||||
|
||||
|
||||
def test_protection_is_checked_at_start(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"]
|
||||
forge.protect()
|
||||
agent, transport = agent_for(world, ScriptedModel())
|
||||
agent.boot()
|
||||
assert agent._merging == {"sandbox": True}
|
||||
# The check's branch and PR are gone again.
|
||||
assert all(p["state"] == "closed" for p in forge.prs.values())
|
||||
assert forge.agent_merged == []
|
||||
assert not [t for t in said(transport) if "leave merging to you" in t]
|
||||
|
||||
|
||||
def test_unprotected_means_the_operator_merges(world: dict[str, Any]) -> None:
|
||||
forge: Forge = world["forge"] # not protected: the agent could merge anything
|
||||
model = ScriptedModel(RESTATE, [*WORK, *FINISH])
|
||||
agent, transport = agent_for(world, model)
|
||||
agent.boot()
|
||||
assert agent._merging == {"sandbox": False}
|
||||
assert any("I could merge my own PR" in t for t in said(transport))
|
||||
agent.intake()
|
||||
say(transport, world, "go")
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr)
|
||||
agent.watch()
|
||||
assert pr not in forge.agent_merged # left to the operator
|
||||
|
||||
|
||||
def test_a_refused_merge_is_said_once_and_left_to_the_operator(
|
||||
world: dict[str, Any],
|
||||
) -> None:
|
||||
from nisse_dev.gitea import GiteaError
|
||||
|
||||
forge: Forge = world["forge"]
|
||||
agent, transport = to_review(world)
|
||||
pr = loop_of(the_item(world)).pr
|
||||
forge.approve(pr)
|
||||
|
||||
def refuse(repo: str, index: int) -> None:
|
||||
raise GiteaError("gitea POST merge returned 405: not mergeable")
|
||||
|
||||
forge.merge_pull_request = refuse # type: ignore[method-assign]
|
||||
agent.watch()
|
||||
agent.watch()
|
||||
refused = [t for t in said(transport) if "Gitea refused my merge" in t]
|
||||
assert len(refused) == 1
|
||||
assert loop_of(the_item(world)).wait == MERGE
|
||||
Reference in new issue
Block a user