From ce436262f4ae0912387005f30e9f0f6e167620e9 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Tue, 8 Sep 2026 20:03:46 +0000 Subject: [PATCH] Wire delegation to A12's trigger, and implement A3's resume rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps found by auditing spec against code, the second created by 7b landing the harness without its boot-time half. **The harness was not connected to anything.** Only a live test constructed a Delegation; A12's failure-count trigger still gated to the human, which A13 describes as the fallback rather than the path. Consecutive command failures now hand the sub-task to the delegate, with the failures in the brief so it need not rediscover what the local model learned the hard way. A usable handback lets the local loop carry on; anything else escalates, because a cap breach leaves a half-edited tree. **A3 step 5 was unimplemented.** A crash mid-delegation failed safe — boot gated on "restarted mid-EXECUTING" — but always gated, even on the clean cut the spec says should resume, and left the branch orphaned because _abandon only runs while the process lives. - A delegation now records an intent before the branch exists, and settles it whichever way the delegation ends; an aborted delegation is a finished one, and leaving it pending would make the next boot believe work was still in flight - delegation_verdict implements the clean-cut test as an observable check, never model self-assessment: settled if a completion follows the start, absent if the branch was never created, gate on a tool_call with no recorded result, resume otherwise - recover() sweeps at boot, announcing a resume with a notice rather than a question, since a clean cut needs no decision from the operator, and leaving an unclean one pending so the generic sweep gates on it - resume() continues the same delegation rather than starting a second: no new branch, no second delegation_started, because A10 defines a session as the delegation, not the process lifetime Honest limit, stated in the code: "the branch sits at the expected commit" is approximated by the branch existing and carrying the work, since per-turn shas are not recorded in the log, so a branch rewritten underneath us would go undetected. Co-Authored-By: Claude Opus 5 --- nisse-dev/src/nisse_dev/control.py | 7 + nisse-dev/src/nisse_dev/delegation.py | 147 +++++++++++++++++- nisse-dev/src/nisse_dev/executor.py | 50 ++++++- nisse-dev/src/nisse_dev/git.py | 10 ++ nisse-dev/tests/test_delegation.py | 205 ++++++++++++++++++++++++++ 5 files changed, 414 insertions(+), 5 deletions(-) diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index ff9ffd0..ccaa604 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/nisse-dev/src/nisse_dev/control.py @@ -163,6 +163,7 @@ class Orchestrator: publisher: "MilestonePublisher | None" = None, gitea_operators: Iterable[str] = (), board: "PlanBoard | None" = None, + delegate: Any = None, ) -> None: self._store = store self._transport = transport @@ -176,6 +177,7 @@ class Orchestrator: # the other. self._gitea_operators = set(gitea_operators) self._board = board + self._delegate = delegate # Transport and the comment poller deliver on separate threads; # state transitions must not interleave. self._lock = threading.RLock() @@ -184,6 +186,11 @@ class Orchestrator: transport.on_message(self._handle) def start(self, reconcilers: Mapping[str, Reconciler] | None = None) -> None: + # A3 step 5 runs before the generic sweep: an interrupted delegation is + # resumed or discarded by inspecting the branch and the log, and only a + # genuinely unclean cut is left for the sweep below to gate on. + if self._delegate is not None: + self._delegate.recover(self._transport.send_message) # The publisher owns the external-truth query for its own intents, so # a configured publisher always contributes its reconciler (A3). registered: dict[str, Reconciler] = dict(reconcilers or {}) diff --git a/nisse-dev/src/nisse_dev/delegation.py b/nisse-dev/src/nisse_dev/delegation.py index 1d67607..aba2bbe 100644 --- a/nisse-dev/src/nisse_dev/delegation.py +++ b/nisse-dev/src/nisse_dev/delegation.py @@ -37,7 +37,8 @@ from nisse_dev.canonical import ( ) from nisse_dev.git import GitError, GitRepo from nisse_dev.llm import LLMClient, LLMError -from nisse_dev.state import StateStore +from nisse_dev.reconcile import Reconciler +from nisse_dev.state import Intent, StateStore from nisse_dev.tools import EXECUTOR_TOOLS, MUTATING_TOOLS, Workspace DELEGATION_INTENT = "delegation" @@ -169,6 +170,19 @@ class Delegation: ) -> DelegationResult: started_at = self._now() base_branch = self._git.current_branch() + # Recorded before the branch exists, so a crash anywhere after this + # leaves a durable statement of what was in flight (A2/A3). + intent_id = self._store.begin_intent( + DELEGATION_INTENT, + { + "branch": branch, + "base": base_branch, + "brief": brief, + "role": role, + "caps": self._caps.as_payload(), + }, + cause=f"delegating to {self._model}", + ) try: # Commit whatever the caller had in flight onto their own branch # first. Otherwise it would ride along onto the delegation branch @@ -176,6 +190,7 @@ class Delegation: self._git.commit_all("Checkpoint before delegating") self._git.checkout_new_branch(branch) except GitError as exc: + self._store.abort_intent(intent_id, cause=f"could not branch: {exc}") return DelegationResult("failed", f"could not branch for delegation: {exc}") self._store.append_event( @@ -201,8 +216,77 @@ class Delegation: # Abort discards the branch: the caller must never resume on top # of a partially-edited tree (A13). self._abandon(branch, base_branch) + # Settled either way: an aborted delegation is a *finished* one, and + # leaving the intent pending would make the next boot think work was + # still in flight. + self._store.commit_intent(intent_id, cause=f"delegation {result.status}") return result + def resume(self, intent: Intent) -> DelegationResult: + """Continue a delegation the process died in the middle of (A3 step 5). + + The transcript is already in the canonical log, so the loop re-renders + it and carries on. No new branch and no second ``delegation_started``: + this is the same delegation, which is why its vendor-opaque reasoning + may still be replayed (A10 defines a session as the delegation, not the + process lifetime). + """ + payload = intent.payload + branch = str(payload["branch"]) + started_at = self._now() + try: + self._git.checkout_existing_branch(branch) + except GitError as exc: + self._store.commit_intent( + intent.id, cause=f"delegation branch unusable: {exc}" + ) + return DelegationResult("failed", f"could not resume on {branch}: {exc}") + + result = self._loop(str(payload.get("brief", "")), branch, started_at) + result.duration_s = self._now() - started_at + self._store.append_event( + DelegationCompleted( + handback=result.handback.render() if result.handback else result.note, + artifacts=result.artifacts, + tokens=result.tokens, + duration_s=result.duration_s, + exit_status=result.status, + ) + ) + if not result.usable: + self._abandon(branch, str(payload.get("base", "main"))) + self._store.commit_intent(intent.id, cause=f"delegation {result.status}") + return result + + def recover(self, notify: Callable[[str], None]) -> DelegationResult | None: + """Sweep pending delegation intents at boot (A3 step 5). + + Settles what is already finished, discards what never started, resumes + a clean cut — announcing it with a *notice*, not a question, because a + clean cut needs no decision from the operator — and leaves an unclean + one pending so the generic boot sweep gates on it. + """ + for intent in self._store.pending_intents(): + if intent.kind != DELEGATION_INTENT: + continue + verdict = delegation_verdict(self._store, self._git, intent) + branch = intent.payload.get("branch") + if verdict == "settled": + self._store.commit_intent( + intent.id, cause="delegation had already completed" + ) + elif verdict == "absent": + self._store.abort_intent( + intent.id, cause=f"delegation branch {branch} never created" + ) + elif verdict == "resume": + notify( + f"resuming the delegation interrupted on {branch}; " + "the transcript and branch are intact" + ) + return self.resume(intent) + return None + def _loop(self, brief: str, branch: str, started_at: float) -> DelegationResult: system = ( f"{SYSTEM_PROMPT}\n\n{self._workspace.describe()}" @@ -361,3 +445,64 @@ def _parse_handback(arguments: dict[str, Any]) -> Handback | None: next=str(arguments["next"]).strip(), succeeded=succeeded, ) + + +def delegation_reconciler(store: StateStore, git_repo: GitRepo) -> Reconciler: + """A3 step 5: decide whether an interrupted delegation may be resumed. + + The judgement is an observable check, never model self-assessment — the + same reasoning as A12. Returns: + + - ``True`` the delegation is settled or cleanly resumable + - ``False`` it never really started, so there is nothing to resume + - ``None`` the cut is unclean, so boot gates to the operator + + "The branch sits at the expected commit" is approximated by the branch + existing and carrying the delegation's commits: per-turn shas are not + recorded in the log today, so a branch rewritten out from under us would + not be detected. Stated rather than glossed, because the check is weaker + than A3's wording implies. + """ + + def check(intent: Intent) -> bool | None: + verdict = delegation_verdict(store, git_repo, intent) + if verdict in ("resume", "settled"): + return True + return False if verdict == "absent" else None + + return check + + +def delegation_verdict( + store: StateStore, git_repo: GitRepo, intent: Intent +) -> Literal["resume", "settled", "absent", "gate"]: + """The A3 step 5 decision, separated from the Reconciler contract because + boot must *act* on it — resuming is not the same as judging settled.""" + branch = intent.payload.get("branch") + if not branch: + return "gate" + if _completed_after_start(store): + return "settled" # it finished; the intent merely outlived its commit + if not git_repo.branch_exists(str(branch)): + return "absent" # the crash landed before any work existed + events = store.read_events() + if not events: + return "gate" + last = events[-1].event + if isinstance(last, ToolCall): + # A call with no recorded result: its effect is unknown and cannot be + # verified by inspection, so gate (A3). + return "gate" + if not isinstance(last, AssistantText | ToolResult): + return "gate" + return "resume" + + +def _completed_after_start(store: StateStore) -> bool: + """True when the last delegation event in the log is a completion.""" + for stored in reversed(store.read_events()): + if isinstance(stored.event, DelegationCompleted): + return True + if isinstance(stored.event, DelegationStarted): + return False + return False diff --git a/nisse-dev/src/nisse_dev/executor.py b/nisse-dev/src/nisse_dev/executor.py index 988b75b..e9bcefa 100644 --- a/nisse-dev/src/nisse_dev/executor.py +++ b/nisse-dev/src/nisse_dev/executor.py @@ -14,6 +14,7 @@ from dataclasses import dataclass from typing import Any, Literal from nisse_dev.canonical import ToolCall, ToolResult +from nisse_dev.delegation import Delegation from nisse_dev.expert import ASK_EXPERT_TOOL, BudgetExhausted, ExpertDesk from nisse_dev.llm import LLMClient, LLMError, RoleMap from nisse_dev.state import StateStore @@ -58,6 +59,7 @@ class LLMExecutor: max_turns: int = DEFAULT_MAX_TURNS, failure_limit: int = DEFAULT_FAILURE_LIMIT, expert: ExpertDesk | None = None, + delegate: Delegation | None = None, ) -> None: self._client = client self._store = store @@ -66,6 +68,39 @@ class LLMExecutor: self._max_turns = max_turns self._failure_limit = failure_limit self._expert = expert + self._delegate = delegate + + def _escalate( + self, milestone: int, failures: int, last_failure: str + ) -> ExecutorOutcome | None: + """Hand the sub-task to a stronger model, per A12. + + Returns None when the delegation handed back usable work and the local + loop should carry on; an ExecutorOutcome when the caller must stop. + """ + note = f"{failures} consecutive command failures; last: {last_failure[:500]}" + if self._delegate is None: + return ExecutorOutcome("escalate", note) + + # A12 says the failures go in the brief: the delegate must not have to + # rediscover what the local model already learned the hard way. + brief = ( + f"Milestone {milestone} is stuck. The local executor tried and its " + f"commands failed {failures} times in a row.\n\n" + f"Most recent failure:\n{last_failure[:2000]}\n\n" + "The approved plan and the full transcript are in the conversation " + "above. Finish this sub-task, staying inside the approved plan." + ) + result = self._delegate.run( + brief=brief, branch=f"delegation/milestone-{milestone}-{failures}" + ) + if result.usable: + return None + # A13: a cap breach or failure leaves a half-edited tree, so the local + # loop must not resume on top of it. + return ExecutorOutcome( + "escalate", f"{note}; delegation {result.status}: {result.note}" + ) def _tools(self) -> list[dict[str, Any]]: # A12: ask_expert exists in addition to the observable escalation @@ -152,11 +187,18 @@ class LLMExecutor: 0 if outcome.success else consecutive_failures + 1 ) if consecutive_failures >= self._failure_limit: - return ExecutorOutcome( - "escalate", - f"{consecutive_failures} consecutive command failures; " - f"last: {outcome.result[:500]}", + # A12: this is the trigger delegation exists for. + # Escalating to the human is the fallback when there + # is no delegate, not the intended path. + escalation = self._escalate( + milestone, + consecutive_failures, + outcome.result, ) + if escalation is not None: + return escalation + consecutive_failures = 0 + did_work = True return ExecutorOutcome( "capped", f"hit the {self._max_turns}-turn cap for milestone {milestone}" diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 95ac863..b947d99 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -66,6 +66,16 @@ class GitRepo: def checkout_new_branch(self, name: str) -> None: self._run("checkout", "-q", "-b", name) + def checkout_existing_branch(self, name: str) -> None: + self._run("checkout", "-q", name) + + def branch_exists(self, name: str) -> bool: + try: + self._run("rev-parse", "--verify", "--quiet", f"refs/heads/{name}") + except GitError: + return False + return True + def commit_all(self, message: str) -> str | None: """Returns the new commit sha, or None when there was nothing to commit — an empty milestone is a fact to report, not an error.""" diff --git a/nisse-dev/tests/test_delegation.py b/nisse-dev/tests/test_delegation.py index 27aec88..53dcdc8 100644 --- a/nisse-dev/tests/test_delegation.py +++ b/nisse-dev/tests/test_delegation.py @@ -11,11 +11,14 @@ from nisse_dev.canonical import ( DelegationStarted, StoredEvent, ToolCall, + ToolResult, ) from nisse_dev.delegation import ( DELEGATE_TOOLS, + DELEGATION_INTENT, Caps, Delegation, + delegation_reconciler, ) from nisse_dev.git import GitRepo from nisse_dev.llm import LLMError @@ -302,3 +305,205 @@ def test_failure_may_be_reported_without_doing_work( assert result.status == "failed" assert not result.usable assert len(client.calls) == 1 # believed immediately, not challenged + + +# --- A3 step 5: the resume rule ----------------------------------------- + + +def pending_delegation_intent(store: StateStore, branch: str = "d") -> Any: + store.begin_intent( + DELEGATION_INTENT, + {"branch": branch, "base": "main", "brief": "finish it"}, + cause="test", + ) + return store.pending_intents()[-1] + + +def test_intent_is_recorded_and_settled_by_a_completed_delegation( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + client = ScriptedClient( + [[call("write_file", path="a.py", content="x")], [hand_back()]] + ) + build(client, store, ws, repo).run(brief="b", branch="d") + # Nothing left in flight: an aborted delegation is a finished one too. + assert store.pending_intents() == [] + + +def test_aborted_delegation_also_settles_its_intent( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + client = ScriptedClient([[hand_back(succeeded=False, changed="cannot")]]) + build(client, store, ws, repo).run(brief="b", branch="d") + assert store.pending_intents() == [] + + +def test_clean_cut_after_a_tool_result_is_resumable( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + repo.checkout_new_branch("d") + (repo.root / "partial.py").write_text("x") + repo.commit_all("partial work") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolCall(call_id="c", tool="write_file", arguments={})) + store.append_event(ToolResult(call_id="c", result="wrote", success=True)) + intent = pending_delegation_intent(store) + + assert delegation_reconciler(store, repo)(intent) is True + + +def test_dangling_tool_call_gates( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # The effect of the call is unknown and not verifiable by inspection. + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolCall(call_id="c", tool="run_command", arguments={})) + intent = pending_delegation_intent(store) + + assert delegation_reconciler(store, repo)(intent) is None + + +def test_missing_branch_means_nothing_started( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolResult(call_id="c", result="ok", success=True)) + intent = pending_delegation_intent(store, branch="never-created") + + assert delegation_reconciler(store, repo)(intent) is False + + +def test_a_finished_delegation_settles_rather_than_resumes( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event( + DelegationCompleted( + handback="done", + artifacts=[], + tokens=0, + duration_s=1.0, + exit_status="handback", + ) + ) + intent = pending_delegation_intent(store) + + assert delegation_reconciler(store, repo)(intent) is True + + +def test_resume_continues_the_same_delegation( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # Interrupt after one turn, then resume: no second delegation_started, + # because A10 defines a session as the delegation, not the process. + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolResult(call_id="c", result="wrote a.py", success=True)) + intent = pending_delegation_intent(store) + + client = ScriptedClient( + [[call("write_file", path="b.py", content="x")], [hand_back()]] + ) + result = build(client, store, ws, repo).resume(intent) + + assert result.usable + starts = [e for e in store.read_events() if isinstance(e.event, DelegationStarted)] + assert len(starts) == 1 + assert store.pending_intents() == [] + assert (repo.root / "b.py").exists() + + +def test_resume_onto_a_missing_branch_fails_cleanly( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + intent = pending_delegation_intent(store, branch="gone") + result = build(ScriptedClient([]), store, ws, repo).resume(intent) + + assert result.status == "failed" + assert not result.usable + assert store.pending_intents() == [] # not left dangling for the next boot + + +# --- boot recovery sweep ------------------------------------------------ + + +def test_recover_resumes_a_clean_cut_with_a_notice( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolResult(call_id="c", result="wrote a.py", success=True)) + pending_delegation_intent(store) + notices: list[str] = [] + + client = ScriptedClient( + [[call("write_file", path="b.py", content="x")], [hand_back()]] + ) + result = build(client, store, ws, repo).recover(notices.append) + + assert result is not None and result.usable + # A3: a notice, not a question — a clean cut needs no operator decision. + assert notices and "resuming the delegation" in notices[0] + + +def test_recover_settles_a_finished_delegation_without_resuming( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event( + DelegationCompleted( + handback="done", + artifacts=[], + tokens=0, + duration_s=1.0, + exit_status="handback", + ) + ) + pending_delegation_intent(store) + client = ScriptedClient([]) + + assert build(client, store, ws, repo).recover(lambda _m: None) is None + assert store.pending_intents() == [] + assert client.calls == [] # the model was never asked + + +def test_recover_discards_an_intent_whose_branch_never_existed( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + store.append_event(ToolResult(call_id="c", result="ok", success=True)) + pending_delegation_intent(store, branch="never-made") + + assert build(ScriptedClient([]), store, ws, repo).recover(lambda _m: None) is None + assert store.pending_intents() == [] + + +def test_recover_leaves_an_unclean_cut_for_the_boot_gate( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + repo.checkout_new_branch("d") + store.append_event( + DelegationStarted(brief="b", role="delegate", model="m", caps={}) + ) + store.append_event(ToolCall(call_id="c", tool="run_command", arguments={})) + pending_delegation_intent(store) + + assert build(ScriptedClient([]), store, ws, repo).recover(lambda _m: None) is None + # Still pending, so reconcile_on_boot gates rather than blind-resuming. + assert [i.kind for i in store.pending_intents()] == [DELEGATION_INTENT]