Add executor tool loop (A12/A15) — step 5b

- tools.py: four flat tools (read_file, write_file, list_dir,
  run_command) per A15's reduced manifest. Deliberately unsandboxed:
  the VM is the trust boundary (B3) and tool use is auto-approved
  inside an approved unit of work (A4), so path confinement would add
  failure modes without adding safety. What is enforced is
  operational — timeout with process-group kill so a backgrounded
  server cannot outlive its command, output truncation so a build log
  cannot eat the context window, closed stdin so prompts fail fast,
  and a scrubbed environment so project code never sees the agent's
  credentials
- executor.py: renders the log, calls the executor role with the
  reduced manifest, dispatches tool calls, and checkpoints every
  result (A13). Escalation is observable, never self-assessed (A12):
  consecutive command failures and a hard turn cap. Failed reads do
  not count — looking around is not failing
- Until delegation exists (step 7), escalation gates to the human
  with the failures attached
- control.py: start milestone now runs the executor; a stopped
  milestone gates to AWAIT_INPUT

Two bugs found by running it live against real models:

- A turn-ending gate tool (submit_plan) left a tool_call with no
  tool_result, which providers reject outright, so every planning
  turn after the first failed with a 400. The planner now records the
  gate as the call's outcome, which is also the truthful transcript
- The renderer now emits a placeholder for any tool_call lacking a
  result, so a crash between call and result (A3) cannot poison the
  transcript permanently

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nisse-dev/tests/test_executor.py

