From f97e459bc70fc88878322148dd0fa34efb68725e Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Tue, 29 Sep 2026 16:17:41 +0000 Subject: [PATCH] Publish work the executor committed itself, and gate when there is none On the first live run the executor committed its milestone itself, as its plan asked. The publisher only published a commit it made, found a clean tree, reported "no changes to publish", and left the agent in EXECUTING, where every message is answered "busy". - publish(): with nothing left to commit, publish anyway when the branch has commits origin/ lacks; report no changes only when it has none. - push_followup(): same for review fixes, comparing HEAD with what the remote branch holds (ls-remote; pushes go by URL, so there is no tracking ref). - Nothing to publish now gates to AWAIT_INPUT, and the reply returns to the still-approved milestone plan. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- nisse-dev/src/nisse_dev/control.py | 11 +++++- nisse-dev/src/nisse_dev/git.py | 13 +++++++ nisse-dev/src/nisse_dev/publish.py | 15 ++++++-- nisse-dev/tests/test_git.py | 17 +++++++++ nisse-dev/tests/test_pr_open.py | 14 +++++-- nisse-dev/tests/test_publish.py | 60 +++++++++++++++++++++++++++++- 6 files changed, 119 insertions(+), 11 deletions(-) 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