Fix AWAIT_INPUT deadlock and make the live milestone test robust
Found while chasing a flaky live test, but the flakiness was a real bug: only the ask_human path recorded a pending item, so AWAIT_INPUT reached any other way — planning failure, stopped milestone, boot reconciliation — refused every operator reply with "no pending question to answer" and the agent could never be recovered. The operator had no way back short of wiping state. Every path into AWAIT_INPUT now records what a reply should resume, and answering dispatches on it: planning failures and questions resume planning, a stopped milestone returns to its still-approved gate, and the fallback says what commands work instead of dead-ending. Two regression tests cover the recovery paths. The live test also scripted a fixed approve/approve/start sequence, which a real model breaks by legitimately calling ask_human at a gate. It now plays the operator adaptively, responding to whatever the agent actually did. Five consecutive live runs pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
999a38a705
commit
bacf65dbe2
3 files changed
+149
-33
No files matched your search
@@ -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 <project>' 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()
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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, <name>!', 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]}"
|
||||
|
||||
Reference in new issue
Block a user