@@ -0,0 +1,173 @@
+import io
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+from nisse_dev.canonical import AssistantText, CanonicalEvent, StoredEvent, ToolCall
+from nisse_dev.executor import LLMExecutor
+from nisse_dev.llm import LLMError, RoleMap
+from nisse_dev.oplog import OpLog
+from nisse_dev.state import StateStore
+from nisse_dev.tools import Workspace
+
+ROLES = RoleMap(
+    executor="llama3.2",
+    planner="claude-haiku-4-5",
+    delegate="claude-haiku-4-5",
+    consultant="claude-haiku-4-5",
+)
+
+
+class ScriptedClient:
+    def __init__(self, turns: list[list[CanonicalEvent]]) -> None:
+        self._turns = turns
+        self.calls: list[dict[str, Any]] = []
+
+    def complete(
+        self,
+        events: list[StoredEvent],
+        tools: list[dict[str, Any]] | None,
+        model: str,
+        system: str | None = None,
+    ) -> list[CanonicalEvent]:
+        self.calls.append({"tools": tools, "model": model, "system": system})
+        if not self._turns:
+            return [AssistantText(text="done", model=model)]
+        return self._turns.pop(0)
+
+
+class FailingClient:
+    def complete(self, *args: Any, **kwargs: Any) -> list[CanonicalEvent]:
+        raise LLMError("proxy exploded")
+
+
+def call(tool: str, call_id: str = "c1", **arguments: Any) -> ToolCall:
+    return ToolCall(call_id=call_id, tool=tool, arguments=arguments)
+
+
+@pytest.fixture
+def store(tmp_path: Path) -> StateStore:
+    return StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()))
+
+
+@pytest.fixture
+def ws(tmp_path: Path) -> Workspace:
+    return Workspace(tmp_path / "work", timeout_s=10)
+
+
+def build(client: Any, store: StateStore, ws: Workspace, **kw: Any) -> LLMExecutor:
+    return LLMExecutor(client, store, ROLES, ws, **kw)
+
+
+def test_tool_calls_are_executed_and_results_logged(
+    store: StateStore, ws: Workspace
+) -> None:
+    client = ScriptedClient(
+        [
+            [call("write_file", path="app.py", content="print(1)\n")],
+            [AssistantText(text="wrote the file", model="llama3.2")],
+        ]
+    )
+    outcome = build(client, store, ws).run(milestone=1)
+
+    assert outcome.status == "completed"
+    assert (ws.root / "app.py").read_text() == "print(1)\n"
+    kinds = [type(e.event).__name__ for e in store.read_events()]
+    assert kinds == ["ToolCall", "ToolResult", "AssistantText"]
+
+
+def test_uses_executor_role_and_reduced_manifest(
+    store: StateStore, ws: Workspace
+) -> None:
+    client = ScriptedClient([])
+    build(client, store, ws).run(milestone=2)
+    assert client.calls[0]["model"] == "llama3.2"  # never the planner
+    names = [t["function"]["name"] for t in client.calls[0]["tools"]]
+    assert names == ["read_file", "write_file", "list_dir", "run_command"]
+    assert "milestone 2" in client.calls[0]["system"]
+
+
+def test_no_tool_call_ends_the_milestone(store: StateStore, ws: Workspace) -> None:
+    client = ScriptedClient([[AssistantText(text="all steps done", model="m")]])
+    assert build(client, store, ws).run(milestone=1).status == "completed"
+
+
+def test_two_consecutive_command_failures_escalate(
+    store: StateStore, ws: Workspace
+) -> None:
+    client = ScriptedClient(
+        [
+            [call("run_command", command="exit 1")],
+            [call("run_command", command="exit 1")],
+            [call("run_command", command="exit 1")],
+        ]
+    )
+    outcome = build(client, store, ws, failure_limit=2).run(milestone=1)
+    assert outcome.status == "escalate"
+    assert "2 consecutive command failures" in outcome.note
+    assert len(client.calls) == 2  # stopped, did not keep trying
+
+
+def test_a_success_resets_the_failure_count(store: StateStore, ws: Workspace) -> None:
+    client = ScriptedClient(
+        [
+            [call("run_command", command="exit 1")],
+            [call("run_command", command="true")],
+            [call("run_command", command="exit 1")],
+            [AssistantText(text="done", model="m")],
+        ]
+    )
+    assert build(client, store, ws, failure_limit=2).run(milestone=1).status == (
+        "completed"
+    )
+
+
+def test_failed_reads_do_not_count_toward_escalation(
+    store: StateStore, ws: Workspace
+) -> None:
+    client = ScriptedClient(
+        [
+            [call("read_file", path="missing-a.txt")],
+            [call("read_file", path="missing-b.txt")],
+            [AssistantText(text="looked around, done", model="m")],
+        ]
+    )
+    assert build(client, store, ws, failure_limit=2).run(milestone=1).status == (
+        "completed"
+    )
+
+
+def test_turn_cap_is_enforced(store: StateStore, ws: Workspace) -> None:
+    forever = [[call("list_dir", path=".")] for _ in range(50)]
+    client = ScriptedClient(forever)
+    outcome = build(client, store, ws, max_turns=3).run(milestone=1)
+    assert outcome.status == "capped"
+    assert "3-turn cap" in outcome.note
+    assert len(client.calls) == 3
+
+
+def test_llm_error_escalates_rather_than_crashing(
+    store: StateStore, ws: Workspace
+) -> None:
+    outcome = build(FailingClient(), store, ws).run(milestone=1)
+    assert outcome.status == "escalate"
+    assert "proxy exploded" in outcome.note
+
+
+def test_multiple_calls_in_one_turn_all_run(store: StateStore, ws: Workspace) -> None:
+    client = ScriptedClient(
+        [
+            [
+                call("write_file", call_id="a", path="one.txt", content="1"),
+                call("write_file", call_id="b", path="two.txt", content="2"),
+            ],
+            [AssistantText(text="done", model="m")],
+        ]
+    )
+    build(client, store, ws).run(milestone=1)
+    assert (ws.root / "one.txt").exists() and (ws.root / "two.txt").exists()
+    results = [
+        e.event for e in store.read_events() if type(e.event).__name__ == "ToolResult"
+    ]
+    assert {r.call_id for r in results} == {"a", "b"}

nisse-dev/tests/test_planner.py

@@ -108,7 +108,18 @@ def test_model_output_is_appended_to_the_log(store: StateStore) -> None:
     )
     LLMPlanningAgent(client, store, ROLES).plan("arch", "demo")
     kinds = [type(e.event).__name__ for e in store.read_events()]
