Skip to content

feat(streaming): native tool calls during SSE streaming - #971

Open
vernonstinebaker wants to merge 2 commits into
nullclaw:mainfrom
vernonstinebaker:feat/streaming-native-tools
Open

vernonstinebaker wants to merge 2 commits into
nullclaw:mainfrom
vernonstinebaker:feat/streaming-native-tools

Conversation

@vernonstinebaker

Copy link
Copy Markdown
Contributor

Summary

Decouple native tool-call support from the streaming path so that providers which support native tools during streaming can actually emit them.

Previously the agent loop disabled native tools whenever a stream callback was attached, forcing tools into a prompt-injection format that several models could not reliably emit. Worse, the streaming protocol (StreamChunk / StreamChatResult) had no field to carry tool calls, so even when a provider parsed them from the SSE deltas they were dropped at the agent boundary.

Background

The streaming implementation predates native tool support in the OpenAI-compatible protocol. When streaming was added, the response protocol was text-delta-only (StreamChunk{ delta, is_final, token_count }), and the streaming call site passed .tools = null. The explicit native_tools_enabled = !is_streaming and ... gate was added later (during a logging refactor) and codified a limitation that had already become structural rather than intentional — streaming tool-call deltas have been a documented part of the OpenAI-compatible protocol for years.

This restores provider-protocol parity for the streaming path.

Changes

  • providers/root: add tool_calls to StreamChatResult and a supportsStreamingNativeTools() vtable slot, defaulting to false (conservative — providers must opt in).
  • providers/sse: add a ToolCallAccumulator that parses delta.tool_calls (OpenAI-compatible format, including parallel calls indexed by index) across SSE chunks and emits a complete tool_calls slice on the final StreamChatResult.
  • providers/{openai,compatible}: opt in to streaming native tools.
  • providers/{router,reliable}: delegate the capability conservatively to the underlying provider.
  • providers/anthropic: explicitly report false. Anthropic streaming tool-use uses a different event shape (input_json_delta content blocks) and is left as a follow-up to keep this PR focused.
  • agent/root: gate native tools on supportsStreamingNativeTools() when streaming (rather than on !is_streaming), and propagate stream_result.tool_calls into the agent loop instead of dropping them.

Tests

  • Regression tests asserting native tools are requested during streaming when the provider advertises the capability.
  • Regression tests asserting tool_calls emitted on the streaming path reach the agent loop (not dropped).
  • ToolCallAccumulator unit tests for delta accumulation, parallel-indexed calls, and malformed-input safety.

Validation

zig build test --summary all
Build Summary: 13/13 steps succeeded; 7362/7371 tests passed (9 skipped)

0 failures, 0 leaks (std.testing.allocator).

zig fmt --check src/ clean.

Security / Risk

  • No change to tool execution policy, allowlists, or sandboxing. The shell tool's existing security gates (command allowlist, redirection policy, risk classification) apply identically to tool calls arriving via the streaming path.
  • No secrets, tokens, or payloads are logged.
  • Conservative capability default (false): only OpenAI-compatible providers opt in. Adding the capability to a provider that does not actually parse delta.tool_calls would silently drop tool calls, so the opt-in is intentional per provider.

Scope

Provider streaming protocol only. No config schema, vtable signatures beyond the new supportsStreamingNativeTools() slot, or user-facing setup changes. Anthropic streaming tool-use is explicitly out of scope for this PR.

Decouple native tool-call support from the streaming path. Previously the
agent loop disabled native tools whenever a stream callback was attached,
forcing tools into a prompt-injection format that some models could not
reliably emit. The streaming protocol (StreamChunk / StreamChatResult) also
had no field to carry tool calls, so even if a provider parsed them they
were dropped at the agent boundary.

Changes:

- providers/root: add `tool_calls` to StreamChatResult and a
  `supportsStreamingNativeTools()` vtable slot, defaulting false.
- providers/sse: add a ToolCallAccumulator that parses `delta.tool_calls`
  (OpenAI-compatible) across SSE chunks and emits a complete tool_calls
  slice on the final StreamChatResult.
- providers/{openai,compatible}: opt in to streaming native tools.
- providers/{router,reliable}: delegate the capability conservatively.
- providers/anthropic: explicitly report false (Anthropic streaming
  tool-use uses a different event shape and is left for a follow-up).
- agent/root: gate native tools on `supportsStreamingNativeTools()` when
  streaming (not on `!is_streaming`), and propagate
  `stream_result.tool_calls` into the agent loop instead of dropping it.

Adds regression tests asserting native tools are requested during streaming
when the provider advertises the capability, and that tool_calls emitted on
the streaming path reach the agent loop.

Validation: zig build test --summary all → 7362/7371 passed (9 skipped),
0 failures, 0 leaks.
@vernonstinebaker
vernonstinebaker marked this pull request as draft June 29, 2026 12:10
@vernonstinebaker

Copy link
Copy Markdown
Contributor Author

Marking as draft pending #970 (fix(cli): handle arrow keys in agent REPL).

Both PRs modify overlapping regions of src/agent/root.zig. When #970 merges to main, this branch should rebase cleanly onto the updated file. Until then, the two conflict when combined on an integration branch.

