diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index 92695ce..8e21c99 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/nisse-dev/src/nisse_dev/control.py @@ -743,7 +743,16 @@ class Orchestrator: ) return if result.pull_request is None: - self._transport.send_message(result.note) + # Nothing left EXECUTING behind: the operator decides what next, + # and answering returns to the still-approved plan. + self._await_input( + cause=f"milestone {milestone}: {result.note}", + prompt=( + f"milestone {milestone}: {result.note}. Reply to return to " + "the approved plan." + ), + pending={"kind": "milestone_stopped", "milestone": milestone}, + ) return self._data["pr"] = { "number": result.pull_request.number, diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 5421ab7..7db2ef9 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -53,6 +53,19 @@ class GitRepo: def has_changes(self) -> bool: return bool(self._run("status", "--porcelain")) + def commits_since(self, ref: str) -> int: + """How many commits HEAD has that ref lacks. The executor may commit + its own work, so a clean tree does not mean nothing to publish.""" + return int(self._run("rev-list", "--count", f"{ref}..HEAD")) + + def remote_sha( + self, remote_url: str, branch: str, *, token: str | None = None + ) -> str | None: + """What the remote's branch points at, or None if it has no such + branch. Pushes go by URL, so there is no tracking ref to read.""" + out = self._run("ls-remote", remote_url, f"refs/heads/{branch}", secret=token) + return out.split()[0] if out else None + # -- mutation --------------------------------------------------------- def init(self, *, default_branch: str = "main") -> None: diff --git a/nisse-dev/src/nisse_dev/publish.py b/nisse-dev/src/nisse_dev/publish.py index 19060af..c47152e 100644 --- a/nisse-dev/src/nisse_dev/publish.py +++ b/nisse-dev/src/nisse_dev/publish.py @@ -71,9 +71,12 @@ class MilestonePublisher: self._git.checkout_new_branch(branch) sha = self._git.commit_all(f"Milestone {milestone}: {project}") if sha is None: - # Not an error: the executor may have concluded there was nothing - # to change. Report it and let the operator decide. - return PublishResult(None, "no changes to publish") + if self._git.commits_since(f"origin/{self._base_branch}") == 0: + # Not an error: the executor may have concluded there was + # nothing to change. Report it and let the operator decide. + return PublishResult(None, "no changes to publish") + # The executor committed its work itself; publish that. + sha = self._git.head_sha() intent_id = self._store.begin_intent( OPEN_PR_INTENT, @@ -113,7 +116,11 @@ class MilestonePublisher: self._git.checkout_new_branch(branch) sha = self._git.commit_all(message) if sha is None: - return None + # The executor may have committed the fix itself. + sha = self._git.head_sha() + pushed = self._git.remote_sha(self._remote, branch, token=self._token) + if sha == pushed: + return None self._git.push(self._remote, branch, token=self._token) return sha diff --git a/nisse-dev/tests/test_git.py b/nisse-dev/tests/test_git.py index d0a6a03..30f2d6f 100644 --- a/nisse-dev/tests/test_git.py +++ b/nisse-dev/tests/test_git.py @@ -68,3 +68,20 @@ def test_git_error_on_bad_command(tmp_path: Path) -> None: repo.root.mkdir() with pytest.raises(GitError): repo.head_sha() + + +def test_commits_since_and_remote_sha_against_a_local_remote( + repo: GitRepo, tmp_path: Path +) -> None: + remote = GitRepo(tmp_path / "remote.git") + remote.root.mkdir() + remote._run("init", "-q", "--bare") + (repo.root / "a.txt").write_text("a") + base = repo.commit_all("base") + repo.push(str(remote.root), "main") + repo._run("update-ref", "refs/remotes/origin/main", "HEAD") + assert repo.remote_sha(str(remote.root), "main") == base + assert repo.remote_sha(str(remote.root), "absent") is None + (repo.root / "b.txt").write_text("b") + repo.commit_all("ahead") + assert repo.commits_since("origin/main") == 1 diff --git a/nisse-dev/tests/test_pr_open.py b/nisse-dev/tests/test_pr_open.py index 0233c4f..63a1718 100644 --- a/nisse-dev/tests/test_pr_open.py +++ b/nisse-dev/tests/test_pr_open.py @@ -10,7 +10,7 @@ import pytest from nisse_dev.canonical import AssistantText, CanonicalEvent, StoredEvent, ToolCall from nisse_dev.control import ( AWAIT_INPUT, - EXECUTING, + AWAIT_MILESTONE_SIGNOFF, IDLE, PR_OPEN, Orchestrator, @@ -153,11 +153,17 @@ def test_unknown_command_in_pr_open_explains_the_options(tmp_path: Path) -> None assert "next milestone" in transport.outbound[-1].text -def test_no_changes_reports_without_entering_pr_open(tmp_path: Path) -> None: +def test_no_changes_gates_to_the_operator_without_entering_pr_open( + tmp_path: Path, +) -> None: publisher = FakePublisher(result=PublishResult(None, "no changes to publish")) store, transport = run_to_pr(tmp_path, publisher) - assert store.get_state().state == EXECUTING - assert transport.outbound[-1].text == "no changes to publish" + # Not left in EXECUTING, where every message is answered "busy". + assert store.get_state().state == AWAIT_INPUT + assert "no changes to publish" in transport.outbound[-1].text + transport.receive("don", "ok") + assert store.get_state().state == AWAIT_MILESTONE_SIGNOFF + assert "start milestone 1" in transport.outbound[-1].text def test_publish_failure_gates_to_the_operator(tmp_path: Path) -> None: diff --git a/nisse-dev/tests/test_publish.py b/nisse-dev/tests/test_publish.py index 57baf67..b01dc0c 100644 --- a/nisse-dev/tests/test_publish.py +++ b/nisse-dev/tests/test_publish.py @@ -1,4 +1,5 @@ import io +import subprocess from pathlib import Path from typing import Any @@ -37,15 +38,36 @@ class FakeGitea: class FakeGit: - """Records what would have been pushed; no network.""" + """A real repository whose origin/main is a local ref; pushes are + recorded, not sent.""" - def __init__(self, root: Path, *, changes: bool = True) -> None: + def __init__( + self, root: Path, *, changes: bool = True, committed: bool = False + ) -> None: self.repo = GitRepo(root) self.repo.init() self.repo.configure_identity("bot", "bot@example.test") + (root / "README.md").write_text("sandbox") + self.repo.commit_all("Initial commit") + git(root, "update-ref", "refs/remotes/origin/main", "HEAD") if changes: (root / "work.txt").write_text("done") + if committed: + # The executor committing its own work, as on the first live run. + self.repo.commit_all("Add the work") self.pushed: list[str] = [] + self.remote: dict[str, str] = {} + + def head_sha(self) -> str: + return self.repo.head_sha() + + def commits_since(self, ref: str) -> int: + return self.repo.commits_since(ref) + + def remote_sha( + self, remote_url: str, branch: str, *, token: str | None = None + ) -> str | None: + return self.remote.get(branch) def current_branch(self) -> str: return self.repo.current_branch() @@ -58,6 +80,11 @@ class FakeGit: def push(self, remote_url: str, branch: str, *, token: str | None = None) -> None: self.pushed.append(branch) + self.remote[branch] = self.repo.head_sha() + + +def git(root: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=root, check=True, capture_output=True) @pytest.fixture @@ -156,3 +183,32 @@ def test_reconciler_settles_an_intent_left_by_a_crash( store.commit_intent(intent_id, cause="verified") assert client.opened == [] # nothing re-published + + +def test_work_the_executor_committed_itself_is_published( + store: StateStore, tmp_path: Path +) -> None: + git = FakeGit(tmp_path / "work", committed=True) + client = FakeGitea() + result = build(store, git, client).publish( + task_id="t", milestone=1, project="demo", plan="p" + ) + assert result.pull_request is not None + assert git.pushed == ["t/milestone-1"] + assert git.remote["t/milestone-1"] == git.head_sha() + + +def test_a_followup_the_executor_committed_itself_is_pushed( + store: StateStore, tmp_path: Path +) -> None: + git = FakeGit(tmp_path / "work") + publisher = build(store, git, FakeGitea()) + publisher.publish(task_id="t", milestone=1, project="demo", plan="p") + (tmp_path / "work" / "work.txt").write_text("fixed") + git.repo.commit_all("Address review") + sha = publisher.push_followup(task_id="t", milestone=1, message="m") + assert sha == git.head_sha() + assert git.pushed == ["t/milestone-1", "t/milestone-1"] + # Nothing new since that push: no second push, and it says so. + assert publisher.push_followup(task_id="t", milestone=1, message="m") is None + assert len(git.pushed) == 2