-    assert kinds == ["AssistantText", "ToolCall"]
+    assert kinds == ["AssistantText", "ToolCall", "ToolResult"]
+
+
+def test_gate_tool_call_is_answered_in_the_log(store: StateStore) -> None:
+    # submit_plan ends the turn and has no natural result, but a tool call
+    # left dangling makes the next render invalid for the provider.
+    client = ScriptedClient([[plan_call("the plan")]])
+    LLMPlanningAgent(client, store, ROLES).plan("arch", "demo")
+    events = [e.event for e in store.read_events()]
+    assert [type(e).__name__ for e in events] == ["ToolCall", "ToolResult"]
+    assert events[1].call_id == events[0].call_id
+    assert "posted for approval" in events[1].result
 
 
 def test_prose_only_turn_retries_once_then_raises(store: StateStore) -> None:
@@ -170,7 +181,13 @@ def test_use_project_produces_llm_plan_and_transcript(tmp_path: Path) -> None:
 
     events = [e.event for e in store.read_events()]
     kinds = [type(e).__name__ for e in events]
-    assert kinds == ["UserMessage", "ToolCall", "PlanSubmitted", "GateOpened"]
+    assert kinds == [
+        "UserMessage",
+        "ToolCall",
+        "ToolResult",
+        "PlanSubmitted",
+        "GateOpened",
+    ]
     submitted = next(e for e in events if isinstance(e, PlanSubmitted))
     assert submitted.model == "anthropic/claude-opus-5"  # pinned planner
 

nisse-dev/tests/test_render.py

@@ -71,6 +71,33 @@ def test_compaction_replaces_its_range_only() -> None:
     assert contents == ["kept", "[summary of earlier conversation]\nthe old stuff"]
 
 
+def test_tool_call_without_a_result_gets_a_placeholder() -> None:
+    # Providers reject a dangling tool_use; this happens after a crash (A3)
+    # or with turn-ending gate tools like submit_plan.
+    events = stored(
+        [
+            UserMessage(text="go", source="m:1"),
+            ToolCall(call_id="call-x", tool="submit_plan", arguments={}),
+        ]
+    )
+    body = OpenAIChatRenderer().render(events, tools=None, model="q")
+    assert [m["role"] for m in body["messages"]] == ["user", "assistant", "tool"]
+    assert body["messages"][2]["tool_call_id"] == "call-x"
+    assert body["messages"][2]["content"] == "[no result recorded]"
+
+
+def test_answered_tool_call_gets_no_placeholder() -> None:
+    events = stored(
+        [
+            ToolCall(call_id="call-x", tool="run_command", arguments={}),
+            ToolResult(call_id="call-x", result="ok", success=True),
+        ]
+    )
+    body = OpenAIChatRenderer().render(events, tools=None, model="q")
+    assert [m["role"] for m in body["messages"]] == ["assistant", "tool"]
+    assert body["messages"][1]["content"] == "ok"
+
+
 def test_gate_event_renders_as_context() -> None:
     events = stored([GateOpened(gate="arch", reason="plan submitted (rev-1)")])
     body = OpenAIChatRenderer().render(events, tools=None, model="q")

nisse-dev/tests/test_tools.py

@@ -0,0 +1,117 @@
+import os
+from pathlib import Path
+
+import pytest
+
+from nisse_dev.tools import EXECUTOR_TOOLS, MAX_OUTPUT_CHARS, Workspace
+
+
+@pytest.fixture
+def ws(tmp_path: Path) -> Workspace:
+    return Workspace(tmp_path / "work", timeout_s=10)
+
+
+def test_manifest_is_small_and_flat() -> None:
+    # A15: fewer tools, flatter arguments for the local executor.
+    names = [t["function"]["name"] for t in EXECUTOR_TOOLS]
+    assert names == ["read_file", "write_file", "list_dir", "run_command"]
+    for tool in EXECUTOR_TOOLS:
+        props = tool["function"]["parameters"]["properties"]
+        assert all(p["type"] == "string" for p in props.values())
+
+
+def test_write_then_read_round_trip(ws: Workspace) -> None:
+    assert ws.write_file("src/app.py", "print('hi')\n").success
+    outcome = ws.read_file("src/app.py")
+    assert outcome.success
+    assert outcome.result == "print('hi')\n"
+
+
+def test_relative_paths_land_in_the_workspace(ws: Workspace) -> None:
+    ws.write_file("notes.txt", "x")
+    assert (ws.root / "notes.txt").exists()
+
+
+def test_read_missing_file_is_a_failed_result_not_an_exception(
+    ws: Workspace,
+) -> None:
+    outcome = ws.read_file("nope.txt")
+    assert not outcome.success
+    assert "could not read" in outcome.result
+
+
+def test_list_dir(ws: Workspace) -> None:
+    ws.write_file("a.txt", "a")
+    ws.write_file("sub/b.txt", "b")
+    outcome = ws.list_dir(".")
+    assert outcome.success
+    assert outcome.result.splitlines() == ["a.txt", "sub/"]
+
+
+def test_list_empty_dir(ws: Workspace) -> None:
+    assert ws.list_dir(".").result == "(empty)"
+
+
+def test_run_command_success_reports_exit_code_and_stdout(ws: Workspace) -> None:
+    outcome = ws.run_command("echo hello")
+    assert outcome.success
+    assert "exit code: 0" in outcome.result
+    assert "hello" in outcome.result
+
+
+def test_nonzero_exit_is_a_result_not_an_error(ws: Workspace) -> None:
+    outcome = ws.run_command("exit 3")
+    assert not outcome.success  # feeds the A12 failure count
+    assert "exit code: 3" in outcome.result
+
+
+def test_command_runs_in_the_workspace(ws: Workspace) -> None:
+    ws.write_file("marker.txt", "here")
+    assert "marker.txt" in ws.run_command("ls").result
+
+
+def test_command_timeout_is_reported(tmp_path: Path) -> None:
+    ws = Workspace(tmp_path / "work", timeout_s=1)
+    outcome = ws.run_command("sleep 30")
+    assert not outcome.success
+    assert "timed out" in outcome.result
+
+
+def test_timeout_kills_the_whole_process_group(tmp_path: Path) -> None:
+    ws = Workspace(tmp_path / "work", timeout_s=1)
+    pidfile = ws.root / "child.pid"
+    # A backgrounded grandchild must not outlive the timeout.
+    ws.run_command(f"(sleep 30 & echo $! > {pidfile}); sleep 30")
+    assert pidfile.exists()
+    pid = int(pidfile.read_text().strip())
+    with pytest.raises(ProcessLookupError):
+        os.kill(pid, 0)
+
+
+def test_agent_credentials_are_not_inherited(
+    ws: Workspace, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setenv("NISSE_LLM_TOKEN", "sk-secret")
+    monkeypatch.setenv("PATH", os.environ["PATH"])
+    outcome = ws.run_command("echo [${NISSE_LLM_TOKEN}]")
+    assert "sk-secret" not in outcome.result
+    assert "[]" in outcome.result
+
+
+def test_stdin_is_closed_so_prompts_fail_fast(ws: Workspace) -> None:
+    outcome = ws.run_command("read line && echo got")
+    assert not outcome.success
+    assert "got" not in outcome.result
+
+
+def test_long_output_is_truncated(ws: Workspace) -> None:
+    outcome = ws.run_command("printf 'x%.0s' $(seq 1 20000)")
+    assert len(outcome.result) < MAX_OUTPUT_CHARS + 200
+    assert "truncated" in outcome.result
+
+
+def test_dispatch_unknown_tool_and_missing_argument(ws: Workspace) -> None:
+    assert not ws.dispatch("fly", {}).success
+    missing = ws.dispatch("read_file", {})
+    assert not missing.success
+    assert "missing argument" in missing.result