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

Merged
dev-agent merged 3 commits from executor-loop into main 2026-09-07 20:35:33 +00:00
Collaborator

Completes build-order step 5. The agent now does real work end to end: plan, gate, approve, gate, execute — and a new live test proves it by having the model write and verify an actual file on disk.

Sandboxing: none, deliberately

Per your call, this matches the final system rather than exceeding it. A4 makes tool use auto-approved inside an approved unit of work and B3 puts the trust boundary at the VM, so per-command path confinement would only add failure modes. The workspace is where work happens — cwd and the default for relative paths — not a wall, and the module docstring says so plainly. Protecting agent state from the agent stays a mount-layout concern (A2/B3).

What is enforced is operational, not security:

  • Timeout with process-group kill. A backgrounded dev server must not outlive its command. This exposed a bug in my first version: subprocess.run's timeout kills only the direct child, and TimeoutExpired carries no pid, so my group-kill silently no-opped. Now it uses Popen, and a test verifies the grandchild is actually dead.
  • Output truncation, so a build log cannot eat the context window or the canonical log.
  • stdin closed, so a command that waits for input fails fast instead of burning the timeout.
  • Scrubbed environment: project build and test code is arbitrary and never sees NISSE_* credentials.

Executor loop

Renders the canonical log, calls the executor role with the four-tool flat manifest (A15), dispatches calls, and checkpoints every result as it happens (A13). Escalation is observable rather than self-assessed (A12): consecutive run_command failures, plus a hard turn cap. Failed reads deliberately don't count — the model looking around is not the model failing. Until delegation exists (step 7), escalation gates to the human with the failures attached.

Two bugs found only by running it live

  1. Every planning turn after the first failed with a 400. submit_plan ends the turn by design, so it left a tool_call with no tool_result — which Anthropic rejects outright: "tool_use ids were found without tool_result blocks". The planner now records the gate as that call's outcome, which is also the more truthful transcript.
  2. The renderer now emits a placeholder for any tool_call lacking a result, so a crash between a call and its result — exactly the case A3's resume rule contemplates — can't permanently poison the transcript.

Both have regression tests. Neither was reachable from unit tests alone, which is a decent argument for the live tests earning their keep.

Known limitation

The executor runs synchronously inside the transport callback, so status won't answer while a milestone is in flight. Fixing that needs a worker thread and belongs with step 9's unattended milestones; flagging it rather than smuggling it in here.

24 new unit tests. 117 unit + 9 integration green locally, including all four live paths.

🤖 Generated with Claude Code

Completes build-order step 5. **The agent now does real work end to end**: plan, gate, approve, gate, execute — and a new live test proves it by having the model write and verify an actual file on disk. ### Sandboxing: none, deliberately Per your call, this matches the final system rather than exceeding it. A4 makes tool use auto-approved inside an approved unit of work and B3 puts the trust boundary at the VM, so per-command path confinement would only add failure modes. The workspace is *where work happens* — cwd and the default for relative paths — not a wall, and the module docstring says so plainly. Protecting agent state from the agent stays a mount-layout concern (A2/B3). What **is** enforced is operational, not security: - **Timeout with process-group kill.** A backgrounded dev server must not outlive its command. This exposed a bug in my first version: `subprocess.run`'s timeout kills only the direct child, and `TimeoutExpired` carries no pid, so my group-kill silently no-opped. Now it uses `Popen`, and a test verifies the grandchild is actually dead. - **Output truncation**, so a build log cannot eat the context window or the canonical log. - **stdin closed**, so a command that waits for input fails fast instead of burning the timeout. - **Scrubbed environment**: project build and test code is arbitrary and never sees `NISSE_*` credentials. ### Executor loop Renders the canonical log, calls the **executor role** with the four-tool flat manifest (A15), dispatches calls, and checkpoints every result as it happens (A13). Escalation is observable rather than self-assessed (A12): consecutive `run_command` failures, plus a hard turn cap. Failed *reads* deliberately don't count — the model looking around is not the model failing. Until delegation exists (step 7), escalation gates to the human with the failures attached. ### Two bugs found only by running it live 1. **Every planning turn after the first failed with a 400.** `submit_plan` ends the turn by design, so it left a `tool_call` with no `tool_result` — which Anthropic rejects outright: *"`tool_use` ids were found without `tool_result` blocks"*. The planner now records the gate as that call's outcome, which is also the more truthful transcript. 2. **The renderer now emits a placeholder** for any `tool_call` lacking a result, so a crash between a call and its result — exactly the case A3's resume rule contemplates — can't permanently poison the transcript. Both have regression tests. Neither was reachable from unit tests alone, which is a decent argument for the live tests earning their keep. ### Known limitation The executor runs synchronously inside the transport callback, so `status` won't answer while a milestone is in flight. Fixing that needs a worker thread and belongs with step 9's unattended milestones; flagging it rather than smuggling it in here. 24 new unit tests. 117 unit + 9 integration green locally, including all four live paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
dev-agent added 1 commit 2026-09-07 16:35:55 +00:00
Add executor tool loop (A12/A15) — step 5b
CI / checks (push) Failing after 13s
CI / checks (pull_request) Failing after 13s
0fd0b2e4a5
- 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>
dev-agent added 1 commit 2026-09-07 16:37:40 +00:00
Fix process-group kill test to work without an init process
CI / checks (push) Failing after 23s
CI / checks (pull_request) Successful in 30s
999a38a705
os.kill(pid, 0) succeeds on a zombie, and a killed orphan stays a
zombie until reaped — which never happens inside a CI container with
no init. The test read that as "still alive" and failed there while
passing locally. It now reads the process state from /proc, with a
control assertion so a broken liveness check cannot pass silently.
The group kill itself was correct.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dev-agent added 1 commit 2026-09-07 16:46:09 +00:00
Fix AWAIT_INPUT deadlock and make the live milestone test robust
CI / checks (push) Successful in 36s
CI / checks (pull_request) Successful in 29s
bacf65dbe2
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>
dongar approved these changes 2026-09-07 20:33:48 +00:00
dev-agent merged commit 4025ac7875 into main 2026-09-07 20:35:33 +00:00
dev-agent deleted branch executor-loop 2026-09-07 20:35:33 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dev/nisse#12