This PR is complete and validated on its own (zig build test --summary all → 7362/7371 passed, 9 skipped, 0 failures, 0 leaks). The draft status reflects only the sequencing dependency, not the readiness of the change.

@vernonstinebaker

vernonstinebaker commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Merged into the vernonstinebaker/nullclaw fork main (ce2d54ca) — deployed in production from that fork. Keeping the PR open in case upstream revives.

vernonstinebaker pushed a commit to vernonstinebaker/nullclaw that referenced this pull request Sep 23, 2026
…sted

Approved intake U-1/U-2/U-4 complete and upstream authors notified with commit
links and intake-correction feedback. Landed-in-fork notes added to our nine
still-open upstream PRs (nullclaw#987, nullclaw#971, nullclaw#970, nullclaw#966, nullclaw#963, nullclaw#962, nullclaw#959, nullclaw#954, nullclaw#953).
Upstream PRs cannot be marked merged (no write access; re-applied commits never
trigger GitHub merge detection), so these comments are the provenance record.
vernonstinebaker pushed a commit to vernonstinebaker/nullclaw that referenced this pull request Sep 24, 2026
Incremental PR-by-PR plan for exercising the new committer access: wave 0
pre-flight, wave 1 docs (ours first; nullclaw#776 needs three verified corrections,
nullclaw#777 must not merge with its 0.15.2 pin), wave 2 our code PRs smallest-first,
wave 3 nullclaw#987 then un-drafted nullclaw#971. Per-code-PR loop: merge, fork sync, suite,
4-target build, 4-host deploy, two-turn smoke. Docker/OrbStack recorded as
wave-4 decision (upstream nullclaw#449 still open; images publish via nullbuilder).
Docs-only merges flagged to skip rebuild (binary cannot change) — pending
user confirmation.

@DonPrus DonPrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the agent integration, all changed provider implementations, existing streaming consumers, current main, and both discussion comments. The ownership transfer into the normal tool dispatcher is useful, but I am leaving this draft without approval because the new executable tool-call path still has these blockers:

  1. P1 — Do not execute tool calls salvaged from an incomplete/failed stream (src/providers/sse.zig:769–805, ToolCallAccumulator.toOwnedToolCalls). The existing partial-text recovery branches now return accumulated tool calls as well. After some text plus a tool name arrives, a curl timeout/nonzero exit can therefore produce an executable call even if its argument deltas never arrived; the accumulator substitutes {} for missing arguments. Successful EOF also does not require a completed tool-call choice. Please track completion for the selected choice and only expose complete calls; partial text recovery must not authorize partially received tool execution. Cover a disconnect between name and argument deltas and a nonzero curl exit after mixed text/tool output.

  2. P1 — Query streaming tool capability for the actual routed provider (src/providers/router.zig:250–255, src/providers/reliable.zig:610–612). Router checks default_model, while streamChatImpl resolves the requested model. Reliable checks only inner, but can select an explicit extra/vision provider. With an OpenAI default and an Anthropic request, the agent enables native schemas and omits text-mode parameter instructions even though the selected Anthropic streaming implementation intentionally cannot recover native calls. Please make this capability model/request-aware (as vision support already is), and test both directions of a mixed-provider route.

  3. P2 — Keep tool calls from different choices separate (src/providers/sse.zig:97–140). Text uses the first choice, but the accumulator visits every choice and keys slots only by tool-call index. A two-choice response with index 0 in both merges argument strings into {"path":"a"}{"path":"b"}, potentially combining one choice's ID with another's function name. I reproduced this with an isolated regression test. Please select the same choice as text or explicitly key/select by choice index.

  4. P2 — Preserve ownership and propagate allocation errors (src/providers/sse.zig:89, :132–133). Replacing a repeated function name frees the old allocation before the fallible duplicate; on OOM the slot still points at freed storage and deferred cleanup frees it again. Allocate the replacement before releasing the old value. JSON parsing also catches OOM as a harmless malformed event, silently dropping argument fragments; an injected allocation-failure test confirmed this. Please propagate OOM and add allocation-failure coverage across repeated-name and argument assembly paths.

Analogous-path follow-up: OpenRouterProvider.streamChatImpl already uses the same SSE implementation and accepts native schemas, but its vtable is not opted into the new capability. It would be good to cover it in the same feature once the completion/routing contracts above are fixed.

Validation: 417/419 targeted provider/streaming tests passed; the two additional probes for cross-choice corruption and swallowed OOM failed as described. Temporary probes were saved locally and removed from the checkout. Current main merges cleanly at review time. I also read the draft dependency note about #970; resolving that dependency does not by itself address the findings above. No live provider calls were made.

Addresses the review findings on nullclaw#971.

P1 -- a truncated stream could produce an executable tool call.
`toOwnedToolCalls` completed any slot that had a name, substituting
`{}` when no argument deltas had arrived. A curl timeout or nonzero
exit after a tool name but before its arguments therefore yielded a
call the agent would run with invented input. Two changes:

- `ToolCallSlot.saw_arguments` records that an `arguments` key actually
  arrived, even when its value was the empty string, so "nothing
  arrived" is no longer indistinguishable from "this call takes no
  arguments". A slot is complete only when it has a name and
  `saw_arguments`; the `{}` substitution is gone.
- `finalizeStreamResultWithToolCalls` takes a `tool_calls_trusted`
  flag. The three recovery paths (child wait error, nonzero curl exit,
  abnormal termination) pass false, so salvaged text is still returned
  but no tool call is executed. Only the clean-exit path passes true.

P1 -- capability was asked of the wrong provider. The router consulted
`default_model` while `streamChatImpl` resolves the requested model, and
the failover wrapper consulted only `inner` while `streamChatImpl` can
select an explicit extra or vision provider. With an OpenAI default and
an Anthropic request, native schemas were enabled against an
implementation that intentionally cannot recover native calls.

Added `supports_streaming_native_tools_for_model` to the provider
vtable, mirroring the existing `supports_vision_for_model` precedent,
and wired the two agent call sites to ask about `turn_model_name` -- the
model the turn will actually use. The router resolves the requested
model; the failover wrapper mirrors `streamChatImpl`'s own selection
via `resolveProviderTarget`, since vision selection needs the request
and is not available at this call site.

P2 -- tool calls from different choices were merged. Text comes from
choices[0] but the accumulator visited every choice while keying slots
only by tool-call index, so a two-choice response with index 0 in both
concatenated their argument strings into
`{"path":"a"}{"path":"b"}`, pairing one choice's id with another's
function name. The accumulator now reads the same choice as the text.

P2 -- a failed allocation left a dangling slot. The repeated function
name freed the old value before the fallible duplicate, so on OOM the
slot pointed at released storage and the deferred cleanup freed it
again. The replacement is now duplicated before the old value is
released. Related: `appendFromJsonPayload` swallowed
`error.OutOfMemory` from the JSON parser as a malformed event; that is
now propagated, so an allocation failure surfaces instead of silently
dropping argument fragments.
@vernonstinebaker

Copy link
Copy Markdown
Contributor Author

All four findings addressed in 787239d6 + e831fbc9, with the regressions you asked for. Pushed with a re-review request.

P1 — a truncated stream could execute a tool call

Your reproduction was right, and it was two defects:

  1. ToolCallSlot now records saw_arguments — that an arguments key arrived, even when its value was "". Previously "nothing arrived" and "this tool takes no arguments" were indistinguishable, so the {} substitution completed a call that was cut mid-flight. A slot is complete only with a name and saw_arguments; the substitution is gone.
  2. finalizeStreamResultWithToolCalls takes tool_calls_trusted. The three recovery paths (child wait error, nonzero curl exit, abnormal termination) pass false — salvaged text is still returned, but no tool call is executed. Only the clean-exit path passes true. This is the disconnect-before-arguments and nonzero-exit-after-mixed-output case you described.

P1 — capability was asked of the wrong provider

Added supports_streaming_native_tools_for_model to the provider vtable, mirroring the supports_vision_for_model precedent you pointed at, and wired both agent call sites to ask about turn_model_name — the model the turn actually uses.

  • Router resolves the requested model rather than default_model.
  • Failover wrapper mirrors streamChatImpl's own selection via resolveProviderTarget. Note its non-explicit branch deliberately still answers from inner: vision selection needs the request, which isn't available at this call site, and streamChatImpl also uses inner when no vision is requested. Only the explicit-target case was wrong.

Test covers both directions: a capable default answers yes for itself and no for a hint:sonnet route that resolves to the incapable provider.

P2 — cross-choice merging

The accumulator now reads choices[0], the same choice the text path uses. Test: a two-choice response with index 0 in both no longer concatenates argument strings or pairs one choice's id with another's name.

P2 — ownership and OOM

The repeated function name now duplicates before releasing the old value, so an OOM can't leave the slot pointing at freed storage for the deferred cleanup to free again. Also: appendFromJsonPayload propagated error.OutOfMemory from the JSON parser rather than treating it as a malformed event, so an allocation failure surfaces instead of silently dropping argument fragments.

One thing to know before you review

The branch is 115 commits behind main and, as it stood, was not green: gateway.test.run returns AddressInUse when port is already bound leaked 2 allocations. I confirmed by stash that this predates my changes. Merging current main clears it — that test is untouched by this PR. So please review against the merge result rather than the stale branch tip, or rebase first.

Validation

  • Branch: zig fmt --check src/ exit 0 · ReleaseSmall exit 0 · zig build test --summary all 13/13 steps, 7509/7518 passed, 9 skipped, 0 failures
  • Merge result with current main: clean merge, zig fmt exit 0, ReleaseSmall exit 0, zig build test 13/13 steps, 7509/7518 passed, 9 skipped, 0 failures

Left alone

OpenRouterProvider still uses this SSE implementation and accepts native schemas, but its vtable is not opted into the capability. That is the analogous-path follow-up you flagged; I left it out to keep this change to the reviewed findings — say the word and I'll cover it here.

@vernonstinebaker
vernonstinebaker marked this pull request as ready for review October 6, 2026 13:49

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants