From 19fd2fd6fbb2bb9606c07fa478e78632a1f4fbb1 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 18:43:44 -0500 Subject: [PATCH 1/7] feat(tools): streaming tool-call execution foundation (phases A-C) Establishes the streaming tool-call contract and a per-call inactivity watchdog, and removes the batch tool-execution watchdog from the session actor. Foundation for replacing the flat sub-agent/tool timeout model; see openspec/changes/streaming-tool-call-execution. - ToolCallUpdate: non-terminal activity items plus one terminal completion item. - INetclawTool and IToolExecutor gain ExecuteStreamAsync as a default interface method, so the ~28 existing tools and the MCP/AI adapters inherit a single-completion-item default with no change. DispatchingToolExecutor surfaces the resolved tool's own stream. - StreamingToolWatchdog: a two-phase, TimeProvider-driven per-call inactivity watchdog. SessionToolExecutionPipeline consumes each tool call's stream under its own watchdog, replacing the flat per-attempt CancelAfter. Task.WhenAll parallel execution and per-tool failure isolation are preserved. - LlmSessionActor no longer arms the batch ToolExecution watchdog operation and drops the approval pause/resume of it; ProcessingWatchdog now governs only LLM calls and compaction. Phases D-G (spawn_agent streaming, recursion/approval cleanup, opt-in streaming tools, added tests) remain - see the change's tasks.md. Build clean; Netclaw.Actors.Tests 1609/1609 pass; slopwatch clean. --- .../.openspec.yaml | 2 + .../streaming-tool-call-execution/design.md | 237 ++++++++++++++++++ .../streaming-tool-call-execution/proposal.md | 124 +++++++++ .../specs/netclaw-session/spec.md | 36 +++ .../specs/netclaw-subagents/spec.md | 59 +++++ .../specs/netclaw-tools/spec.md | 91 +++++++ .../streaming-tool-call-execution/tasks.md | 102 ++++++++ .../Sessions/LlmSessionActor.cs | 33 +-- .../Pipelines/SessionToolExecutionPipeline.cs | 33 +-- .../Pipelines/StreamingToolWatchdog.cs | 99 ++++++++ .../Tools/DispatchingToolExecutor.cs | 42 ++++ src/Netclaw.Actors/Tools/IToolExecutor.cs | 15 ++ .../INetclawTool.cs | 17 ++ .../ToolCallUpdate.cs | 31 +++ 14 files changed, 875 insertions(+), 46 deletions(-) create mode 100644 openspec/changes/streaming-tool-call-execution/.openspec.yaml create mode 100644 openspec/changes/streaming-tool-call-execution/design.md create mode 100644 openspec/changes/streaming-tool-call-execution/proposal.md create mode 100644 openspec/changes/streaming-tool-call-execution/specs/netclaw-session/spec.md create mode 100644 openspec/changes/streaming-tool-call-execution/specs/netclaw-subagents/spec.md create mode 100644 openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md create mode 100644 openspec/changes/streaming-tool-call-execution/tasks.md create mode 100644 src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs create mode 100644 src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs diff --git a/openspec/changes/streaming-tool-call-execution/.openspec.yaml b/openspec/changes/streaming-tool-call-execution/.openspec.yaml new file mode 100644 index 000000000..66da1ae97 --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-17 diff --git a/openspec/changes/streaming-tool-call-execution/design.md b/openspec/changes/streaming-tool-call-execution/design.md new file mode 100644 index 000000000..47193a291 --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/design.md @@ -0,0 +1,237 @@ +## Context + +A `spawn_agent` tool call is bounded today by four uncoordinated timers: the +parent batch `ProcessingWatchdog` (`SessionConfig.ToolExecutionTimeout`, 90s), +the per-tool attempt `CancelAfter` in `ExecuteToolAttemptAsync` (90s), the +sub-agent's own flat timer (`SubAgentConfig.DefaultTimeoutSeconds`, 60s), and the +spawn `Ask` (~65s). Whichever fires first wins, so an identical task passes or +fails by timing, and a sub-agent actively streaming an LLM response is killed +anyway. + +`ProcessingWatchdog` is structurally single-operation — one operation id, one +timer key. It cannot represent N concurrent tool calls. PR #1035 tried to keep +sub-agents alive by having them heartbeat the parent so it could refresh that +single watchdog; with two parallel `spawn_agent` calls a healthy sibling's +heartbeats mask a wedged one. That PR is abandoned. + +The deeper problem: `INetclawTool.ExecuteAsync` returns `Task`. A tool is +either pending or done — there is no liveness channel. Every long-running tool +needs a bespoke timer. This change gives every tool call a uniform liveness +channel: a stream. + +## Goals / Non-Goals + +**Goals:** + +- Make tool-call execution streaming so liveness is uniform across all tools. +- A per-call inactivity watchdog so parallel tool calls are monitored + independently — a healthy call cannot mask a stalled sibling. +- Move tool-call liveness out of `LlmSessionActor` and `SubAgentActor` into the + tool-execution layer. +- Keep the existing `Task.WhenAll` parallel tool-execution model unchanged. +- Keep the ~28 existing tools and the MCP/AI adapters working with no change. +- Preserve per-call failure isolation: one tool failing never discards a + sibling's result or fails the turn. +- Provide the streaming foundation issue #1038 (background mode) builds on. + +**Non-Goals:** + +- Implementing background/detached `spawn_agent` runs (#1038). +- Mapping MCP `notifications/progress` to activity items (follow-on). +- Changing the LLM-call or compaction watchdog behavior. +- Streaming intermediate tool output into the LLM context — activity items are + ephemeral by design. +- Persisting or wire-serializing `ToolCallUpdate` items. + +## Decisions + +### D1. Tool execution yields `IAsyncEnumerable` + +**Decision:** A tool call produces a stream of `ToolCallUpdate` items: zero or +more non-terminal `ToolActivity` items (a phase label plus an optional output +chunk), then exactly one terminal `ToolCompleted` item carrying the result +string, file attachments, and any sub-agent runs/findings the current +`ToolCallResult` carries. A tool failure or watchdog timeout is surfaced by the +tool-execution layer as a terminal error result, not as an escaping exception. + +**Rationale:** A stream is an ordered, cancellable, terminable liveness channel +that works for every tool, drives the per-call watchdog, and matches the shape +issue #1038 needs. It mirrors the existing LLM streaming path, which already +folds a stream of deltas into one `ChatResponse`. + +**Alternatives considered:** + +- Keep `Task` and add a side-channel progress callback (today's + `OnSubAgentActivity`). Rejected: ad-hoc, sub-agent-specific, and not ordered + or terminable. +- A sub-agent-specific run registry on the session actor. Rejected: solves only + sub-agents and re-introduces shared parent-side state. + +### D2. The streaming method is a default interface method on `INetclawTool` + +**Decision:** `INetclawTool` gains +`IAsyncEnumerable ExecuteStreamAsync(...)` as a default interface +method. Its default body yields one `ToolCompleted` wrapping +`await ExecuteAsync(...)`. `IToolExecutor` / `DispatchingToolExecutor` gain the +matching `ExecuteStreamAsync`. Tools that benefit override the method: +`SpawnAgentTool`, `ShellTool`, `WebFetchTool`. + +**Rationale:** Default interface methods may be async iterators, so all ~28 +`NetclawTool` implementations and the `McpToolAdapter` / `AIToolAdapter` +(which implement `INetclawTool` directly, not via the base) inherit streaming +with no code change. The blast radius is a contract addition plus ~3 opt-in +overrides, not a 28-tool rewrite. + +**Alternatives considered:** + +- A method on the `NetclawTool` base class. Rejected: the MCP and AI + adapters do not derive from the base and would be left out. +- A separate `IStreamingNetclawTool` opt-in interface. Rejected: the executor + would need a type check per call; a default method is uniform. + +### D3. A per-call two-phase inactivity watchdog in the tool-execution layer + +**Decision:** `SessionToolExecutionPipeline.ExecuteSingleToolAsync` consumes each +tool's stream under a per-call, two-phase inactivity watchdog: a generous +*first-item* budget (time to the first `ToolCallUpdate`) and a tighter +*inter-item* budget that resets on every item. On expiry the watchdog cancels +that call's `CancellationToken` and the call yields a terminal timeout error. +The watchdog is a small, pure, `TimeProvider`-driven helper. It replaces the +flat `CancelAfter` in `ExecuteToolAttemptAsync`. + +**Rationale:** Per-call monitoring means parallel tool calls are independent — a +healthy call cannot keep a stalled sibling's timer alive. The two-phase shape +mirrors the LLM watchdog (`PrefillTimeout` then `FirstTokenTimeout`). Enforcement +is centralized in one helper; a streaming tool's only obligation is to emit +activity items. + +**Alternatives considered:** + +- A single flat per-call timeout. Rejected: cannot distinguish a slow first + response from a stalled stream, the same conflation the LLM watchdog already + resolved (`two-phase-streaming-timeout`). + +### D4. `ProcessingWatchdog` reverts to LLM-only; the batch tool watchdog is removed + +**Decision:** `LlmSessionActor.HandleToolCallResponse` no longer arms a +`ToolExecution` operation on `ProcessingWatchdog`. The `ToolExecution` watchdog +handling, `RefreshIfCurrent`, and `PauseToolExecutionWatchdogForApprovalWait` / +`ResumeToolExecutionWatchdogAfterApprovalWait` are removed. `ProcessingWatchdog` +governs only LLM calls and compaction. + +**Rationale:** Tool-call liveness is now the tool-execution layer's job. Keeping +a separate single-operation batch watchdog re-creates the racing-timers bug and +can fail a whole turn while a tool is legitimately running. + +### D5. Activity items are ephemeral — only the terminal result reaches the LLM + +**Decision:** Only the terminal `ToolCompleted` result becomes the `role=Tool` +message appended to the conversation, still clamped to `maxInlineToolResultChars`. +`ToolActivity` items are consumed only by the per-call watchdog and an optional +live UI / session-output relay; they are never accumulated into the LLM context. + +**Rationale:** This mirrors LLM streaming — the user may watch deltas, but the +stored message is the final assembled response. It guarantees a chatty streaming +tool (e.g. `shell_execute` stdout) cannot blow out the context window. + +### D6. `spawn_agent` is a streaming tool; `SubAgentActor` keeps its inactivity watchdog + +**Decision:** `SpawnAgentTool` overrides the streaming method. Instead of +`SubAgentSpawner.SpawnAsync` doing `Ask` and blocking, the +sub-agent's progress is surfaced as `ToolActivity` items and the terminal +`SubAgentResult` as the `ToolCompleted` item. `SubAgentActor` keeps its own +internal inactivity watchdog (self-governance). The absolute wall-clock backstop +is dropped: a run is bounded by the per-call inactivity watchdog plus +`SubAgentActor.MaxToolIterations`. + +**Rationale:** A sub-agent becomes an ordinary streaming tool — same liveness +model as a long shell command. Industry practice (OpenCode `steps`, Claude Code +`max_turns`) bounds agent runs by an iteration cap plus an inactivity timeout, +not a wall-clock cap; netclaw already has `MaxToolIterations`. + +**Alternatives considered:** + +- Keep the absolute backstop as defense-in-depth. Rejected: redundant with the + inactivity watchdog and iteration cap, and its "must exceed inherited budgets" + invariant is unenforceable config foot-gun. + +### D7. Sub-agent recursion is denied by one resolution-time filter + +**Decision:** `spawn_agent` is denied to sub-agents by the single +`SubAgentToolPolicy` denylist applied in `SubAgentSpawner.ResolveTools`. The +redundant `spawn_agent` string-compare in the `SubAgentActor` constructor is +removed. + +**Rationale:** One authoritative filter at tool resolution is easier to reason +about than three overlapping string-matches. The actor trusts that +`definition.Tools` is already resolved. + +### D8. Approval policy is the single authoritative tool-access boundary + +**Decision:** The non-interactive safe-list auto-grant is removed from +`ToolAccessPolicy`. An unapproved tool invoked in a non-interactive session +(e.g. a reminder- or webhook-triggered sub-agent) fails closed with a legible +error that names the tool and the reason. + +**Rationale:** The approval policy already defines what each audience may do; +the implicit hardcoded safe-list was a second, weaker boundary. Removing it means +everything a tool can do is inside the envelope the operator already granted. +Fail-closed with a legible error matches the repo's default-deny posture. + +### D9. Per-call failures never fault the batch + +**Decision:** `ExecuteSingleToolAsync` continues to catch every per-tool +exception and timeout and return it as a `role=Tool` error result keyed to that +`ToolCallId`; it never throws. `ExecuteToolsAsync` keeps `Task.WhenAll`, so all +N results — successes and errors — always reach the LLM as tool-result messages. + +**Rationale:** A wedged sub-agent among N must not discard healthy siblings' +results or fail the turn. Every `tool_use` must also receive a `tool_result` for +conversation validity. Removing the batch watchdog (D4) eliminates the only path +that today bypasses this isolation. + +## Risks / Trade-offs + +- **[Risk]** `INetclawTool` contract change touches a core abstraction. -> + **Mitigation:** the streaming method is a default interface method; existing + tools and adapters are untouched and verified by a regression test. +- **[Risk]** A tool that ignores its `CancellationToken` can still hang the + `await foreach`. -> **Mitigation:** retain a hard `Task`-level cancellation + around stream enumeration; the per-call watchdog cancels the token and the + enumeration is abandoned. +- **[Risk]** Removing the non-interactive auto-grant changes behavior for + reminder/webhook-triggered sub-agents. -> **Mitigation:** intentional; the + failure is legible and operators widen access through the persistent approval + store. +- **[Trade-off]** A slow MCP tool that emits no progress is governed only by the + first-item budget. -> **Mitigation:** that budget is generous and operator- + configurable; mapping MCP progress notifications is a planned follow-on. + +## Migration Plan + +1. Add the `ToolCallUpdate` type and the `ExecuteStreamAsync` default interface + method on `INetclawTool`; add it to `IToolExecutor` / `DispatchingToolExecutor`. +2. Add the per-call two-phase streaming watchdog helper and switch + `SessionToolExecutionPipeline.ExecuteSingleToolAsync` to consume streams. +3. Remove the `ToolExecution` operation from `ProcessingWatchdog` usage in + `LlmSessionActor`. +4. Override the streaming method for `SpawnAgentTool`, `ShellTool`, `WebFetchTool`; + wire `SubAgentActor` progress into the spawn tool's stream; drop the absolute + backstop. +5. Collapse sub-agent recursion to the single `SubAgentToolPolicy` filter; remove + the non-interactive auto-grant from `ToolAccessPolicy`. +6. Add the per-call inactivity budgets to `SessionConfig` and + `netclaw-config.v1.schema.json`. +7. Update tests, runbooks, and the eval suite. + +Rollback: the default interface method makes the contract additive, so reverting +is removing the per-call watchdog and restoring the flat `CancelAfter` and the +`ToolExecution` watchdog operation. + +## Open Questions + +- Exact default values for the first-item and inter-item budgets, and whether + they live on `SessionConfig` or a tool-scoped config section — resolved during + implementation so long as the observable two-phase contract holds. +- Whether `MaxToolIterations` becomes per-`SubAgentProfile` configurable in this + change or a follow-on — implementation may defer it. diff --git a/openspec/changes/streaming-tool-call-execution/proposal.md b/openspec/changes/streaming-tool-call-execution/proposal.md new file mode 100644 index 000000000..1f619387b --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/proposal.md @@ -0,0 +1,124 @@ +## Why + +`spawn_agent` sub-agents are killed mid-stream by a flat ~90s timeout. A single +`spawn_agent` tool call is bounded by four uncoordinated timers — the parent +batch `ProcessingWatchdog` (`ToolExecutionTimeout`, 90s), the per-tool attempt +`CancelAfter` (90s), the sub-agent's own flat timer (`SubAgentConfig.DefaultTimeoutSeconds`, +60s), and the spawn `Ask` (~65s) — and whichever fires first wins. An identical +task passes or fails by timing, and a sub-agent actively streaming an LLM +response is killed anyway. Diagnosed from three production sessions. + +The root cause is not sub-agent-specific. `INetclawTool.ExecuteAsync` returns +`Task`: a tool is either pending or done, with no liveness signal in +between. Anything genuinely long-running — a delegated sub-agent, a long shell +command, a slow MCP tool — has no way to say "still working," so it must be +special-cased with bespoke timers. + +A prior attempt (PR #1035) bolted a per-sub-agent heartbeat protocol onto the +parent's structurally single-operation `ProcessingWatchdog`; with two parallel +`spawn_agent` calls a healthy sibling's heartbeats keep the shared watchdog +alive and mask a wedged sibling. That PR is abandoned in favor of this change. + +The fix is to make the tool-call abstraction itself streaming, so liveness is a +first-class, uniform property of every tool call and a sub-agent stops being a +special case. + +## Source PRDs + +- `PRD-001` (Netclaw MVP): reliable tool execution and predictable runtime + behavior for delegated work. +- `PRD-002` (Gateway Security Envelope): default-deny, fail-closed approval as + the single authoritative tool-access boundary. +- `PRD-006` (MCP Tool Integration): MCP server tools execute under the same + contract as first-party tools. + +## What Changes + +- Tool execution becomes streaming: a tool call yields an `IAsyncEnumerable` of + `ToolCallUpdate` items — zero or more non-terminal *activity* items, then + exactly one terminal *completion* item carrying the result. +- The streaming method is a default interface method on `INetclawTool`; its + default body wraps the existing `Task` execution as a single terminal + item, so every existing tool — including `McpToolAdapter` and `AIToolAdapter` — + works unchanged. Only `SpawnAgentTool`, `ShellTool`, and `WebFetchTool` opt + into real streaming. +- A per-call, two-phase inactivity watchdog moves into the tool-execution layer + (`SessionToolExecutionPipeline`): a generous first-item budget, then a tighter + inter-item budget that resets on each activity item. It replaces the flat + per-attempt `CancelAfter`. +- `ProcessingWatchdog` reverts to governing LLM calls and compaction only. The + parent batch tool-execution watchdog is removed; per-tool liveness is the + tool-execution layer's responsibility, owned by neither `LlmSessionActor` nor + `SubAgentActor`. +- `spawn_agent` becomes a streaming tool: the sub-agent's progress is surfaced as + activity items, the terminal `SubAgentResult` as the completion item. +- **Invariant**: only the terminal completion item enters the conversation and + the LLM context (still clamped to `maxInlineToolResultChars`). Activity items + are ephemeral — they drive the watchdog and an optional live UI relay only. +- `SubAgentActor` keeps its own internal inactivity watchdog; the absolute + wall-clock backstop is dropped — a run is bounded by per-call inactivity plus + `MaxToolIterations`. +- Sub-agent recursion is denied by a single `SubAgentToolPolicy` denylist filter + at tool resolution; the redundant `SubAgentActor` constructor string-compare is + removed. +- The non-interactive safe-list auto-grant is removed from `ToolAccessPolicy`; + the approval policy is the single authoritative boundary, and an unapproved + tool in a non-interactive session fails closed with a legible error. + +### In scope vs out of scope + +In scope: the streaming tool-call contract, the per-call watchdog, the +`ProcessingWatchdog` revert, `spawn_agent` as a streaming tool, the recursion +and approval cleanup. + +Out of scope: + +- Background mode for `spawn_agent` (issue #1038) — this change provides the + streaming foundation but does not implement detached runs. +- Mapping MCP `notifications/progress` onto activity items — MCP tools work via + the single-item default; the progress upgrade is a follow-on. + +## Capabilities + +### New Capabilities + +- None — this modifies existing tool-execution, sub-agent, and session behavior. + +### Modified Capabilities + +- `netclaw-tools`: tool execution becomes a streaming contract with a per-call + two-phase inactivity watchdog; non-streaming tools and MCP/AI adapters inherit + a single-terminal-item default; only the terminal result enters LLM context. +- `netclaw-subagents`: `spawn_agent` executes as a streaming tool; sub-agent runs + are bounded by inactivity plus iteration cap with no wall-clock backstop; + recursion is denied by one resolution-time filter. +- `netclaw-session`: `ProcessingWatchdog` no longer covers tool execution; the + parent batch tool-execution watchdog is removed; per-tool failures never fault + the batch. + +## Impact + +- **Contract**: `INetclawTool` gains a streaming default interface method; + `IToolExecutor` / `DispatchingToolExecutor` gain `ExecuteStreamAsync`. New + `ToolCallUpdate` type. ~28 `NetclawTool` tools and the MCP/AI adapters + need no change. +- **Actor**: `LlmSessionActor.HandleToolCallResponse` stops arming a + `ToolExecution` watchdog operation; `ProcessingWatchdog.RefreshIfCurrent` and + the approval-pause/resume of the tool watchdog are removed. `SubAgentActor` + keeps its inactivity watchdog and loses the absolute backstop. +- **Pipeline**: `SessionToolExecutionPipeline` consumes streams with the new + per-call watchdog; `Task.WhenAll` over independent tool calls is preserved. +- **Config**: new per-call tool-streaming inactivity budgets (first-item / + inter-item) in `SessionConfig`; `netclaw-config.v1.schema.json` updated in the + same change per the schema-sync rule. +- **Security**: removing the non-interactive auto-grant means a non-interactive + sub-agent (reminder/webhook-triggered) calling an unapproved tool fails closed + with a legible error rather than being silently auto-granted; everything a tool + can do stays inside the approval policy the operator already granted. +- **Operations**: a wedged tool or sub-agent is caught by its own per-call + watchdog and surfaced as a tool-result error; sibling tool calls and the turn + survive. +- **No wire-format or persistence changes** — runtime behavior only. +- **Tests/eval**: new per-call watchdog unit tests (`FakeTimeProvider`); parallel + spawn and mixed-batch integration tests; eval suite re-run for the tool-surface + and `SessionConfig` change. diff --git a/openspec/changes/streaming-tool-call-execution/specs/netclaw-session/spec.md b/openspec/changes/streaming-tool-call-execution/specs/netclaw-session/spec.md new file mode 100644 index 000000000..bcc31e36f --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/specs/netclaw-session/spec.md @@ -0,0 +1,36 @@ +## ADDED Requirements + +### Requirement: The processing watchdog covers LLM calls and compaction only + +The session processing watchdog SHALL govern only LLM streaming calls and +compaction. It SHALL NOT be armed for tool execution. Tool-call liveness SHALL be +the responsibility of the per-call watchdog in the tool-execution layer, owned by +neither the session actor nor the sub-agent actor. + +#### Scenario: Tool execution does not arm the processing watchdog + +- **GIVEN** the session dispatches a batch of tool calls +- **THEN** the processing watchdog is not armed for a tool-execution operation +- **AND** each tool call is monitored only by its own per-call watchdog + +#### Scenario: A long tool call does not trip a session-level timeout + +- **GIVEN** a tool call that runs longer than the former tool-execution budget +- **AND** it is emitting activity within its per-call inactivity budget +- **THEN** no session-level watchdog fails the turn +- **AND** the tool call runs to completion + +### Requirement: A tool batch fails the turn only on infrastructure failure + +A batch of tool calls SHALL complete and deliver every tool-result message +whenever each tool call resolves to a result — success or per-call error. The +turn SHALL be failed wholesale only when the tool-execution pipeline itself fails +for reasons outside any individual tool call. + +#### Scenario: A per-call timeout does not fail the turn + +- **GIVEN** a batch of tool calls +- **WHEN** one call times out under its per-call watchdog +- **THEN** that call returns a timeout error result +- **AND** the batch completes and delivers all tool-result messages +- **AND** the turn is not failed wholesale diff --git a/openspec/changes/streaming-tool-call-execution/specs/netclaw-subagents/spec.md b/openspec/changes/streaming-tool-call-execution/specs/netclaw-subagents/spec.md new file mode 100644 index 000000000..26a8cf54e --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/specs/netclaw-subagents/spec.md @@ -0,0 +1,59 @@ +## ADDED Requirements + +### Requirement: spawn_agent executes as a streaming tool + +The `spawn_agent` tool SHALL execute as a streaming tool: while the sub-agent +runs, the tool call SHALL emit activity items reflecting sub-agent progress and +SHALL finish with a terminal completion item carrying the sub-agent result. The +parent session SHALL NOT bound the sub-agent with a dedicated wall-clock `Ask` +timeout; sub-agent liveness SHALL be observed through the tool call's stream and +its per-call inactivity watchdog. + +#### Scenario: Sub-agent progress surfaces as activity items + +- **GIVEN** a `spawn_agent` tool call +- **WHEN** the spawned sub-agent is making progress +- **THEN** the tool call emits activity items while the sub-agent runs +- **AND** the call's per-call inactivity watchdog is satisfied by that activity + +#### Scenario: Wedged sub-agent is caught without affecting siblings + +- **GIVEN** two `spawn_agent` tool calls running in parallel +- **AND** one sub-agent is wedged and emits no activity +- **WHEN** the wedged call's inactivity budget elapses +- **THEN** that call yields a terminal timeout error +- **AND** the healthy sub-agent completes normally and returns its result +- **AND** both tool-result messages reach the LLM + +### Requirement: Sub-agent runs are bounded by inactivity and iteration count + +A sub-agent run SHALL be bounded by the per-call inactivity watchdog and by the +sub-agent's maximum tool-iteration count. The system SHALL NOT impose an absolute +wall-clock cap on a sub-agent run that is continuously producing activity. + +#### Scenario: A responsive long sub-agent is not killed by a wall-clock cap + +- **GIVEN** a sub-agent that runs longer than any single inactivity budget +- **AND** it emits activity continuously +- **THEN** it is not terminated by an absolute wall-clock timeout +- **AND** it runs until completion or the tool-iteration limit + +#### Scenario: A stalled sub-agent is terminated by inactivity + +- **GIVEN** a sub-agent that stops producing any activity +- **WHEN** its inactivity budget elapses +- **THEN** the run is terminated and the call yields a timeout error + +### Requirement: Sub-agents cannot spawn sub-agents + +The `spawn_agent` tool SHALL be denied to sub-agents by a single tool-policy +denylist applied when a sub-agent's tool set is resolved. A sub-agent's resolved +tool set SHALL never include `spawn_agent`, regardless of what its profile lists +or inherits. + +#### Scenario: spawn_agent is absent from a resolved sub-agent tool set + +- **GIVEN** a sub-agent profile that lists or inherits `spawn_agent` +- **WHEN** the sub-agent's tools are resolved +- **THEN** `spawn_agent` is excluded from the resolved tool set +- **AND** the sub-agent cannot invoke it diff --git a/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md b/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md new file mode 100644 index 000000000..63fe3a8ea --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md @@ -0,0 +1,91 @@ +## ADDED Requirements + +### Requirement: Tool execution is a streaming contract + +Tool execution SHALL be expressed as a stream: an invocation yields an ordered +sequence of `ToolCallUpdate` items — zero or more non-terminal *activity* items +followed by exactly one terminal *completion* item. The completion item SHALL +carry the tool result and any file attachments and sub-agent outputs the +invocation produced. + +`INetclawTool` SHALL expose the streaming method as a default interface method +whose default implementation yields a single completion item wrapping the tool's +existing non-streaming result. A tool that does not override the method SHALL +therefore behave identically to its current non-streaming behavior. + +#### Scenario: Non-streaming tool yields a single completion item + +- **GIVEN** a tool that does not override the streaming method +- **WHEN** it is invoked +- **THEN** exactly one terminal completion item is produced +- **AND** it carries the same result the non-streaming execution would have returned + +#### Scenario: Streaming tool emits activity then completion + +- **GIVEN** a tool that overrides the streaming method (e.g. `shell_execute`) +- **WHEN** it is invoked and runs over time +- **THEN** it emits one or more activity items while working +- **AND** it finishes with exactly one terminal completion item + +### Requirement: Per-call two-phase inactivity watchdog + +Each tool call SHALL be monitored by its own two-phase inactivity watchdog in the +tool-execution layer: a first-item budget bounding the time to the first +`ToolCallUpdate`, and an inter-item budget that resets on every subsequent item. +When a budget elapses the watchdog SHALL cancel that call, and the call SHALL +yield a terminal error result identifying the tool and the timeout. + +The watchdog SHALL be per call. Parallel tool calls SHALL be monitored +independently — activity on one call SHALL NOT extend the budget of another. + +#### Scenario: Stalled stream trips the inter-item budget + +- **GIVEN** a streaming tool call that has emitted at least one activity item +- **WHEN** no further item arrives within the inter-item budget +- **THEN** the watchdog cancels the call +- **AND** the call yields a terminal error naming the tool and the timeout + +#### Scenario: Slow first item trips the first-item budget + +- **GIVEN** a tool call that has emitted no items yet +- **WHEN** the first-item budget elapses +- **THEN** the watchdog cancels the call +- **AND** the call yields a terminal error naming the tool and the timeout + +#### Scenario: A healthy call does not mask a stalled sibling + +- **GIVEN** two tool calls executing in parallel +- **AND** one is emitting activity items steadily +- **AND** the other has gone silent past its inter-item budget +- **THEN** the silent call is timed out independently +- **AND** the healthy call continues unaffected + +### Requirement: Only the terminal result enters the conversation + +Only the terminal completion item's result SHALL be appended to the conversation +as the tool-result message, clamped to the configured maximum inline tool-result +size. Activity items SHALL NOT be accumulated into the conversation or the LLM +context; they serve only the per-call watchdog and an optional live output relay. + +#### Scenario: Streamed intermediate output does not reach the LLM + +- **GIVEN** a streaming tool that emits many activity items with output chunks +- **WHEN** the call completes +- **THEN** the LLM receives exactly one tool-result message for the call +- **AND** that message contains only the terminal result, clamped as today +- **AND** the intermediate activity chunks are absent from the conversation + +### Requirement: Per-tool failures are isolated from the batch + +A tool call that fails or times out SHALL yield a terminal error result keyed to +its own tool-call id. It SHALL NOT abort, discard, or fault sibling tool calls +executing in the same batch. Every tool call in a batch SHALL produce exactly one +tool-result message — success or error. + +#### Scenario: One tool fails, siblings still return + +- **GIVEN** a batch of tool calls executed in parallel +- **WHEN** one call fails or times out +- **THEN** that call produces a tool-result message containing its error +- **AND** every other call produces its normal tool-result message +- **AND** the turn continues with all tool-result messages delivered to the LLM diff --git a/openspec/changes/streaming-tool-call-execution/tasks.md b/openspec/changes/streaming-tool-call-execution/tasks.md new file mode 100644 index 000000000..f97c27af3 --- /dev/null +++ b/openspec/changes/streaming-tool-call-execution/tasks.md @@ -0,0 +1,102 @@ +# Tasks: Streaming Tool-Call Execution + +## Phase A: Streaming contract foundation + +- [ ] Add a `ToolCallUpdate` type: a non-terminal activity variant (phase label + + optional output chunk) and a terminal completion variant (result string + + file attachments + completed sub-agent runs + accepted findings) +- [ ] Add `ExecuteStreamAsync` to `INetclawTool` as a default interface method — + default body yields one terminal completion item wrapping the existing + `ExecuteAsync(arguments, context, ct)` +- [ ] Add `ExecuteStreamAsync` to `IToolExecutor`; implement in + `DispatchingToolExecutor` (authorize, resolve, surface the tool's stream, + redact secrets per item, log) +- [ ] Verify build clean (0 warnings); the ~28 `NetclawTool` tools and + `McpToolAdapter` / `AIToolAdapter` compile with no change +- **Acceptance:** a tool that does not override `ExecuteStreamAsync` produces + exactly one terminal completion item carrying its current result + +## Phase B: Per-call streaming watchdog + +- [ ] Add a two-phase `StreamingToolWatchdog` helper (first-item budget + + inter-item budget that resets on each item), `TimeProvider`-driven, with no + Akka or actor dependency +- [ ] Switch `SessionToolExecutionPipeline.ExecuteSingleToolAsync` to consume + `ExecuteStreamAsync` under the per-call watchdog; remove the flat `CancelAfter` + in `ExecuteToolAttemptAsync` +- [ ] On budget expiry: cancel the call's token; yield a terminal error result + naming the tool and the timeout (keyed to the tool-call id) +- [ ] Add first-item and inter-item tool inactivity budgets to `SessionConfig`, + `RawSessionConfig`, and `BindFromConfiguration` +- [ ] Update `src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json` + with the new properties (defaults included; schema-sync rule) +- [ ] Verify build; `Task.WhenAll` over independent tool calls still drives + `ExecuteToolsAsync` +- **Acceptance:** a stalled stream trips the inter-item budget; a slow first + item trips the first-item budget; a healthy parallel call is unaffected + +## Phase C: Revert ProcessingWatchdog to LLM-only + +- [ ] `LlmSessionActor.HandleToolCallResponse` no longer arms a `ToolExecution` + operation on `ProcessingWatchdog` +- [ ] Remove `ToolExecution`-operation handling, `ProcessingWatchdog.RefreshIfCurrent`, + and `PauseToolExecutionWatchdogForApprovalWait` / + `ResumeToolExecutionWatchdogAfterApprovalWait` +- [ ] Confirm `ProcessingWatchdog` governs only `LlmCall` and `Compaction` +- [ ] Verify build; update/trim watchdog tests that asserted tool-execution + watchdog behavior +- **Acceptance:** dispatching a tool batch arms no processing-watchdog operation; + a long tool call does not trip a session-level timeout + +## Phase D: spawn_agent as a streaming tool + +- [ ] `SpawnAgentTool` overrides `ExecuteStreamAsync`; route `SubAgentActor` + progress into a `Channel` consumed as the tool's stream +- [ ] `SubAgentSpawner` surfaces the run as a stream instead of a blocking + `Ask`; terminal `SubAgentResult` becomes the completion item +- [ ] `SubAgentActor` keeps its own internal inactivity watchdog; remove the + absolute wall-clock backstop and its timer +- [ ] Verify build +- **Acceptance:** a `spawn_agent` call emits activity while the sub-agent works; + two parallel `spawn_agent` calls with one wedged — the wedged one times out + independently, the healthy one returns, both tool-result messages reach the LLM + +## Phase E: Recursion and approval cleanup + +- [ ] Collapse sub-agent recursion to the single `SubAgentToolPolicy` denylist in + `SubAgentSpawner.ResolveTools`; remove the `spawn_agent` string-compare in the + `SubAgentActor` constructor +- [ ] Remove the non-interactive safe-list auto-grant from `ToolAccessPolicy`; + ensure an unapproved tool in a non-interactive session fails closed with a + legible error naming the tool and the reason +- [ ] Verify build +- **Acceptance:** `spawn_agent` is absent from any resolved sub-agent tool set; a + non-interactive sub-agent calling an unapproved tool fails with a legible error + +## Phase F: Opt-in streaming tools + +- [ ] `ShellTool` overrides `ExecuteStreamAsync` to emit stdout/stderr as activity + items; the terminal item carries the assembled, clamped result (replaces + buffer-everything-then-truncate) +- [ ] `WebFetchTool` overrides `ExecuteStreamAsync` to emit fetch-progress + activity items (optional within this change) +- **Acceptance:** streamed shell output appears as activity items only; the LLM + still receives one clamped terminal result per call + +## Phase G: Tests, docs, eval + +- [ ] Unit tests for `StreamingToolWatchdog` with `FakeTimeProvider` — + deterministic, no `Task.Delay` (per `CLAUDE.md` testing rules) +- [ ] Regression test: a non-streaming tool yields exactly one terminal item +- [ ] Integration test: two concurrent `spawn_agent` calls, one wedged — wedged + one caught independently, healthy result + timeout error both reach the LLM +- [ ] Integration test: mixed batch `[spawn_agent, hung tool]` — the hung tool is + still caught; no whole-batch failure +- [ ] Update `docs/runbooks/subagents.md` and any tool-timeout operator guidance +- [ ] Update the `netclaw-operations` system skill if tool/timeout guidance + changed (System Skills Sync Rule) +- [ ] `dotnet slopwatch analyze` — no new violations; `./scripts/Add-FileHeaders.ps1 -Verify` +- [ ] Run `./evals/run-evals.sh` (tool surface + `SessionConfig` changed) +- **Acceptance:** full `Netclaw.Actors.Tests` / `Netclaw.Configuration.Tests` + suites pass; eval suite passes; manual repro confirms a heavy `spawn_agent` no + longer dies mid-stream and parallel spawns with one wedged do not hang the turn diff --git a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs index 1cd95f175..039df28d4 100644 --- a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs +++ b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs @@ -799,8 +799,6 @@ private void Processing() msg.IsMessy, msg.Candidates); - PauseToolExecutionWatchdogForApprovalWait(msg.CallId.Value); - EmitOutput(msg); }); @@ -852,8 +850,6 @@ await PersistApprovalCandidatesAsync( _pendingToolInteractions.Remove(msg.CallId.Value); - ResumeToolExecutionWatchdogAfterApprovalWait(); - // Complete the TCS so the blocked pipeline task can proceed _approvalChannel.Complete(msg.CallId, decision); }); @@ -1681,10 +1677,11 @@ private void HandleToolCallResponse( var tp = _timeProvider; var sessionDir = GetSessionDirectory(); var maxInlineToolResultChars = _config.Tuning.MaxInlineToolResultChars; + // Per-call inactivity watchdogs in the tool-execution pipeline govern + // tool liveness; the session ProcessingWatchdog covers only LLM calls + // and compaction, so no batch tool-execution watchdog is armed here. var toolExecutionTimeout = _config.ToolExecutionTimeout; - _watchdog.Start(ProcessingWatchdog.ToolExecution, toolExecutionTimeout, Timers); - // Capture subscriber snapshot for subagent activity notifications. // These are emitted directly from the tool execution thread via Tell(), // which is thread-safe. The snapshot ensures we don't read _subscribers @@ -3067,30 +3064,6 @@ private void EmitUsageOutput(UsageDetails usage) }, OutputFilter.Usage); } - private void PauseToolExecutionWatchdogForApprovalWait(string callId) - { - if (!string.Equals(_watchdog.CurrentOperationName, ProcessingWatchdog.ToolExecution, StringComparison.Ordinal)) - return; - - _watchdog.Stop(Timers); - _log.Info("Paused tool-execution watchdog while waiting for approval for call {CallId}", callId); - } - - private void ResumeToolExecutionWatchdogAfterApprovalWait() - { - if (_pendingToolInteractions.Count > 0) - return; - - if (_currentPhase != SessionPhase.Processing) - return; - - if (_watchdog.CurrentOperationName is not null) - return; - - _watchdog.Start(ProcessingWatchdog.ToolExecution, _config.ToolExecutionTimeout, Timers); - _log.Info("Resumed tool-execution watchdog after approval response"); - } - private void FailCurrentTurn(string errorMessage, Exception cause, ErrorCategory category = ErrorCategory.Unknown) { CompleteReminderInFlight(_currentTurnSource?.ReminderId); diff --git a/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs b/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs index e8cece55c..5ea333ce9 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs @@ -261,7 +261,7 @@ public static async Task ExecuteSingleToolAsync( } } - resultText = await ExecuteToolAttemptAsync(executor, tc, context, timeout, ct); + resultText = await ExecuteToolAttemptAsync(executor, tc, context, timeout, timeProvider, ct); sw.Stop(); auditLogger?.Log(BuildAuditEntry(sessionId, tc, timeProvider, sw.Elapsed, meta) with @@ -339,7 +339,7 @@ or ApprovalDecision.ApprovedAlways string.Join(", ", ctx.Patterns)); } - resultText = await ExecuteToolAttemptAsync(executor, tc, context, timeout, ct); + resultText = await ExecuteToolAttemptAsync(executor, tc, context, timeout, timeProvider, ct); sw.Stop(); var patternStr = string.Join(", ", ctx.Patterns); @@ -441,27 +441,28 @@ private static async Task ExecuteToolAttemptAsync( FunctionCallContent toolCall, ToolExecutionContext context, TimeSpan timeout, + TimeProvider timeProvider, CancellationToken cancellationToken) { var grantedOneTimeToolName = context.OneTimeApprovedToolName; var grantedOneTimePatterns = context.OneTimeApprovedPatterns; - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - if (timeout != Timeout.InfiniteTimeSpan) - timeoutCts.CancelAfter(timeout); - try { - return await executor.ExecuteAsync(toolCall, context, timeoutCts.Token); - } - catch (OperationCanceledException ex) - when (!cancellationToken.IsCancellationRequested - && timeout != Timeout.InfiniteTimeSpan - && timeoutCts.IsCancellationRequested) - { - throw new TimeoutException( - $"Tool execution exceeded timeout of {timeout.TotalSeconds:F0}s", - ex); + // The tool call is consumed as a stream under a per-call inactivity + // watchdog. A non-streaming tool emits only the terminal completion + // item, so the watchdog reduces to the former flat timeout for it; + // a streaming tool stays alive while it reports activity. Inactivity + // surfaces as TimeoutException, which the caller turns into a + // per-tool error result without faulting sibling tool calls. + return await StreamingToolWatchdog.ConsumeAsync( + executor.ExecuteStreamAsync(toolCall, context, cancellationToken), + toolCall.Name, + firstItemBudget: timeout, + interItemBudget: timeout, + timeProvider, + onActivity: null, + cancellationToken); } finally { diff --git a/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs new file mode 100644 index 000000000..b31f5d72e --- /dev/null +++ b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs @@ -0,0 +1,99 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Netclaw.Tools; + +namespace Netclaw.Actors.Sessions.Pipelines; + +/// +/// Consumes a single tool call's stream under a +/// per-call, two-phase inactivity watchdog: the call must produce its first +/// item within firstItemBudget, and every subsequent item resets the +/// timer to interItemBudget. Inactivity past the current budget cancels +/// the call and surfaces a . +/// +/// This is the only liveness control for a tool call — there is no batch-level +/// watchdog. Each call has its own watchdog, so a healthy parallel call cannot +/// extend (or mask) a stalled sibling. The timer is created through the supplied +/// so it can be virtualized in tests. +/// +internal static class StreamingToolWatchdog +{ + /// + /// Enumerate under the inactivity watchdog and + /// return the terminal completion result. Activity items reset the watchdog + /// and are forwarded to ; only the terminal + /// contributes the returned result. + /// + public static async Task ConsumeAsync( + IAsyncEnumerable stream, + string toolName, + TimeSpan firstItemBudget, + TimeSpan interItemBudget, + TimeProvider timeProvider, + Action? onActivity, + CancellationToken ct) + { + using var watchdogCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + + // `await using` the timer: ITimer.DisposeAsync waits for any in-flight + // callback to finish before `watchdogCts` (declared above it, so disposed + // after it) is disposed, so the callback's Cancel() can never race a + // disposed token source. + await using var timer = timeProvider.CreateTimer( + static state => ((CancellationTokenSource)state!).Cancel(), + watchdogCts, + firstItemBudget, + Timeout.InfiniteTimeSpan); + + var currentBudget = firstItemBudget; + string? result = null; + + var enumerator = stream.GetAsyncEnumerator(watchdogCts.Token); + try + { + while (true) + { + bool hasNext; + try + { + hasNext = await enumerator.MoveNextAsync(); + } + catch (OperationCanceledException) + when (watchdogCts.IsCancellationRequested && !ct.IsCancellationRequested) + { + throw new TimeoutException( + $"Tool '{toolName}' produced no activity for " + + $"{currentBudget.TotalSeconds:F0}s and was stopped. It may be stuck — " + + "please try again, or simplify the request."); + } + + if (!hasNext) + break; + + // Any item — activity or completion — is liveness: reset the + // watchdog to the (tighter) inter-item budget. + currentBudget = interItemBudget; + timer.Change(interItemBudget, Timeout.InfiniteTimeSpan); + + switch (enumerator.Current) + { + case ToolCompletedUpdate completed: + result = completed.Result; + break; + case ToolActivityUpdate activity: + onActivity?.Invoke(activity); + break; + } + } + } + finally + { + await enumerator.DisposeAsync(); + } + + return result ?? $"Tool '{toolName}' completed without producing a result."; + } +} diff --git a/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs b/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs index 3358fb70b..ff4f594af 100644 --- a/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs +++ b/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using System.Diagnostics; +using System.Runtime.CompilerServices; using Microsoft.Extensions.AI; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -90,6 +91,47 @@ public async Task AuthorizeAsync(FunctionCallContent toolCall, ToolExecutionCont _ = await AuthorizeCoreAsync(toolCall, context, ct); } + public async IAsyncEnumerable ExecuteStreamAsync( + FunctionCallContent toolCall, + ToolExecutionContext? context = null, + [EnumeratorCancellation] CancellationToken ct = default) + { + if (_registry.GetByName(toolCall.Name) is null) + { + _logger.LogWarning("Unknown tool requested: {ToolName}", toolCall.Name); + yield return new ToolCompletedUpdate($"Unknown tool: {toolCall.Name}"); + yield break; + } + + // Authorization throws (ToolApprovalRequiredException / ToolAccessDeniedException) + // before the first item is produced; the tool-execution pipeline handles + // those exactly as it does for the non-streaming path. + var tool = await AuthorizeCoreAsync(toolCall, context, ct); + var execContext = context ?? ToolExecutionContext.Empty; + + var sw = Stopwatch.StartNew(); + await foreach (var update in tool.ExecuteStreamAsync(toolCall.Arguments, execContext, ct)) + { + switch (update) + { + case ToolCompletedUpdate completed: + sw.Stop(); + var redacted = SecretOutputRedactor.Redact(completed.Result); + _logger.LogInformation( + "Tool executed: {ToolName} ({Duration}ms, {ResultLength} chars)", + toolCall.Name, sw.ElapsedMilliseconds, redacted.Length); + yield return new ToolCompletedUpdate(redacted); + break; + case ToolActivityUpdate { OutputChunk: not null } activity: + yield return activity with { OutputChunk = SecretOutputRedactor.Redact(activity.OutputChunk) }; + break; + default: + yield return update; + break; + } + } + } + private async Task AuthorizeCoreAsync(FunctionCallContent toolCall, ToolExecutionContext? context, CancellationToken ct) { if (context is not null) diff --git a/src/Netclaw.Actors/Tools/IToolExecutor.cs b/src/Netclaw.Actors/Tools/IToolExecutor.cs index 5dfaeb455..e11d74ff2 100644 --- a/src/Netclaw.Actors/Tools/IToolExecutor.cs +++ b/src/Netclaw.Actors/Tools/IToolExecutor.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Runtime.CompilerServices; using Microsoft.Extensions.AI; using Netclaw.Actors.Protocol; using Netclaw.Tools; @@ -18,6 +19,20 @@ public interface IToolExecutor Task ExecuteAsync(FunctionCallContent toolCall, ToolExecutionContext? context = null, CancellationToken ct = default); Task AuthorizeAsync(FunctionCallContent toolCall, ToolExecutionContext? context = null, CancellationToken ct = default); + + /// + /// Execute a tool call as a stream of items. The + /// default implementation runs and yields its + /// result as a single terminal completion item; + /// overrides this to surface the resolved tool's own stream. + /// + async IAsyncEnumerable ExecuteStreamAsync( + FunctionCallContent toolCall, + ToolExecutionContext? context = null, + [EnumeratorCancellation] CancellationToken ct = default) + { + yield return new ToolCompletedUpdate(await ExecuteAsync(toolCall, context, ct)); + } } /// diff --git a/src/Netclaw.Tools.Abstractions/INetclawTool.cs b/src/Netclaw.Tools.Abstractions/INetclawTool.cs index 7fdc7a9e5..4c568ef93 100644 --- a/src/Netclaw.Tools.Abstractions/INetclawTool.cs +++ b/src/Netclaw.Tools.Abstractions/INetclawTool.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Runtime.CompilerServices; using System.Text.Json; using Microsoft.Extensions.AI; @@ -48,4 +49,20 @@ public interface INetclawTool /// Task ExecuteAsync(IDictionary? arguments, ToolExecutionContext context, CancellationToken ct = default) => ExecuteAsync(arguments, ct); + + /// + /// Execute the tool as a stream of items: zero + /// or more non-terminal items, then exactly + /// one terminal . The default implementation + /// runs the non-streaming context overload and yields its result as a single + /// completion item, so tools that do not stream behave identically. Long- + /// running tools override this to emit liveness/progress while they work. + /// + async IAsyncEnumerable ExecuteStreamAsync( + IDictionary? arguments, + ToolExecutionContext context, + [EnumeratorCancellation] CancellationToken ct = default) + { + yield return new ToolCompletedUpdate(await ExecuteAsync(arguments, context, ct)); + } } diff --git a/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs b/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs new file mode 100644 index 000000000..2fd3e4eff --- /dev/null +++ b/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs @@ -0,0 +1,31 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +namespace Netclaw.Tools; + +/// +/// One item in a streaming tool-call result. A tool invocation yields zero or +/// more non-terminal items followed by exactly +/// one terminal . +/// +public abstract record ToolCallUpdate; + +/// +/// A non-terminal progress/liveness signal emitted while a tool is still +/// running. Activity items drive the per-call inactivity watchdog and an +/// optional live output relay; they are never accumulated into LLM context. +/// +/// A short label describing what the tool is doing. +/// +/// Optional incremental output (e.g. streamed shell stdout) for live display. +/// +public sealed record ToolActivityUpdate(string Phase, string? OutputChunk = null) : ToolCallUpdate; + +/// +/// The terminal item of a tool-call stream. Its is the only +/// part of the stream that becomes the tool-result message in the conversation. +/// +/// The tool's final result text. +public sealed record ToolCompletedUpdate(string Result) : ToolCallUpdate; From 9767e2b7654f5f2058e5defb0331cfb3d45c18d2 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 19:04:18 -0500 Subject: [PATCH 2/7] fix(tools): race-free per-call tool watchdog; harden phase A-C foundation Review cleanup of the streaming tool-call foundation before phase D. - StreamingToolWatchdog: replace the reset-on-each-item timer with a periodic poll of a last-activity timestamp. Resetting a timer cannot recall an already-elapsed callback, so a reset racing the callback could cancel a still-live streaming tool (a false timeout). The poll model has no such race; the callback never touches the timer. - A tool-call stream that ends with no completion item now throws instead of synthesizing a result string (no silent fallback); the pipeline turns it into a contained per-tool error. - Introduce ToolWatchdogBudget (with a Flat factory) so the watchdog takes one budget value instead of two positionally-ambiguous TimeSpans. - Remove the now-dead ProcessingWatchdog.ToolExecution constant and its unreachable switch arm; correct ProcessingWatchdog's summary. - Re-document SessionConfig.ToolExecutionTimeout and its schema entry: it is now a per-tool-call inactivity budget, not a batch timeout. Build clean; Netclaw.Actors.Tests 1609/1609 pass; slopwatch clean. --- .../Sessions/Handlers/ProcessingWatchdog.cs | 6 +- .../Sessions/LlmSessionActor.cs | 1 - .../Pipelines/SessionToolExecutionPipeline.cs | 13 ++- .../Pipelines/StreamingToolWatchdog.cs | 89 ++++++++++++------- .../Schemas/netclaw-config.v1.schema.json | 2 +- src/Netclaw.Configuration/SessionConfig.cs | 8 +- 6 files changed, 69 insertions(+), 50 deletions(-) diff --git a/src/Netclaw.Actors/Sessions/Handlers/ProcessingWatchdog.cs b/src/Netclaw.Actors/Sessions/Handlers/ProcessingWatchdog.cs index 11f36ad95..a9f76d962 100644 --- a/src/Netclaw.Actors/Sessions/Handlers/ProcessingWatchdog.cs +++ b/src/Netclaw.Actors/Sessions/Handlers/ProcessingWatchdog.cs @@ -8,13 +8,13 @@ namespace Netclaw.Actors.Sessions.Handlers; /// -/// Manages the processing watchdog timer that detects stuck LLM calls, -/// tool executions, and compaction operations. +/// Manages the processing watchdog timer that detects stuck LLM calls and +/// compaction operations. Tool-call liveness is handled per call by the +/// tool-execution pipeline, not here. /// internal sealed class ProcessingWatchdog { public const string LlmCall = "llm-call"; - public const string ToolExecution = "tool-execution"; public const string Compaction = "compaction"; private static readonly object TimerKey = new(); diff --git a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs index 039df28d4..80ff4cad5 100644 --- a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs +++ b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs @@ -944,7 +944,6 @@ await PersistApprovalCandidatesAsync( var timeout = msg.OperationName switch { - ProcessingWatchdog.ToolExecution => _config.ToolExecutionTimeout, ProcessingWatchdog.LlmCall => _config.FirstTokenTimeout, _ => _config.TurnLlmTimeout }; diff --git a/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs b/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs index 5ea333ce9..291a793f8 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs @@ -449,17 +449,14 @@ private static async Task ExecuteToolAttemptAsync( try { - // The tool call is consumed as a stream under a per-call inactivity - // watchdog. A non-streaming tool emits only the terminal completion - // item, so the watchdog reduces to the former flat timeout for it; - // a streaming tool stays alive while it reports activity. Inactivity - // surfaces as TimeoutException, which the caller turns into a - // per-tool error result without faulting sibling tool calls. + // Consumed as a stream under a per-call inactivity watchdog. A + // non-streaming tool emits only the terminal item, so a flat budget + // is equivalent to the former timeout; inactivity throws + // TimeoutException, which the caller turns into a per-tool error. return await StreamingToolWatchdog.ConsumeAsync( executor.ExecuteStreamAsync(toolCall, context, cancellationToken), toolCall.Name, - firstItemBudget: timeout, - interItemBudget: timeout, + ToolWatchdogBudget.Flat(timeout), timeProvider, onActivity: null, cancellationToken); diff --git a/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs index b31f5d72e..01021fcec 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs @@ -8,49 +8,67 @@ namespace Netclaw.Actors.Sessions.Pipelines; /// -/// Consumes a single tool call's stream under a -/// per-call, two-phase inactivity watchdog: the call must produce its first -/// item within firstItemBudget, and every subsequent item resets the -/// timer to interItemBudget. Inactivity past the current budget cancels -/// the call and surfaces a . +/// Inactivity budget for one tool call: bounds the wait +/// for the first , the gap +/// between later items. uses one value for both. A +/// non-positive budget disables the watchdog. +/// +internal readonly record struct ToolWatchdogBudget(TimeSpan FirstItem, TimeSpan InterItem) +{ + public static ToolWatchdogBudget Flat(TimeSpan budget) => new(budget, budget); +} + +/// +/// Consumes one tool call's stream under a per-call +/// inactivity watchdog and returns the terminal completion result. /// /// This is the only liveness control for a tool call — there is no batch-level -/// watchdog. Each call has its own watchdog, so a healthy parallel call cannot -/// extend (or mask) a stalled sibling. The timer is created through the supplied -/// so it can be virtualized in tests. +/// watchdog, so a healthy parallel call cannot extend (or mask) a stalled +/// sibling. A periodic timer polls a last-activity timestamp rather than being +/// reset on each item: resetting a timer cannot recall an already-elapsed +/// callback, so a reset racing the callback could cancel a still-live call — +/// polling has no such race. The timer comes from the supplied +/// so it can be virtualized in tests. A tool whose +/// iterator ignores the enumerator's cancellation token cannot be force-stopped. /// internal static class StreamingToolWatchdog { - /// - /// Enumerate under the inactivity watchdog and - /// return the terminal completion result. Activity items reset the watchdog - /// and are forwarded to ; only the terminal - /// contributes the returned result. - /// + private static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(1); + public static async Task ConsumeAsync( IAsyncEnumerable stream, string toolName, - TimeSpan firstItemBudget, - TimeSpan interItemBudget, + ToolWatchdogBudget budget, TimeProvider timeProvider, Action? onActivity, CancellationToken ct) { using var watchdogCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - // `await using` the timer: ITimer.DisposeAsync waits for any in-flight - // callback to finish before `watchdogCts` (declared above it, so disposed - // after it) is disposed, so the callback's Cancel() can never race a - // disposed token source. + // Shared with the timer callback. Both are long fields (atomic reads/ + // writes); the consumer writes them on each item, the callback reads them. + var lastActivity = timeProvider.GetTimestamp(); + var budgetTicks = budget.FirstItem.Ticks; + + // `await using`, declared after watchdogCts so it disposes first: + // ITimer.DisposeAsync waits for any in-flight callback to finish, so the + // callback's Cancel() can never race a disposed token source. The + // callback never touches the timer, so there is no reset/disposal race. await using var timer = timeProvider.CreateTimer( - static state => ((CancellationTokenSource)state!).Cancel(), - watchdogCts, - firstItemBudget, - Timeout.InfiniteTimeSpan); + _ => + { + var allowed = TimeSpan.FromTicks(Volatile.Read(ref budgetTicks)); + if (allowed > TimeSpan.Zero + && timeProvider.GetElapsedTime(Volatile.Read(ref lastActivity)) >= allowed) + { + watchdogCts.Cancel(); + } + }, + state: null, + PollInterval, + PollInterval); - var currentBudget = firstItemBudget; string? result = null; - var enumerator = stream.GetAsyncEnumerator(watchdogCts.Token); try { @@ -64,19 +82,19 @@ public static async Task ConsumeAsync( catch (OperationCanceledException) when (watchdogCts.IsCancellationRequested && !ct.IsCancellationRequested) { + // Watchdog fired (not caller cancellation): report a timeout. + var stalled = TimeSpan.FromTicks(Volatile.Read(ref budgetTicks)); throw new TimeoutException( - $"Tool '{toolName}' produced no activity for " - + $"{currentBudget.TotalSeconds:F0}s and was stopped. It may be stuck — " - + "please try again, or simplify the request."); + $"Tool '{toolName}' produced no activity for {stalled.TotalSeconds:F0}s " + + "and was stopped. It may be stuck — please try again, or simplify the request."); } if (!hasNext) break; - // Any item — activity or completion — is liveness: reset the - // watchdog to the (tighter) inter-item budget. - currentBudget = interItemBudget; - timer.Change(interItemBudget, Timeout.InfiniteTimeSpan); + // Any item is liveness; later items are held to the tighter budget. + Volatile.Write(ref budgetTicks, budget.InterItem.Ticks); + Volatile.Write(ref lastActivity, timeProvider.GetTimestamp()); switch (enumerator.Current) { @@ -94,6 +112,9 @@ public static async Task ConsumeAsync( await enumerator.DisposeAsync(); } - return result ?? $"Tool '{toolName}' completed without producing a result."; + // A stream that ends with no completion item violates the tool-call + // contract — fail loudly rather than synthesizing a result. + return result ?? throw new InvalidOperationException( + $"Tool '{toolName}' stream ended without a completion item."); } } diff --git a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json index 106dd5a11..d2be44d19 100644 --- a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json +++ b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json @@ -133,7 +133,7 @@ "ToolExecutionTimeoutSeconds": { "type": "integer", "minimum": 1, - "description": "Timeout in seconds for one tool-execution batch." + "description": "Per-tool-call inactivity budget in seconds: a tool call must produce its first result (or stream item) within this time, and each later item resets it." }, "SidecarLlmTimeoutSeconds": { "type": "integer", diff --git a/src/Netclaw.Configuration/SessionConfig.cs b/src/Netclaw.Configuration/SessionConfig.cs index a67597fc4..eb2f746af 100644 --- a/src/Netclaw.Configuration/SessionConfig.cs +++ b/src/Netclaw.Configuration/SessionConfig.cs @@ -49,9 +49,11 @@ public sealed record SessionConfig public TimeSpan TurnLlmTimeout { get; init; } = TimeSpan.FromMinutes(3); /// - /// Timeout for one tool-execution batch (all tool calls emitted - /// by a single assistant response). Prevents indefinite hangs when one or - /// more tools block forever. + /// Per-tool-call inactivity budget. Each tool call is consumed as a stream + /// under its own watchdog: the call must produce its first item within this + /// budget, and each later item resets it. A non-streaming tool emits only + /// its single result, so it must finish within this budget. Enforced per + /// call, so a stuck tool is caught without affecting siblings in the batch. /// public TimeSpan ToolExecutionTimeout { get; init; } = TimeSpan.FromSeconds(90); From e0d5911f29003812ec35e1fec133d8711b33bfc9 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 19:24:19 -0500 Subject: [PATCH 3/7] feat(subagents): stream spawn_agent runs; simplify sub-agent tool policy Phases D and E of the streaming tool-call refactor. Phase D - spawn_agent as a streaming tool: - SubAgentActor's flat run timer becomes an inactivity watchdog, re-armed on every progress event (LLM response, tool batch, and a throttled streaming ping emitted while a long LLM call is in flight). A sub-agent making progress is never killed mid-run; a stalled one is caught. - The sub-agent emits activity items through a channel; SpawnAgentTool overrides ExecuteStreamAsync to surface them as its tool-call stream, so the parent's per-call watchdog sees a long-but-healthy delegated run as alive. The terminal item carries the sub-agent's result. - SubAgentSpawner drops the fixed Ask timeout: the sub-agent self-bounds (inactivity watchdog + tool-iteration cap, always replies) and the spawning tool call's token cancels a fully-wedged run. Phase E - recursion and approval cleanup: - SubAgentToolPolicy becomes a denylist (spawn_agent only): sub-agents inherit the parent session's tool policy instead of a hardcoded safe-list. - ToolAccessPolicy drops the non-interactive safe-list auto-grant; the approval policy is authoritative for every channel, so a non-interactive caller fails closed unless the patterns are already approved. - Discovery text and the affected tests updated for the inheritance model. Build clean; Netclaw.Actors.Tests 1609/1609, Netclaw.Daemon.Tests 557/557, Netclaw.Configuration.Tests 312/312 pass; slopwatch clean. --- .../Tools/ToolApprovalGateTests.cs | 8 +- .../SubAgents/SpawnAgentTool.cs | 100 +++++++++++++++--- src/Netclaw.Actors/SubAgents/SubAgentActor.cs | 62 +++++++++-- .../SubAgents/SubAgentProtocol.cs | 8 ++ .../SubAgents/SubAgentSpawner.cs | 30 ++++-- src/Netclaw.Actors/Tools/ToolAccessPolicy.cs | 14 +-- .../SubAgentDiscoveryContextLayer.cs | 9 +- .../SubAgentToolPolicy.cs | 25 +++-- .../Mcp/ToolIndexUpdaterTests.cs | 3 +- 9 files changed, 198 insertions(+), 61 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs index 1aba3c7aa..449ff6136 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs @@ -494,7 +494,7 @@ public void mcp_tool_without_server_default_falls_through_to_default_mode() // ── Subagent approval gate (SupportsInteractiveApproval=false) ── [Fact] - public void Safe_list_tool_auto_grants_when_interactive_approval_unsupported() + public void Non_interactive_tool_requires_approval_when_policy_requires_approval() { var config = new ToolConfig(); config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig @@ -514,8 +514,10 @@ public void Safe_list_tool_auto_grants_when_interactive_approval_unsupported() var decision = policy.AuthorizeInvocation(tool, subagentCtx); - Assert.True(decision.Allowed); - Assert.False(decision.NeedsApproval); + // No safe-list auto-grant: the approval policy is authoritative for every + // channel, so a non-interactive caller fails closed to requires-approval. + Assert.True(decision.NeedsApproval); + Assert.NotNull(decision.ApprovalContext); } [Fact] diff --git a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs index 4c9f70c72..6fe4bd923 100644 --- a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs +++ b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs @@ -1,9 +1,11 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- using System.ComponentModel; +using System.Runtime.CompilerServices; +using System.Threading.Channels; using Netclaw.Configuration; using Netclaw.Tools; @@ -55,16 +57,90 @@ protected override Task ExecuteAsync(Params args, CancellationToken ct) protected override async Task ExecuteAsync(Params args, ToolExecutionContext context, CancellationToken ct) { - // Defense-in-depth: block subagent spawning for Public audience or when subagent subsystem is disabled - var audience = context.Audience; - if (audience == TrustAudience.Public || !_subAgentConfig.Enabled) - return "Error: This tool is not available."; + var (error, profile) = Resolve(args, context); + if (error is not null) + return error; + + var result = await _spawner.SpawnAsync(profile!, args.Task, args.Context, context, ct); + return result.Success + ? result.Output + : $"Subagent '{args.Agent}' failed: {result.Output}"; + } + + /// + /// Streaming entry point: the sub-agent's liveness/progress activity is + /// surfaced as the tool call's stream, so the parent's per-call watchdog + /// keeps a long-but-healthy delegated run alive. The terminal item carries + /// the sub-agent's final result. + /// + public async IAsyncEnumerable ExecuteStreamAsync( + IDictionary? arguments, + ToolExecutionContext context, + [EnumeratorCancellation] CancellationToken ct = default) + { + Params? args = null; + string? failure = null; + if (arguments is null) + { + failure = $"Error: No arguments provided for tool '{Name}'."; + } + else + { + try + { + args = ParseArguments(arguments); + } + catch (Exception ex) + { + failure = $"Error parsing arguments for tool '{Name}': {ex.Message}"; + } + } + + SubAgentProfile? profile = null; + if (failure is null) + { + (failure, profile) = Resolve(args!, context); + } + + if (failure is not null) + { + yield return new ToolCompletedUpdate(failure); + yield break; + } + + // The spawner writes the sub-agent's activity into this channel and + // completes it (even on failure) when the run ends. + var channel = Channel.CreateUnbounded(); + var spawnTask = _spawner.SpawnAsync( + profile!, args!.Task, args.Context, context, ct, activitySink: channel.Writer); + + await foreach (var activity in channel.Reader.ReadAllAsync(ct)) + yield return activity; + + var result = await spawnTask; + yield return new ToolCompletedUpdate( + result.Success + ? result.Output + : $"Subagent '{args.Agent}' failed: {result.Output}"); + } + + /// + /// Validate the invocation and resolve the requested agent. Returns an error + /// string (and a null profile) when the spawn must be refused, otherwise a + /// null error and the resolved profile. + /// + private (string? Error, SubAgentProfile? Profile) Resolve(Params args, ToolExecutionContext context) + { + // Defense-in-depth: block subagent spawning for Public audience or when + // the subagent subsystem is disabled. + if (context.Audience == TrustAudience.Public || !_subAgentConfig.Enabled) + return ("Error: This tool is not available.", null); if (string.IsNullOrWhiteSpace(args.Agent)) - return "Error: 'agent' parameter is required."; + return ("Error: 'agent' parameter is required.", null); if (string.IsNullOrWhiteSpace(args.Task)) - return "Error: 'task' parameter is required."; + return ("Error: 'task' parameter is required.", null); _loader?.SyncInto(_registry); @@ -73,16 +149,12 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon { var available = _registry.GetUserFacing(); if (available.Count == 0) - return $"Error: No subagents are available. Agent '{args.Agent}' not found. Author one at {_paths.AgentsDirectory}/*.md or define a skill with metadata.subagent once #661 lands."; + return ($"Error: No subagents are available. Agent '{args.Agent}' not found. Author one at {_paths.AgentsDirectory}/*.md or define a skill with metadata.subagent once #661 lands.", null); var names = string.Join(", ", available.Select(a => a.Name)); - return $"Error: Unknown agent '{args.Agent}'. Available agents: {names}"; + return ($"Error: Unknown agent '{args.Agent}'. Available agents: {names}", null); } - var result = await _spawner.SpawnAsync(profile, args.Task, args.Context, context!, ct); - - return result.Success - ? result.Output - : $"Subagent '{args.Agent}' failed: {result.Output}"; + return (null, profile); } } diff --git a/src/Netclaw.Actors/SubAgents/SubAgentActor.cs b/src/Netclaw.Actors/SubAgents/SubAgentActor.cs index 6dd64844e..6f0a957b7 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentActor.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentActor.cs @@ -3,7 +3,9 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Diagnostics; using System.Text; +using System.Threading.Channels; using Akka.Actor; using Akka.Event; using Microsoft.Extensions.AI; @@ -32,6 +34,7 @@ public sealed class SubAgentActor : ReceiveActor, IWithTimers private const int MaxToolIterations = 10; private const string EmptyResponseMarker = "(no response)"; private const string TimeoutTimerKey = "subagent-timeout"; + private static readonly TimeSpan StreamPingInterval = TimeSpan.FromSeconds(2); private readonly SubAgentDefinition _definition; private readonly IChatClient _chatClient; @@ -48,6 +51,8 @@ public sealed class SubAgentActor : ReceiveActor, IWithTimers private IActorRef _replyTo = ActorRefs.Nobody; private CancellationTokenSource? _executionCts; private IParentApprovalBridge? _approvalBridge; + private ChannelWriter? _activitySink; + private TimeSpan _inactivityBudget; public ITimerScheduler Timers { get; set; } = null!; private CancellationTokenRegistration _externalCancellationRegistration; @@ -134,8 +139,12 @@ private void Idle() var self = Self; // Capture before callback — Self requires active actor context _externalCancellationRegistration = msg.Cancellation.Register(() => self.Tell(SubAgentCancelled.Instance)); - // Schedule wall-clock timeout - Timers.StartSingleTimer(TimeoutTimerKey, SubAgentTimeout.Instance, msg.Timeout); + // The run is bounded by an inactivity watchdog re-armed on every + // progress event (LLM response, tool batch, streaming ping), so a + // sub-agent making progress is never killed and a stalled one is. + _activitySink = msg.ActivitySink; + _inactivityBudget = msg.Timeout; + ArmInactivityTimer(); // Build initial conversation: system prompt (from file, verbatim) + task as user message. // If the caller supplied runtime context, prefix it onto the user message so the @@ -155,6 +164,7 @@ private void Processing() { Receive(msg => { + ArmInactivityTimer(); var response = msg.Response; var lastMessage = response.Messages[^1]; @@ -184,6 +194,8 @@ private void Processing() Receive(msg => { + RecordProgress("processing tool results"); + // Append tool results as MEAI messages and log each result foreach (var result in msg.ToolResults) { @@ -232,10 +244,15 @@ private void Processing() Receive(_ => { _executionCts?.Cancel(); - _log.Warning("SubAgent [{AgentName}] timed out after {Iterations} tool iterations", - _definition.Name, _toolIterationCount); - Complete(false, "Subagent timed out"); + _log.Warning( + "SubAgent [{AgentName}] timed out: no activity for {Budget}s after {Iterations} tool iterations", + _definition.Name, _inactivityBudget.TotalSeconds, _toolIterationCount); + Complete(false, $"Subagent timed out: no activity for {_inactivityBudget.TotalSeconds:F0}s."); }); + + // Throttled liveness ping from a streaming LLM call — progress, even + // before the full response message arrives. + Receive(_ => RecordProgress("the model is responding")); } private void HandleToolCalls(AiChatMessage assistantMessage, List toolCalls) @@ -244,6 +261,7 @@ private void HandleToolCalls(AiChatMessage assistantMessage, List tc.Name)); + RecordProgress($"running tools: {toolNames}"); _log.Info("SubAgent [{AgentName}] calling tools: [{ToolNames}]", _definition.Name, toolNames); @@ -261,6 +279,7 @@ private void HandleToolCalls(AiChatMessage assistantMessage, List(_history); @@ -305,6 +324,21 @@ private void Complete(bool success, string output) Context.Stop(Self); } + /// Re-arm the inactivity watchdog (Akka replaces the same-key timer). + private void ArmInactivityTimer() + => Timers.StartSingleTimer(TimeoutTimerKey, SubAgentTimeout.Instance, _inactivityBudget); + + /// Emit a liveness/progress item to the spawning tool's stream, if any. + private void EmitActivity(string phase) + => _activitySink?.TryWrite(new ToolActivityUpdate(phase)); + + /// Record forward progress: re-arm the inactivity watchdog and emit activity. + private void RecordProgress(string phase) + { + ArmInactivityTimer(); + EmitActivity(phase); + } + private List BuildFindings(string output, string? sessionId) { var content = output?.Trim() ?? string.Empty; @@ -395,9 +429,18 @@ internal static async Task InvokeLlmAsync( // only in streaming mode), leaving the assistant message with no // TextContent and causing the subagent to report empty "(no response)". var updates = new List(); + var pingThrottle = Stopwatch.StartNew(); await foreach (var update in client.GetStreamingResponseAsync(messages, options, ct)) { updates.Add(update); + + // Throttled liveness ping so the actor re-arms its inactivity + // watchdog and surfaces activity during a long streaming call. + if (pingThrottle.Elapsed >= StreamPingInterval) + { + pingThrottle.Restart(); + self.Tell(SubAgentStreamPing.Instance); + } } var response = updates.ToChatResponse(); @@ -531,7 +574,7 @@ private static string BuildSystemPrompt(SubAgentDefinition definition) return SystemPromptAssembler.Assemble(agents: definition.SystemPrompt, projectInstructions: definition.ProjectInstructions); } - /// Singleton timeout marker message. + /// Singleton inactivity-watchdog marker message. private sealed class SubAgentTimeout { public static readonly SubAgentTimeout Instance = new(); @@ -544,6 +587,13 @@ private sealed class SubAgentCancelled private SubAgentCancelled() { } } + /// Throttled self-message: the streaming LLM call is still producing output. + private sealed class SubAgentStreamPing + { + public static readonly SubAgentStreamPing Instance = new(); + private SubAgentStreamPing() { } + } + // ── Reuse LlmSessionActor's internal message types ── // These are internal to Netclaw.Actors so accessible here. diff --git a/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs b/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs index 71ba01688..f76fd8e17 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Threading.Channels; using Akka.Actor; using Microsoft.Extensions.AI; using Netclaw.Configuration; @@ -98,6 +99,13 @@ public sealed record RunSubAgent : INoSerializationVerificationNeeded /// approval requests back to the interactive user instead of auto-denying. /// public IParentApprovalBridge? ApprovalBridge { get; init; } + + /// + /// Optional sink for liveness/progress activity emitted while the sub-agent + /// runs. The spawning tool surfaces these as its tool-call stream so the + /// parent's per-call watchdog sees a long-but-healthy run as alive. + /// + public ChannelWriter? ActivitySink { get; init; } } /// diff --git a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs index 1f826eae8..4e1ce62a1 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using System.Diagnostics; +using System.Threading.Channels; using Akka.Actor; using Microsoft.Extensions.Logging; using Netclaw.Actors.Tools; @@ -57,7 +58,8 @@ public async Task SpawnAsync( string? runtimeContext, ToolExecutionContext context, CancellationToken ct = default, - string? systemPromptOverlay = null) + string? systemPromptOverlay = null, + ChannelWriter? activitySink = null) { if (context.SpawnChildActor is null) { @@ -130,9 +132,14 @@ public async Task SpawnAsync( ParentSessionDirectory = context.SessionDirectory, ParentProjectDirectory = context.ProjectDirectory, Cancellation = ct, - ApprovalBridge = context.ApprovalBridge + ApprovalBridge = context.ApprovalBridge, + ActivitySink = activitySink }, - timeout: subAgentTimeout.Add(TimeSpan.FromSeconds(5)), + // No Ask timeout: the sub-agent self-bounds via its inactivity + // watchdog and tool-iteration cap (it always replies), and ct — + // the spawning tool call's token, governed by the parent's + // per-call watchdog — cancels a fully-wedged run. + timeout: Timeout.InfiniteTimeSpan, cancellationToken: ct); sw.Stop(); @@ -176,12 +183,15 @@ public async Task SpawnAsync( AgentName = new AgentName(profile.Name) }; } + finally + { + // Terminate the streaming caller's activity reader even on failure. + activitySink?.TryComplete(); + } } private IReadOnlyList ResolveTools(SubAgentProfile profile) { - var isUserFacing = profile.Visibility == SubAgentVisibility.UserFacing; - // When no tools specified, inherit all registered tools (matches Claude Code behavior). // When tools are specified, use them as a whitelist to limit access. IEnumerable candidates; @@ -210,21 +220,19 @@ private IReadOnlyList ResolveTools(SubAgentProfile profile) candidates = resolved; } - if (!isUserFacing) - return candidates.ToList(); - - // User-facing subagents are restricted to SubAgentToolPolicy's safe list. + // Sub-agents inherit the parent session's runtime tool policy; the only + // static sub-agent-specific filter denies recursive spawn_agent delegation. var tools = new List(); foreach (var tool in candidates) { - if (SubAgentToolPolicy.IsAllowedForUserFacing(tool.Name)) + if (SubAgentToolPolicy.IsAllowedForSubAgent(tool.Name)) { tools.Add(tool); } else { _logger.LogDebug( - "SubAgent [{AgentName}] tool '{ToolName}' filtered by SubAgentToolPolicy", + "SubAgent [{AgentName}] tool '{ToolName}' denied by SubAgentToolPolicy", profile.Name, tool.Name); } } diff --git a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs index 64efd5ead..12ce597e1 100644 --- a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs +++ b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs @@ -293,15 +293,11 @@ private ToolAccessDecision CheckApprovalGate( if (mode == ToolApprovalMode.Auto) return ToolAccessDecision.Allow(); - // Non-interactive channels (reminders, webhooks, sub-agents without parent - // approval channel): tools on the safe list are auto-granted. Everything else - // falls through to the normal approval extraction path — the executor will - // check the persistent approval store and allow if all patterns are pre-approved. - if (context?.SupportsInteractiveApproval == false - && SubAgentToolPolicy.IsAllowedForUserFacing(toolName.Value)) - { - return ToolAccessDecision.Allow(); - } + // The approval policy is authoritative for every channel — there is no + // safe-list auto-grant for non-interactive callers. A non-interactive + // caller (reminder, webhook, sub-agent without an approval bridge) that + // hits an approval-gated tool fails closed unless the patterns are + // already in the persistent approval store. // Approval prompts carry three views of the invocation: // - `patterns`: the exact blocked units shown to the user and reused by diff --git a/src/Netclaw.Configuration/SubAgentDiscoveryContextLayer.cs b/src/Netclaw.Configuration/SubAgentDiscoveryContextLayer.cs index ab98b3a80..fd8f1232f 100644 --- a/src/Netclaw.Configuration/SubAgentDiscoveryContextLayer.cs +++ b/src/Netclaw.Configuration/SubAgentDiscoveryContextLayer.cs @@ -54,14 +54,15 @@ internal static string BuildIndex(IReadOnlyList agents, string { if (agents.Count == 0) { - var allowedTools = string.Join(", ", SubAgentToolPolicy.GetAllowedUserFacingTools()); + var deniedTools = string.Join(", ", SubAgentToolPolicy.GetDeniedSubAgentTools()); return string.Join('\n', [ "[available-subagents — use spawn_agent to delegate]", string.Empty, "No user-facing subagents are currently registered.", $"Agents directory: {agentsDirectory}", - $"Allowed tools for user-facing agents: {allowedTools}", + "Sub-agents inherit the parent session's tool policy.", + $"Denied tools for sub-agents: {deniedTools}", string.Empty, "To add one: create an agent definition at /.md. The next turn or subagent lookup reloads it automatically.", "Then call `spawn_agent(agent: \"\", task: \"\", context: \"\")`." @@ -79,8 +80,8 @@ internal static string BuildIndex(IReadOnlyList agents, string lines.Add($"## {agent.Name}"); lines.Add(agent.Description); lines.Add(agent.ToolNames.Count == 0 - ? "Tools: all registered tools, then filtered for user-facing safety" - : $"Tools: {string.Join(", ", agent.ToolNames)}"); + ? "Tools: inherited from parent session policy, except denied sub-agent tools" + : $"Tools: {string.Join(", ", agent.ToolNames)} (except denied sub-agent tools)"); lines.Add($"Timeout: {agent.TimeoutSeconds}s"); lines.Add(string.Empty); } diff --git a/src/Netclaw.Configuration/SubAgentToolPolicy.cs b/src/Netclaw.Configuration/SubAgentToolPolicy.cs index 9317dddf2..4cca99422 100644 --- a/src/Netclaw.Configuration/SubAgentToolPolicy.cs +++ b/src/Netclaw.Configuration/SubAgentToolPolicy.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -6,23 +6,22 @@ namespace Netclaw.Configuration; /// -/// Central policy for tools exposed to user-facing subagents. -/// File-authored agents are intentionally constrained to a conservative, -/// read-oriented tool set so they do not bypass the main session's safety model. +/// Central policy for tools exposed to sub-agents. Sub-agents inherit the parent +/// session's audience, boundary, approval, and shell policies; this static deny +/// list only prevents recursive delegation loops. /// public static class SubAgentToolPolicy { - private static readonly HashSet SafeUserFacingToolNames = new(StringComparer.Ordinal) + private static readonly HashSet DeniedSubAgentToolNames = new(StringComparer.Ordinal) { - "attach_file", - "file_read", - "web_fetch", - "web_search" + "spawn_agent" }; - public static bool IsAllowedForUserFacing(string toolName) - => SafeUserFacingToolNames.Contains(toolName); + /// True unless the tool is statically denied to sub-agents. + public static bool IsAllowedForSubAgent(string toolName) + => !DeniedSubAgentToolNames.Contains(toolName); - public static IReadOnlyList GetAllowedUserFacingTools() - => SafeUserFacingToolNames.OrderBy(x => x, StringComparer.Ordinal).ToArray(); + /// The tools statically denied to sub-agents, sorted. + public static IReadOnlyList GetDeniedSubAgentTools() + => DeniedSubAgentToolNames.OrderBy(x => x, StringComparer.Ordinal).ToArray(); } diff --git a/src/Netclaw.Daemon.Tests/Mcp/ToolIndexUpdaterTests.cs b/src/Netclaw.Daemon.Tests/Mcp/ToolIndexUpdaterTests.cs index 0a96ef932..920c1ade1 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/ToolIndexUpdaterTests.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/ToolIndexUpdaterTests.cs @@ -48,7 +48,8 @@ public async Task StartAsync_with_no_user_facing_agents_sets_actionable_discover Assert.Contains("available-subagents", discovery, StringComparison.OrdinalIgnoreCase); Assert.Contains(paths.AgentsDirectory, discovery, StringComparison.Ordinal); - foreach (var tool in SubAgentToolPolicy.GetAllowedUserFacingTools()) + Assert.Contains("Sub-agents inherit the parent session's tool policy", discovery, StringComparison.Ordinal); + foreach (var tool in SubAgentToolPolicy.GetDeniedSubAgentTools()) Assert.Contains(tool, discovery, StringComparison.Ordinal); } From 31933656c5e82dc0560a2527f7622a76575e0a3d Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 19:33:06 -0500 Subject: [PATCH 4/7] fix(subagents): complete activity channel on early spawn failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review cleanup of phases D-E. - SubAgentSpawner.SpawnAsync's early-return paths (no session context, no resolvable tools) bypassed the finally that completes the activity channel, leaving SpawnAgentTool.ExecuteStreamAsync's reader hung until the per-call watchdog timed out — a misleading "stalled" error instead of the real failure. Complete the channel before those returns. - SpawnAgentTool.ExecuteStreamAsync awaits the spawn task in a finally, so the sub-agent is observed and stopped even when enumeration is abandoned by cancellation. - Extract NetclawTool.TryParse so the streaming and string-returning paths share one argument-parse and error-wording implementation. - Extract SpawnAgentTool.FormatResult; both execution paths use it. - Correct the SubAgentSpawner Ask-timeout comment: a healthy run is inactivity-bounded, not wall-clock-bounded. Build clean; Netclaw.Actors.Tests 1609/1609, Netclaw.Daemon.Tests 557/557, Netclaw.Configuration.Tests 312/312 pass; slopwatch clean. --- .../SubAgents/SpawnAgentTool.cs | 50 +++++++------------ .../SubAgents/SubAgentSpawner.cs | 12 +++-- src/Netclaw.Tools.Abstractions/NetclawTool.cs | 20 +++++--- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs index 6fe4bd923..82cf8954a 100644 --- a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs +++ b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs @@ -62,45 +62,24 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon return error; var result = await _spawner.SpawnAsync(profile!, args.Task, args.Context, context, ct); - return result.Success - ? result.Output - : $"Subagent '{args.Agent}' failed: {result.Output}"; + return FormatResult(args.Agent, result); } /// /// Streaming entry point: the sub-agent's liveness/progress activity is /// surfaced as the tool call's stream, so the parent's per-call watchdog - /// keeps a long-but-healthy delegated run alive. The terminal item carries - /// the sub-agent's final result. + /// keeps a long-but-healthy delegated run alive. /// public async IAsyncEnumerable ExecuteStreamAsync( IDictionary? arguments, ToolExecutionContext context, [EnumeratorCancellation] CancellationToken ct = default) { - Params? args = null; - string? failure = null; - if (arguments is null) - { - failure = $"Error: No arguments provided for tool '{Name}'."; - } - else - { - try - { - args = ParseArguments(arguments); - } - catch (Exception ex) - { - failure = $"Error parsing arguments for tool '{Name}': {ex.Message}"; - } - } + var (failure, args) = TryParse(arguments); SubAgentProfile? profile = null; if (failure is null) - { (failure, profile) = Resolve(args!, context); - } if (failure is not null) { @@ -113,17 +92,24 @@ public async IAsyncEnumerable ExecuteStreamAsync( var channel = Channel.CreateUnbounded(); var spawnTask = _spawner.SpawnAsync( profile!, args!.Task, args.Context, context, ct, activitySink: channel.Writer); + try + { + await foreach (var activity in channel.Reader.ReadAllAsync(ct)) + yield return activity; - await foreach (var activity in channel.Reader.ReadAllAsync(ct)) - yield return activity; - - var result = await spawnTask; - yield return new ToolCompletedUpdate( - result.Success - ? result.Output - : $"Subagent '{args.Agent}' failed: {result.Output}"); + yield return new ToolCompletedUpdate(FormatResult(args.Agent, await spawnTask)); + } + finally + { + // Observe the spawn even when enumeration is abandoned (cancellation), + // so the sub-agent is stopped before this tool call returns. + await spawnTask; + } } + private static string FormatResult(string agent, SubAgentResult result) + => result.Success ? result.Output : $"Subagent '{agent}' failed: {result.Output}"; + /// /// Validate the invocation and resolve the requested agent. Returns an error /// string (and a null profile) when the spawn must be refused, otherwise a diff --git a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs index 4e1ce62a1..4f6dff8bb 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs @@ -64,6 +64,7 @@ public async Task SpawnAsync( if (context.SpawnChildActor is null) { _logger.LogWarning("SubAgent [{AgentName}] cannot spawn — no session context available", profile.Name); + activitySink?.TryComplete(); return new SubAgentResult { Success = false, @@ -76,6 +77,7 @@ public async Task SpawnAsync( if (tools.Count == 0) { _logger.LogWarning("SubAgent [{AgentName}] has no resolvable tools — cannot spawn", profile.Name); + activitySink?.TryComplete(); return new SubAgentResult { Success = false, @@ -135,10 +137,12 @@ public async Task SpawnAsync( ApprovalBridge = context.ApprovalBridge, ActivitySink = activitySink }, - // No Ask timeout: the sub-agent self-bounds via its inactivity - // watchdog and tool-iteration cap (it always replies), and ct — - // the spawning tool call's token, governed by the parent's - // per-call watchdog — cancels a fully-wedged run. + // No Ask timeout: a healthy run is inactivity-bounded, not + // wall-clock-bounded (like the parent LLM session), so any finite + // ceiling could pre-empt a legitimately long run. A stalled run + // self-completes via the sub-agent's inactivity watchdog; a wedged + // run is cancelled through ct — the spawning tool call's token, + // governed by the parent's per-call watchdog. timeout: Timeout.InfiniteTimeSpan, cancellationToken: ct); diff --git a/src/Netclaw.Tools.Abstractions/NetclawTool.cs b/src/Netclaw.Tools.Abstractions/NetclawTool.cs index 922133b91..a08e6107e 100644 --- a/src/Netclaw.Tools.Abstractions/NetclawTool.cs +++ b/src/Netclaw.Tools.Abstractions/NetclawTool.cs @@ -55,21 +55,29 @@ public async Task ExecuteAsync(IDictionary? arguments, /// public async Task ExecuteAsync(IDictionary? arguments, ToolExecutionContext context, CancellationToken ct = default) + { + var (error, args) = TryParse(arguments); + return error is not null ? error : await ExecuteAsync(args!, context, ct); + } + + /// + /// Deserialize raw LLM arguments, returning a tool-result error string + /// instead of throwing. Shared by the string-returning and streaming + /// execution paths so their argument-error wording cannot drift. + /// + protected (string? Error, TParams? Args) TryParse(IDictionary? arguments) { if (arguments is null) - return $"Error: No arguments provided for tool '{Name}'."; + return ($"Error: No arguments provided for tool '{Name}'.", null); - TParams args; try { - args = ParseArguments(arguments); + return (null, ParseArguments(arguments)); } catch (Exception ex) { - return $"Error parsing arguments for tool '{Name}': {ex.Message}"; + return ($"Error parsing arguments for tool '{Name}': {ex.Message}", null); } - - return await ExecuteAsync(args, context, ct); } // Partial method — implemented by the source generator From f5a7fb949de72c10d75b73ae7030f697718777a1 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 20:03:15 -0500 Subject: [PATCH 5/7] test(tools): streaming tool-call watchdog and parallel-isolation coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase G tests for the streaming tool-call refactor. - StreamingToolCallTests: FakeTimeProvider-driven unit tests for StreamingToolWatchdog — first-item and inter-item budget timeouts, activity resetting the budget, the missing-completion-item contract, independent per-call bounding, and the INetclawTool non-streaming default adapter (one terminal completion item). - SessionToolExecutionPipelineTests: a stalled tool call is timed out by its own per-call watchdog without faulting a healthy sibling — both produce a tool-result message and the batch is not failed wholesale. - TestStreamingHelpers.ParkUntilCancelledAsync: shared cancellation-gated park idiom for the new streaming test fakes. Netclaw.Actors.Tests 1617/1617 pass; build clean; slopwatch clean. --- .../Pipelines/StreamingToolCallTests.cs | 170 ++++++++++++++++++ .../SessionToolExecutionPipelineTests.cs | 74 ++++++++ .../Sessions/TestStreamingHelpers.cs | 12 ++ 3 files changed, 256 insertions(+) create mode 100644 src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs diff --git a/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs b/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs new file mode 100644 index 000000000..039fd73c5 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs @@ -0,0 +1,170 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Runtime.CompilerServices; +using System.Threading.Channels; +using Microsoft.Extensions.Time.Testing; +using Netclaw.Actors.Sessions.Pipelines; +using Netclaw.Actors.Tests.Memory; +using Netclaw.Actors.Tests.Sessions; +using Netclaw.Tools; +using Xunit; + +namespace Netclaw.Actors.Tests.Sessions.Pipelines; + +/// +/// Unit tests for the streaming tool-call contract: the per-call +/// and the INetclawTool default +/// streaming adapter. Time is virtualized with +/// so the watchdog's timeout behavior is deterministic. +/// +public sealed class StreamingToolCallTests +{ + private static readonly ToolWatchdogBudget FiveSeconds = ToolWatchdogBudget.Flat(TimeSpan.FromSeconds(5)); + + [Fact] + public async Task First_item_budget_trips_when_no_item_arrives() + { + var time = new FakeTimeProvider(); + var task = StreamingToolWatchdog.ConsumeAsync( + StallAsync(TestContext.Current.CancellationToken), "stall_tool", FiveSeconds, time, onActivity: null, TestContext.Current.CancellationToken); + + time.Advance(TimeSpan.FromSeconds(6)); + + var ex = await Assert.ThrowsAsync(() => task); + Assert.Contains("stall_tool", ex.Message); + } + + [Fact] + public async Task Inter_item_budget_trips_after_activity_then_stall() + { + var time = new FakeTimeProvider(); + var channel = Channel.CreateUnbounded(); + var activitySeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var task = StreamingToolWatchdog.ConsumeAsync( + channel.Reader.ReadAllAsync(TestContext.Current.CancellationToken), + "stream_tool", + new ToolWatchdogBudget(TimeSpan.FromSeconds(100), TimeSpan.FromSeconds(5)), + time, + onActivity: _ => activitySeen.TrySetResult(), + TestContext.Current.CancellationToken); + + channel.Writer.TryWrite(new ToolActivityUpdate("working")); + await activitySeen.Task; + + // The first item switched the budget to the tighter inter-item value; + // the stream then goes silent. + time.Advance(TimeSpan.FromSeconds(6)); + + await Assert.ThrowsAsync(() => task); + } + + [Fact] + public async Task Healthy_stream_returns_the_terminal_result() + { + var time = new FakeTimeProvider(); + + var result = await StreamingToolWatchdog.ConsumeAsync( + CompletingAsync("done"), "ok_tool", FiveSeconds, time, onActivity: null, TestContext.Current.CancellationToken); + + Assert.Equal("done", result); + } + + [Fact] + public async Task Stream_without_a_completion_item_throws() + { + var time = new FakeTimeProvider(); + var task = StreamingToolWatchdog.ConsumeAsync( + ActivityOnlyAsync(), "no_result_tool", FiveSeconds, time, onActivity: null, TestContext.Current.CancellationToken); + + await Assert.ThrowsAsync(() => task); + } + + [Fact] + public async Task Activity_within_budget_keeps_the_call_alive() + { + var time = new FakeTimeProvider(); + var channel = Channel.CreateUnbounded(); + var activitySeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var task = StreamingToolWatchdog.ConsumeAsync( + channel.Reader.ReadAllAsync(TestContext.Current.CancellationToken), + "slow_tool", + FiveSeconds, + time, + onActivity: _ => activitySeen.TrySetResult(), + TestContext.Current.CancellationToken); + + // Activity at 3s (within the 5s budget) resets the watchdog... + time.Advance(TimeSpan.FromSeconds(3)); + channel.Writer.TryWrite(new ToolActivityUpdate("still working")); + await activitySeen.Task; + + // ...so another 3s — 6s total, but only 3s since the reset — does not trip it. + time.Advance(TimeSpan.FromSeconds(3)); + channel.Writer.TryWrite(new ToolCompletedUpdate("finished")); + channel.Writer.Complete(); + + Assert.Equal("finished", await task); + } + + [Fact] + public async Task Concurrent_calls_are_bounded_independently() + { + var time = new FakeTimeProvider(); + + var healthy = StreamingToolWatchdog.ConsumeAsync( + CompletingAsync("healthy"), "healthy_tool", FiveSeconds, time, onActivity: null, TestContext.Current.CancellationToken); + var stalled = StreamingToolWatchdog.ConsumeAsync( + StallAsync(TestContext.Current.CancellationToken), "stalled_tool", FiveSeconds, time, onActivity: null, TestContext.Current.CancellationToken); + + // The healthy call returns its real result, unaffected by the stalled sibling. + Assert.Equal("healthy", await healthy); + + time.Advance(TimeSpan.FromSeconds(6)); + await Assert.ThrowsAsync(() => stalled); + } + + [Fact] + public async Task Non_streaming_tool_yields_one_terminal_completion_item() + { + // A tool that does not override ExecuteStreamAsync inherits the + // INetclawTool default: exactly one terminal completion item. + INetclawTool tool = new FakeNetclawTool("greet", "hello there"); + + var updates = new List(); + await foreach (var update in tool.ExecuteStreamAsync( + new Dictionary(), ToolExecutionContext.Empty, TestContext.Current.CancellationToken)) + { + updates.Add(update); + } + + var completed = Assert.IsType(Assert.Single(updates)); + Assert.Equal("hello there", completed.Result); + } + + private static async IAsyncEnumerable StallAsync( + [EnumeratorCancellation] CancellationToken ct = default) + { + // The stream deliberately produces no item: the per-call watchdog is + // the only thing that can end it. + await TestStreamingHelpers.ParkUntilCancelledAsync(ct); + yield break; + } + + private static async IAsyncEnumerable CompletingAsync(string result) + { + await Task.Yield(); + yield return new ToolActivityUpdate("working"); + yield return new ToolCompletedUpdate(result); + } + + private static async IAsyncEnumerable ActivityOnlyAsync() + { + await Task.Yield(); + yield return new ToolActivityUpdate("working"); + } +} diff --git a/src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs b/src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs index 1b8f8019c..82d007011 100644 --- a/src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Runtime.CompilerServices; using Akka.Actor; using Akka.Hosting; using Akka.Hosting.TestKit; @@ -183,6 +184,79 @@ await probe.ExpectMsgAsync( await pipelineTask.WaitAsync(TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); } + [Fact] + public async Task A_stalled_tool_call_times_out_without_failing_its_healthy_sibling() + { + var executor = new ParallelStreamingExecutor(); + var probe = CreateTestProbe("parallel-tool-probe"); + + var toolCalls = new List + { + new("call-fast", "fast_tool", new Dictionary()), + new("call-slow", "slow_tool", new Dictionary()) + }; + + var pipelineTask = SessionToolExecutionPipeline.ExecuteToolsAsync( + executor, + toolCalls, + new SessionId("D1/parallel-watchdog-test"), + source: null, + auditLogger: null, + timeProvider: TimeProvider.System, + sessionDir: Path.GetTempPath(), + maxInlineToolResultChars: 4096, + timeout: TimeSpan.FromSeconds(1), + self: probe.Ref, + emitSubAgentOutput: _ => { }, + spawnChildActor: static (_, _, _) => Task.FromResult(new object())); + + // Real-time: the slow tool's per-call watchdog trips ~1-2s in (1s budget + // plus the 1s poll interval). The ceiling stays tight so a regression — + // a watchdog that never fires — surfaces fast rather than hanging. + var completed = await probe.ExpectMsgAsync( + TimeSpan.FromSeconds(8), + cancellationToken: TestContext.Current.CancellationToken); + await pipelineTask.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + // Each call has its own watchdog: the stalled one is timed out + // independently, the healthy one returns, and the batch is not failed + // wholesale — both produce a tool-result message. + Assert.Equal(2, completed.ToolResults.Count); + var fast = completed.ToolResults.Single(r => r.Name == "fast_tool"); + var slow = completed.ToolResults.Single(r => r.Name == "slow_tool"); + Assert.Equal("fast_tool-ok", fast.Content); + Assert.Contains("slow_tool", slow.Content); + Assert.Contains("no activity", slow.Content); + } + + /// + /// Streaming executor: slow_tool never produces an item (its per-call + /// watchdog must time it out); every other tool completes immediately. + /// + private sealed class ParallelStreamingExecutor : IToolExecutor + { + public Task AuthorizeAsync(FunctionCallContent toolCall, ToolExecutionContext? context = null, CancellationToken ct = default) + => Task.CompletedTask; + + public Task ExecuteAsync(FunctionCallContent toolCall, ToolExecutionContext? context = null, CancellationToken ct = default) + => throw new NotSupportedException("ParallelStreamingExecutor is streaming-only."); + + public async IAsyncEnumerable ExecuteStreamAsync( + FunctionCallContent toolCall, + ToolExecutionContext? context = null, + [EnumeratorCancellation] CancellationToken ct = default) + { + if (toolCall.Name == "slow_tool") + { + // Never produces an item — the per-call watchdog must time it out. + await TestStreamingHelpers.ParkUntilCancelledAsync(ct); + } + + await Task.Yield(); + yield return new ToolCompletedUpdate($"{toolCall.Name}-ok"); + } + } + private sealed class ApprovalThenSuccessExecutor : IToolExecutor { private int _attempt; diff --git a/src/Netclaw.Actors.Tests/Sessions/TestStreamingHelpers.cs b/src/Netclaw.Actors.Tests/Sessions/TestStreamingHelpers.cs index 2a540da63..c04097061 100644 --- a/src/Netclaw.Actors.Tests/Sessions/TestStreamingHelpers.cs +++ b/src/Netclaw.Actors.Tests/Sessions/TestStreamingHelpers.cs @@ -18,6 +18,18 @@ public static async IAsyncEnumerable NeverCompletesAsync( yield break; } + /// + /// Completes (as cancelled) only when is cancelled. + /// Lets a test fake park a stream so the consumer's watchdog or cancellation + /// is the only thing that can end it. + /// + public static async Task ParkUntilCancelledAsync(CancellationToken ct) + { + var parked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using (ct.Register(static state => ((TaskCompletionSource)state!).TrySetCanceled(), parked)) + await parked.Task; + } + public static async IAsyncEnumerable ReturnTextAsync( string text, [EnumeratorCancellation] CancellationToken cancellationToken = default) From 1fb954053c79aaf861dac60592611750d42596b0 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 20:40:57 -0500 Subject: [PATCH 6/7] fix(tools): enforce terminal streaming completion --- docs/runbooks/subagents.md | 30 +++++---- docs/spec/configuration.md | 2 +- .../.system/files/subagent-authoring/SKILL.md | 12 ++-- .../specs/netclaw-tools/spec.md | 6 +- .../streaming-tool-call-execution/tasks.md | 64 +++++++++---------- .../Pipelines/StreamingToolCallTests.cs | 23 +++++++ .../Pipelines/StreamingToolWatchdog.cs | 3 +- 7 files changed, 83 insertions(+), 57 deletions(-) diff --git a/docs/runbooks/subagents.md b/docs/runbooks/subagents.md index 7b6d7d3a3..d3c364618 100644 --- a/docs/runbooks/subagents.md +++ b/docs/runbooks/subagents.md @@ -85,18 +85,21 @@ first user message is just the raw task, identical to the pre-context protocol. ### Execution 1. The `spawn_agent` tool resolves the named agent from the definition registry. -2. Tools listed in the agent's definition are resolved from the tool registry - and filtered against `SubAgentToolPolicy` for user-facing agents. +2. Tools listed in the agent's definition are resolved from the tool registry. + Subagents inherit the parent session's runtime tool policy, then + `SubAgentToolPolicy` removes tools that are statically denied to subagents + (`spawn_agent`). 3. A `SubAgentActor` is spawned as a **child of the session actor** (supervised, lifecycle-managed — stops when the session stops). 4. The subagent runs an autonomous LLM loop: call tools, process results, repeat. -5. After at most 10 tool iterations or the configured timeout, the subagent - returns its final text response. +5. After at most 10 tool iterations, a final response, or an inactivity timeout, + the subagent returns its final text response. 6. The main agent receives this response as the `spawn_agent` tool result. Child creation is marshaled back onto the session actor thread, so supervision stays within Akka's actor-thread rules. If the parent tool call is cancelled or -times out, the subagent is cancelled too. +times out, the subagent is cancelled too. The timeout is an inactivity budget: a +responsive subagent is not stopped merely because wall-clock time has elapsed. ### Observability @@ -168,9 +171,9 @@ findings into clear, well-organized summaries. |-------|----------|---------|-------------| | `name` | Yes | — | Unique identifier. Used in `spawn_agent(agent: "")`. Duplicate names across files are rejected with a warning. | | `description` | Yes | — | One-line description shown in the `[available-subagents]` discovery block. | -| `tools` | No | (attempt all, then filter) | List of tool names. When omitted, the runtime starts from all registered tools, then filters user-facing agents through the safe allowlist. When specified, it acts as a whitelist before the same user-facing filter is applied. | +| `tools` | No | (inherit all except denied) | List of tool names. When omitted, the runtime starts from all registered tools available to the parent session, then removes statically denied subagent tools. When specified, it acts as a whitelist before the same denylist is applied. | | `modelRole` | No | `Compaction` | `Compaction` (cheaper/faster) or `Main` (full model). | -| `timeoutSeconds` | No | `60` | Wall-clock timeout in seconds. | +| `timeoutSeconds` | No | `60` | Inactivity timeout in seconds. The watchdog resets when the subagent makes progress. | | `visibility` | No | `user-facing` | `user-facing` (visible to `spawn_agent`) or `internal` (platform-owned, hidden). Accepts both hyphenated and PascalCase. | | `emitStructuredFindings` | No | `false` | When true, successful output becomes a memory-candidate finding for parent-session review. | @@ -211,15 +214,14 @@ ignored at the glob layer and never logged. ### Tool access When `tools` is omitted from the frontmatter, the runtime starts from all -registered tools and then filters user-facing subagents through the safe -allowlist (`attach_file`, `file_read`, `web_fetch`, `web_search`). This keeps -file-authored subagents read-oriented even if the parent session has broader -tool access. +registered tools available under the parent session's audience, boundary, +approval, and shell policies. It then applies the subagent denylist, which +prevents recursive delegation through `spawn_agent`. When `tools` is specified, it acts as a whitelist limiting which tools the -subagent can access before the same user-facing allowlist is applied. Use this -when you want to restrict a subagent to specific capabilities (e.g., read-only -access via `tools: [file_read, web_search]`). +subagent can access before the same subagent denylist and runtime policy checks +are applied. Use this when you want to restrict a subagent to specific +capabilities (e.g., read-only access via `tools: [file_read, web_search]`). Spawned subagents inherit the parent session's `session_dir` and current `project_dir` as read-only grounding. That means file tools resolve against the diff --git a/docs/spec/configuration.md b/docs/spec/configuration.md index e324df913..fe8a3408c 100644 --- a/docs/spec/configuration.md +++ b/docs/spec/configuration.md @@ -167,7 +167,7 @@ Tuning parameters for LLM session behavior. | `MaxToolCallsPerTurn` | int | `30` | Max individual tool calls per turn. At ~75% a budget nudge is injected; at 100% tools are stripped and the model is asked to summarize. | | `SidecarLlmTimeoutSeconds` | int | `90` | Timeout for sidecar LLM calls (title generation, observer summaries, memory extraction). | | `TurnLlmTimeoutSeconds` | int | `180` | Timeout for the primary per-turn LLM streaming call before forcing an error/recovery path. | -| `ToolExecutionTimeoutSeconds` | int | `90` | Timeout for one tool-execution batch before failing the turn safely. | +| `ToolExecutionTimeoutSeconds` | int | `90` | Per-tool-call inactivity budget. A tool must produce its first result or stream item within this time, and each later item resets the budget. | ### Tools diff --git a/feeds/skills/.system/files/subagent-authoring/SKILL.md b/feeds/skills/.system/files/subagent-authoring/SKILL.md index 71261015b..337b607e0 100644 --- a/feeds/skills/.system/files/subagent-authoring/SKILL.md +++ b/feeds/skills/.system/files/subagent-authoring/SKILL.md @@ -3,7 +3,7 @@ name: subagent-authoring description: "How to create and troubleshoot file-defined subagents in ~/.netclaw/agents. Load when the user asks to add, edit, or debug subagent definitions, or when a skill routes via metadata.subagent." metadata: author: netclaw - version: "1.2.1" + version: "1.2.2" --- # Subagent Authoring @@ -75,9 +75,9 @@ The markdown body below the closing `---` must also be non-empty. | Field | Default | Notes | |------|---------|-------| -| `tools` | (attempt all, then filter) | List of tool names. When omitted, the runtime starts from all registered tools, then filters user-facing agents through the safe allowlist. When specified, it acts as a whitelist before the same filter is applied. | +| `tools` | (inherit all except denied) | List of tool names. When omitted, the runtime starts from all registered tools available to the parent session, then removes statically denied subagent tools. When specified, it acts as a whitelist before the same denylist is applied. | | `modelRole` | `Compaction` | `Main` or `Compaction` (case-insensitive). Invalid values fall back to `Compaction`. | -| `timeoutSeconds` | `60` | Wall-clock timeout for subagent execution. | +| `timeoutSeconds` | `60` | Inactivity timeout for subagent execution. The watchdog resets when the subagent makes progress. | | `visibility` | `user-facing` | Accepts `user-facing`, `UserFacing`, `internal`, or `Internal`. Invalid values fall back to `user-facing`. | | `emitStructuredFindings` | `false` | When true, successful output is emitted as findings for parent-session review. | @@ -106,9 +106,9 @@ Summarize the latest planning notes and highlight next actions. - Follow the user's existing plan format and structure ``` -This agent does not automatically inherit every parent-session tool. User-facing -subagents are filtered to the safe allowlist (`attach_file`, `file_read`, -`web_fetch`, `web_search`) even when `tools` is omitted. +This agent inherits the parent session's runtime tool policy. User-facing +subagents then apply the static subagent denylist, which blocks recursive +delegation through `spawn_agent` even when `tools` is omitted. ## Fail-loud loader behavior diff --git a/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md b/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md index 63fe3a8ea..a5d1b88ea 100644 --- a/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md +++ b/openspec/changes/streaming-tool-call-execution/specs/netclaw-tools/spec.md @@ -5,8 +5,10 @@ Tool execution SHALL be expressed as a stream: an invocation yields an ordered sequence of `ToolCallUpdate` items — zero or more non-terminal *activity* items followed by exactly one terminal *completion* item. The completion item SHALL -carry the tool result and any file attachments and sub-agent outputs the -invocation produced. +carry the tool result text. Invocation side effects that are not part of the +tools-abstractions assembly contract, such as file attachments and sub-agent +outputs, SHALL continue to be collected through the tool execution context and +returned by the session tool-execution pipeline with the terminal result. `INetclawTool` SHALL expose the streaming method as a default interface method whose default implementation yields a single completion item wrapping the tool's diff --git a/openspec/changes/streaming-tool-call-execution/tasks.md b/openspec/changes/streaming-tool-call-execution/tasks.md index f97c27af3..9005c2e1a 100644 --- a/openspec/changes/streaming-tool-call-execution/tasks.md +++ b/openspec/changes/streaming-tool-call-execution/tasks.md @@ -2,74 +2,74 @@ ## Phase A: Streaming contract foundation -- [ ] Add a `ToolCallUpdate` type: a non-terminal activity variant (phase label + - optional output chunk) and a terminal completion variant (result string + - file attachments + completed sub-agent runs + accepted findings) -- [ ] Add `ExecuteStreamAsync` to `INetclawTool` as a default interface method — +- [x] Add a `ToolCallUpdate` type: a non-terminal activity variant (phase label + + optional output chunk) and a terminal completion variant (result string) +- [x] Add `ExecuteStreamAsync` to `INetclawTool` as a default interface method — default body yields one terminal completion item wrapping the existing `ExecuteAsync(arguments, context, ct)` -- [ ] Add `ExecuteStreamAsync` to `IToolExecutor`; implement in +- [x] Add `ExecuteStreamAsync` to `IToolExecutor`; implement in `DispatchingToolExecutor` (authorize, resolve, surface the tool's stream, redact secrets per item, log) -- [ ] Verify build clean (0 warnings); the ~28 `NetclawTool` tools and +- [x] Verify build clean (0 warnings); the ~28 `NetclawTool` tools and `McpToolAdapter` / `AIToolAdapter` compile with no change - **Acceptance:** a tool that does not override `ExecuteStreamAsync` produces exactly one terminal completion item carrying its current result ## Phase B: Per-call streaming watchdog -- [ ] Add a two-phase `StreamingToolWatchdog` helper (first-item budget + +- [x] Add a two-phase `StreamingToolWatchdog` helper (first-item budget + inter-item budget that resets on each item), `TimeProvider`-driven, with no Akka or actor dependency -- [ ] Switch `SessionToolExecutionPipeline.ExecuteSingleToolAsync` to consume +- [x] Switch `SessionToolExecutionPipeline.ExecuteSingleToolAsync` to consume `ExecuteStreamAsync` under the per-call watchdog; remove the flat `CancelAfter` in `ExecuteToolAttemptAsync` -- [ ] On budget expiry: cancel the call's token; yield a terminal error result +- [x] On budget expiry: cancel the call's token; yield a terminal error result naming the tool and the timeout (keyed to the tool-call id) -- [ ] Add first-item and inter-item tool inactivity budgets to `SessionConfig`, - `RawSessionConfig`, and `BindFromConfiguration` -- [ ] Update `src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json` - with the new properties (defaults included; schema-sync rule) -- [ ] Verify build; `Task.WhenAll` over independent tool calls still drives +- [x] Keep `SessionConfig.ToolExecutionTimeoutSeconds` as the single per-call + inactivity budget used for both first-item and inter-item phases +- [x] Update `src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json` + with the revised per-call inactivity description +- [x] Verify build; `Task.WhenAll` over independent tool calls still drives `ExecuteToolsAsync` - **Acceptance:** a stalled stream trips the inter-item budget; a slow first item trips the first-item budget; a healthy parallel call is unaffected ## Phase C: Revert ProcessingWatchdog to LLM-only -- [ ] `LlmSessionActor.HandleToolCallResponse` no longer arms a `ToolExecution` +- [x] `LlmSessionActor.HandleToolCallResponse` no longer arms a `ToolExecution` operation on `ProcessingWatchdog` -- [ ] Remove `ToolExecution`-operation handling, `ProcessingWatchdog.RefreshIfCurrent`, +- [x] Remove `ToolExecution`-operation handling, `ProcessingWatchdog.RefreshIfCurrent`, and `PauseToolExecutionWatchdogForApprovalWait` / `ResumeToolExecutionWatchdogAfterApprovalWait` -- [ ] Confirm `ProcessingWatchdog` governs only `LlmCall` and `Compaction` -- [ ] Verify build; update/trim watchdog tests that asserted tool-execution +- [x] Confirm `ProcessingWatchdog` governs only `LlmCall` and `Compaction` +- [x] Verify build; update/trim watchdog tests that asserted tool-execution watchdog behavior - **Acceptance:** dispatching a tool batch arms no processing-watchdog operation; a long tool call does not trip a session-level timeout ## Phase D: spawn_agent as a streaming tool -- [ ] `SpawnAgentTool` overrides `ExecuteStreamAsync`; route `SubAgentActor` +- [x] `SpawnAgentTool` overrides `ExecuteStreamAsync`; route `SubAgentActor` progress into a `Channel` consumed as the tool's stream -- [ ] `SubAgentSpawner` surfaces the run as a stream instead of a blocking - `Ask`; terminal `SubAgentResult` becomes the completion item -- [ ] `SubAgentActor` keeps its own internal inactivity watchdog; remove the +- [x] `SubAgentSpawner` surfaces the run through an activity channel while still + awaiting the terminal `SubAgentResult` from the child actor; terminal result + becomes the completion item +- [x] `SubAgentActor` keeps its own internal inactivity watchdog; remove the absolute wall-clock backstop and its timer -- [ ] Verify build +- [x] Verify build - **Acceptance:** a `spawn_agent` call emits activity while the sub-agent works; two parallel `spawn_agent` calls with one wedged — the wedged one times out independently, the healthy one returns, both tool-result messages reach the LLM ## Phase E: Recursion and approval cleanup -- [ ] Collapse sub-agent recursion to the single `SubAgentToolPolicy` denylist in +- [x] Collapse sub-agent recursion to the single `SubAgentToolPolicy` denylist in `SubAgentSpawner.ResolveTools`; remove the `spawn_agent` string-compare in the `SubAgentActor` constructor -- [ ] Remove the non-interactive safe-list auto-grant from `ToolAccessPolicy`; +- [x] Remove the non-interactive safe-list auto-grant from `ToolAccessPolicy`; ensure an unapproved tool in a non-interactive session fails closed with a legible error naming the tool and the reason -- [ ] Verify build +- [x] Verify build - **Acceptance:** `spawn_agent` is absent from any resolved sub-agent tool set; a non-interactive sub-agent calling an unapproved tool fails with a legible error @@ -85,17 +85,17 @@ ## Phase G: Tests, docs, eval -- [ ] Unit tests for `StreamingToolWatchdog` with `FakeTimeProvider` — +- [x] Unit tests for `StreamingToolWatchdog` with `FakeTimeProvider` — deterministic, no `Task.Delay` (per `CLAUDE.md` testing rules) -- [ ] Regression test: a non-streaming tool yields exactly one terminal item +- [x] Regression test: a non-streaming tool yields exactly one terminal item - [ ] Integration test: two concurrent `spawn_agent` calls, one wedged — wedged one caught independently, healthy result + timeout error both reach the LLM -- [ ] Integration test: mixed batch `[spawn_agent, hung tool]` — the hung tool is +- [x] Integration test: mixed batch `[healthy tool, hung tool]` — the hung tool is still caught; no whole-batch failure -- [ ] Update `docs/runbooks/subagents.md` and any tool-timeout operator guidance -- [ ] Update the `netclaw-operations` system skill if tool/timeout guidance +- [x] Update `docs/runbooks/subagents.md` and any tool-timeout operator guidance +- [x] Update the affected system skill guidance if tool/sub-agent guidance changed (System Skills Sync Rule) -- [ ] `dotnet slopwatch analyze` — no new violations; `./scripts/Add-FileHeaders.ps1 -Verify` +- [x] `dotnet slopwatch analyze` — no new violations; `./scripts/Add-FileHeaders.ps1 -Verify` - [ ] Run `./evals/run-evals.sh` (tool surface + `SessionConfig` changed) - **Acceptance:** full `Netclaw.Actors.Tests` / `Netclaw.Configuration.Tests` suites pass; eval suite passes; manual repro confirms a heavy `spawn_agent` no diff --git a/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs b/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs index 039fd73c5..ca575fb80 100644 --- a/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs @@ -83,6 +83,22 @@ public async Task Stream_without_a_completion_item_throws() await Assert.ThrowsAsync(() => task); } + [Fact] + public async Task Completion_item_is_terminal_even_if_iterator_does_not_finish() + { + var time = new FakeTimeProvider(); + + var result = await StreamingToolWatchdog.ConsumeAsync( + CompleteThenStallAsync(TestContext.Current.CancellationToken), + "complete_then_stall_tool", + FiveSeconds, + time, + onActivity: null, + TestContext.Current.CancellationToken); + + Assert.Equal("done", result); + } + [Fact] public async Task Activity_within_budget_keeps_the_call_alive() { @@ -167,4 +183,11 @@ private static async IAsyncEnumerable ActivityOnlyAsync() await Task.Yield(); yield return new ToolActivityUpdate("working"); } + + private static async IAsyncEnumerable CompleteThenStallAsync( + [EnumeratorCancellation] CancellationToken ct = default) + { + yield return new ToolCompletedUpdate("done"); + await TestStreamingHelpers.ParkUntilCancelledAsync(ct); + } } diff --git a/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs index 01021fcec..369bf2ec3 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/StreamingToolWatchdog.cs @@ -99,8 +99,7 @@ public static async Task ConsumeAsync( switch (enumerator.Current) { case ToolCompletedUpdate completed: - result = completed.Result; - break; + return completed.Result; case ToolActivityUpdate activity: onActivity?.Invoke(activity); break; From 18f4d72f33d1600b08790c74173e2f1088b0bafc Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 17 May 2026 22:07:09 -0500 Subject: [PATCH 7/7] fix(tools): address streaming review nits --- .../SubAgents/SpawnAgentTool.cs | 6 ++--- .../SubAgents/SubAgentProtocol.cs | 6 ++--- .../SubAgents/SubAgentSpawner.cs | 3 +++ src/Netclaw.Tools.Abstractions/NetclawTool.cs | 25 ++++++++++++++----- .../ToolCallUpdate.cs | 2 +- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs index 82cf8954a..e3bb40d85 100644 --- a/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs +++ b/src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs @@ -75,11 +75,9 @@ public async IAsyncEnumerable ExecuteStreamAsync( ToolExecutionContext context, [EnumeratorCancellation] CancellationToken ct = default) { - var (failure, args) = TryParse(arguments); - SubAgentProfile? profile = null; - if (failure is null) - (failure, profile) = Resolve(args!, context); + if (TryParse(arguments, out var failure, out var args)) + (failure, profile) = Resolve(args, context); if (failure is not null) { diff --git a/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs b/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs index f76fd8e17..dc09aabc4 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentProtocol.cs @@ -102,8 +102,9 @@ public sealed record RunSubAgent : INoSerializationVerificationNeeded /// /// Optional sink for liveness/progress activity emitted while the sub-agent - /// runs. The spawning tool surfaces these as its tool-call stream so the - /// parent's per-call watchdog sees a long-but-healthy run as alive. + /// runs. Streaming spawn_agent calls provide this so the parent's + /// per-call watchdog sees a long-but-healthy run as alive. Non-streaming + /// callers leave it null because no parent stream is observing activity. /// public ChannelWriter? ActivitySink { get; init; } } @@ -132,4 +133,3 @@ public sealed record SubAgentResult : INoSerializationVerificationNeeded /// public int FindingsCount { get; init; } } - diff --git a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs index 4f6dff8bb..302942a65 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs @@ -135,6 +135,9 @@ public async Task SpawnAsync( ParentProjectDirectory = context.ProjectDirectory, Cancellation = ct, ApprovalBridge = context.ApprovalBridge, + // Null for non-streaming callers such as routed skills and + // the legacy ExecuteAsync path. Streaming spawn_agent calls + // pass a real sink so parent tool liveness sees progress. ActivitySink = activitySink }, // No Ask timeout: a healthy run is inactivity-bounded, not diff --git a/src/Netclaw.Tools.Abstractions/NetclawTool.cs b/src/Netclaw.Tools.Abstractions/NetclawTool.cs index a08e6107e..1c229bbb3 100644 --- a/src/Netclaw.Tools.Abstractions/NetclawTool.cs +++ b/src/Netclaw.Tools.Abstractions/NetclawTool.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using Microsoft.Extensions.AI; @@ -56,8 +57,9 @@ public async Task ExecuteAsync(IDictionary? arguments, /// public async Task ExecuteAsync(IDictionary? arguments, ToolExecutionContext context, CancellationToken ct = default) { - var (error, args) = TryParse(arguments); - return error is not null ? error : await ExecuteAsync(args!, context, ct); + return TryParse(arguments, out var error, out var args) + ? await ExecuteAsync(args, context, ct) + : error; } /// @@ -65,18 +67,29 @@ public async Task ExecuteAsync(IDictionary? arguments, /// instead of throwing. Shared by the string-returning and streaming /// execution paths so their argument-error wording cannot drift. /// - protected (string? Error, TParams? Args) TryParse(IDictionary? arguments) + protected bool TryParse( + IDictionary? arguments, + [NotNullWhen(false)] out string? error, + [NotNullWhen(true)] out TParams? args) { if (arguments is null) - return ($"Error: No arguments provided for tool '{Name}'.", null); + { + error = $"Error: No arguments provided for tool '{Name}'."; + args = null; + return false; + } try { - return (null, ParseArguments(arguments)); + error = null; + args = ParseArguments(arguments); + return true; } catch (Exception ex) { - return ($"Error parsing arguments for tool '{Name}': {ex.Message}", null); + error = $"Error parsing arguments for tool '{Name}': {ex.Message}"; + args = null; + return false; } } diff --git a/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs b/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs index 2fd3e4eff..30cb467bb 100644 --- a/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs +++ b/src/Netclaw.Tools.Abstractions/ToolCallUpdate.cs @@ -10,7 +10,7 @@ namespace Netclaw.Tools; /// more non-terminal items followed by exactly /// one terminal . /// -public abstract record ToolCallUpdate; +public interface ToolCallUpdate; /// /// A non-terminal progress/liveness signal emitted while a tool is still