diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index c0104c1..f950a28 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/nisse-dev/src/nisse_dev/control.py @@ -99,7 +99,9 @@ def _fresh_data() -> dict[str, Any]: return { "revs": {}, "gate": None, - "question": None, + # What an operator reply in AWAIT_INPUT should resume. Every path into + # AWAIT_INPUT must set this, or the reply has nowhere to go. + "pending": None, "milestone": 1, "task_id": None, "roles": None, # A11 mapping, pinned at task start @@ -138,11 +140,14 @@ class Orchestrator: self._store.oplog.task_id = self._data.get("task_id") gate = self._data.get("gate") if report.action == "await_input": - self._transition(AWAIT_INPUT, cause="boot: unresolved intents") - self._transport.ask_question( - "restart found unresolved work: " - + "; ".join(report.notes) - + " — how should I proceed?" + self._await_input( + cause="boot: unresolved intents", + prompt=( + "restart found unresolved work: " + + "; ".join(report.notes) + + " — how should I proceed?" + ), + pending={"kind": "boot"}, ) elif state.state == IDLE or ( state.state in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF) and gate @@ -157,9 +162,10 @@ class Orchestrator: else: # Interrupted mid-turn (planning, executing, unanswered question): # never blind-resume (A3.6). - self._transition(AWAIT_INPUT, cause=f"boot: restarted in {state.state}") - self._transport.ask_question( - f"restarted mid-{state.state}; continue or re-plan?" + self._await_input( + cause=f"boot: restarted in {state.state}", + prompt=f"restarted mid-{state.state}; continue or re-plan?", + pending={"kind": "boot"}, ) def _handle(self, msg: InboundMessage) -> None: @@ -217,17 +223,18 @@ class Orchestrator: gate_name, self._project, feedback=feedback, answer=answer ) except PlanningError as exc: - self._transition(AWAIT_INPUT, cause=f"planning failed: {exc}") - self._transport.ask_question( - f"planning for {gate_name} failed ({exc}); retry or re-plan?" + self._await_input( + cause=f"planning failed: {exc}", + prompt=f"planning for {gate_name} failed ({exc}); how should I proceed?", + pending={"kind": "planning_failed", "gate": gate_name}, ) return if isinstance(action, AskHuman): - self._data["question"] = {"text": action.question, "gate": gate_name} - self._transition( - AWAIT_INPUT, cause=f"agent question during {gate_name} planning" + self._await_input( + cause=f"agent question during {gate_name} planning", + prompt=action.question, + pending={"kind": "question", "gate": gate_name}, ) - self._transport.ask_question(action.question) return rev = self._data["revs"].get(gate_name, 0) + 1 self._data["revs"][gate_name] = rev @@ -334,23 +341,48 @@ class Orchestrator: # until then a finished milestone simply reports and waits. self._transport.send_message(f"milestone {milestone} done: {outcome.note}") return - self._transition(AWAIT_INPUT, cause=f"milestone {milestone} {outcome.status}") - self._transport.ask_question( - f"milestone {milestone} stopped ({outcome.status}): {outcome.note}" + self._await_input( + cause=f"milestone {milestone} {outcome.status}", + prompt=f"milestone {milestone} stopped ({outcome.status}): {outcome.note}", + pending={"kind": "milestone_stopped", "milestone": milestone}, ) + def _await_input(self, *, cause: str, prompt: str, pending: dict[str, Any]) -> None: + self._data["pending"] = pending + self._transition(AWAIT_INPUT, cause=cause) + self._transport.ask_question(prompt) + def _handle_answer(self, text: str) -> None: - question = self._data.get("question") - if question is None: + pending = self._data.get("pending") + if pending is None: + # Reached only if state was forced externally; say what works + # rather than dead-ending the operator. self._transport.send_message( - "awaiting operator resolution; no pending question to answer" + "waiting for direction, but I have no pending item to resume; " + "try 'status', or 'use ' to start over" ) return - self._data["question"] = None - gate_name = question["gate"] - back = PLANNING_ARCH if gate_name == "arch" else MILESTONE_PLANNING - self._transition(back, cause="operator answered question") - self._run_planning_turn(gate_name, answer=text) + self._data["pending"] = None + match pending.get("kind"): + case "question" | "planning_failed": + gate_name = pending["gate"] + back = PLANNING_ARCH if gate_name == "arch" else MILESTONE_PLANNING + self._transition(back, cause="operator answered; resuming planning") + self._run_planning_turn(gate_name, answer=text) + case "milestone_stopped": + milestone = pending["milestone"] + self._transition( + AWAIT_MILESTONE_SIGNOFF, + cause="operator answered; plan still approved", + ) + self._transport.send_message( + f"noted. the milestone {milestone} plan is still approved — " + f"say 'start milestone {milestone}' to run it again" + ) + case _: + self._transport.send_message( + "noted. say 'status' for where things stand" + ) def _transition(self, new_state: str, *, cause: str) -> None: state = self._store.get_state() diff --git a/nisse-dev/tests/test_planner.py b/nisse-dev/tests/test_planner.py index bef25e0..9dcce14 100644 --- a/nisse-dev/tests/test_planner.py +++ b/nisse-dev/tests/test_planner.py @@ -12,7 +12,14 @@ from nisse_dev.canonical import ( ToolCall, UserMessage, ) -from nisse_dev.control import AskHuman, Orchestrator, PlanningError, SubmitPlan +from nisse_dev.control import ( + AWAIT_ARCH_SIGNOFF, + AWAIT_MILESTONE_SIGNOFF, + AskHuman, + Orchestrator, + PlanningError, + SubmitPlan, +) from nisse_dev.llm import RoleMap from nisse_dev.oplog import OpLog from nisse_dev.planner import PLANNER_TOOLS, LLMPlanningAgent @@ -212,6 +219,53 @@ def test_planning_failure_gates_to_await_input(tmp_path: Path) -> None: assert transport.outbound[-1].kind == "question" +def test_operator_can_recover_from_a_planning_failure(tmp_path: Path) -> None: + # Regression: AWAIT_INPUT reached by any path other than ask_human used to + # record no pending item, so every operator reply was refused with "no + # pending question" and the agent was stuck for good. + prose = [AssistantText(text="prose", model="m")] + client = ScriptedClient([prose, prose, [plan_call("recovered plan")]]) + _, store, transport = build(tmp_path, client) + transport.receive("don", "use demo") + assert store.get_state().state == "AWAIT_INPUT" + + transport.receive("don", "try again, keep it short") + + assert store.get_state().state == AWAIT_ARCH_SIGNOFF + assert "recovered plan" in transport.outbound[-1].text + + +def test_operator_can_recover_after_a_stopped_milestone(tmp_path: Path) -> None: + class StoppedExecutor: + def run(self, milestone: int) -> Any: + from nisse_dev.executor import ExecutorOutcome + + return ExecutorOutcome("escalate", "tests failed twice") + + store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) + transport = InMemoryTransport() + client = ScriptedClient([[plan_call("p1")], [plan_call("p2")]]) + orch = Orchestrator( + store, + transport, + LLMPlanningAgent(client, store, ROLES), + operators={"don"}, + roles=ROLES, + executor=StoppedExecutor(), + ) + orch.start() + transport.receive("don", "use demo") + transport.receive("don", "approve") + transport.receive("don", "approve") + transport.receive("don", "start milestone 1") + assert store.get_state().state == "AWAIT_INPUT" + + transport.receive("don", "ok, I fixed the dependency") + + assert store.get_state().state == AWAIT_MILESTONE_SIGNOFF + assert "start milestone 1" in transport.outbound[-1].text + + def test_rejection_feedback_reaches_the_planner_via_the_log( tmp_path: Path, ) -> None: diff --git a/tests/integration/test_milestone_live.py b/tests/integration/test_milestone_live.py index c0db593..8a8ea62 100644 --- a/tests/integration/test_milestone_live.py +++ b/tests/integration/test_milestone_live.py @@ -12,7 +12,13 @@ from pathlib import Path import pytest -from nisse_dev.control import EXECUTING, Orchestrator +from nisse_dev.control import ( + AWAIT_ARCH_SIGNOFF, + AWAIT_INPUT, + AWAIT_MILESTONE_SIGNOFF, + EXECUTING, + Orchestrator, +) from nisse_dev.envfile import load_env_file from nisse_dev.executor import LLMExecutor from nisse_dev.llm import LiteLLMClient, RoleMap @@ -35,6 +41,32 @@ pytestmark = [ ] +def _drive_to_execution( + store: StateStore, transport: InMemoryTransport, max_steps: int = 10 +) -> None: + """Play the operator. A scripted approve/approve/start sequence is not + robust against a live model: ask_human is a legitimate move at any gate, + and a question shifts the state machine under a fixed script. So respond + to whatever the agent actually did.""" + for _ in range(max_steps): + state = store.get_state() + assert state is not None + if state.state == EXECUTING: + return + gate = state.data.get("gate") or {} + if state.state == AWAIT_INPUT: + transport.receive("don", "Use your best judgement and proceed.") + elif state.state == AWAIT_MILESTONE_SIGNOFF and gate.get("approved"): + transport.receive("don", "start milestone 1") + elif state.state in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF): + transport.receive("don", "approve") + else: + raise AssertionError(f"unexpected state {state.state}") + raise AssertionError( + f"never reached execution; last message: {transport.outbound[-1]}" + ) + + def test_plan_approve_execute_produces_a_real_file(tmp_path: Path) -> None: assert os.environ.get("NISSE_LLM_TOKEN"), ( f"{SENTINEL} is set, so the live path is expected to work, " @@ -63,13 +95,11 @@ def test_plan_approve_execute_produces_a_real_file(tmp_path: Path) -> None: "use greet. Milestone 1: create greet.py in the workspace root defining " "greet(name) returning 'Hello, !', and verify it by running python.", ) - transport.receive("don", "approve") # architecture gate - transport.receive("don", "approve") # milestone gate - transport.receive("don", "start milestone 1") + _drive_to_execution(store, transport) state = store.get_state() assert state is not None - assert state.state in (EXECUTING, "AWAIT_INPUT") + assert state.state in (EXECUTING, AWAIT_INPUT) created = list(workspace.root.rglob("*.py")) assert created, f"executor created no Python file; said: {transport.outbound[-1]}"