Merge pull request 'Wire delegation to A12's trigger, and implement A3's resume rule' (#25) from delegation-resume into main

Reviewed-on: dongar/nisse#25
Reviewed-by: Don Garrett <dgarrett@acm.org>

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 {})

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

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}"

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."""

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]