fix(agent): suppress tool protocol when no tools are available#6546
Conversation
449ab0e to
b09e830
Compare
b09e830 to
4def38f
Compare
WareWolf-MoonWall
left a comment
There was a problem hiding this comment.
I've read the full diff across all five touched files: agent.rs, dispatcher.rs, loop_.rs, delegate.rs, and orchestrator/mod.rs.
🟢 What looks good — Defense-in-depth across all code paths
The no-tools guard fires consistently at four independent checkpoints: Agent::should_send_tool_specs() (no specs sent to the provider), Agent::parse_response_for_effective_tools() (no calls parsed from the response), run_tool_call_loop (no native collection, no fallback XML parser, no parse-issue detection, think-tag stripping applied), and XmlToolDispatcher::prompt_instructions() (empty protocol block). Each layer would be independently correct; together they eliminate the category of bugs where one code path emits tool scaffolding while another path is silently skipped. This is the right depth of defense for a change touching agent/runtime prompt behavior.
🟢 What looks good — Prompt/description alignment via effective_tool_names
The effective_tool_names → tool_descs.retain → build_tool_instructions_for_names chain in both process_message and start_channels ensures the prose tool catalog in the system prompt reflects exactly what the runtime will actually execute. The retain_registered_tool_descriptions helper enforces the same invariant in the CLI run path. This removes the drift between described tools and executable tools that was the root of the original issue.
🟡 Warning — Sequencing risk with #6544 and #6541
orchestrator/mod.rs, agent.rs, loop_.rs, and delegate.rs are all modified by #6544 and #6541 as well. These will produce merge conflicts. The recommended sequence: merge #6541 first (session key scoping), then #6544 (adds sends_native_tool_specs to PromptContext), then this PR rebased against both. Before posting the rebase, confirm that build_tool_instructions_for_names (added here) is consistent with whatever final import line state results from the prior two merges.
🔵 Suggestion — Unit test for the partial-exclusion path
The effective_tool_names filtering (AutonomyLevel < Full, some tools excluded) is covered by CI but lacks a focused unit test for the "some excluded, some retained" case on build_tool_instructions_for_names. Adding one would make future regressions in the filter logic immediately visible without a full integration run.
Approved.
tidux
left a comment
There was a problem hiding this comment.
I read the full diff across the runtime agent paths and the channel orchestrator path, including the no-tools response parsing, native tool-spec gating, prompt-section omission, and effective tool-name filtering.
🟢 What looks good — Empty effective tool sets are handled as a first-class state
The change treats "no effective tools" as its own runtime state instead of as a degenerate tool-enabled turn. Agent::should_send_tool_specs() avoids sending an empty native tools array, Agent::parse_response_for_effective_tools() prevents tool-like text from being parsed when there is nothing executable, and run_tool_call_loop applies the same rule to both native calls and text fallback parsing. That is the right security posture for the #5287 slice because tool-looking model output cannot become executable action unless the runtime actually has an effective tool set for the turn.
🟢 What looks good — Prompt text now matches executable availability
The prompt-side filtering is consistent with the runtime behavior: tool_descs is retained against the effective registered names, non-native XML instructions are built only for those names, and the system prompt sections omit tool honesty/protocol scaffolding when no tools remain. This closes the confusing local-model path where the assistant was told how to call tools that the runtime would not actually expose.
🟢 What looks good — Regression coverage hits the risky paths
The new tests cover the important failure modes directly: no native specs sent for an empty effective set, XML-looking output preserved as text, <think> stripping retained, prompt sections omitted, and the channel prompt still emitting exactly one protocol block when a tool is present. I also reran the focused local checks:
cargo test -p zeroclaw-runtime no_tools --lib
cargo test -p zeroclaw-runtime turn_with_no_effective_tools --lib
cargo test -p zeroclaw-channels prompt_includes_single_tool_protocol_block_after_append --lib
All passed.
Approved.
Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior.
4def38f to
275ddcb
Compare
#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
zeroclaw-labs#6546) Treat empty effective tool sets as a no-tools turn across prompt assembly, provider request shape, and parser execution. Preserve reasoning-tag stripping while avoiding execution of tool-like output when no tools are available. Add focused regressions for native request shape, XML text preservation, prompt scaffolding, and channel protocol prompt behavior. 6a1c6ff
Summary
master(all contributions)<think>...</think>stripping in no-tools responses so reasoning scratchpad text does not leak just because tool parsing is disabled.runtime_profile = "ollama_local"config surface, a strict-parser config knob, provider-specific local-model profiles, or a full local-first mode. Those remain follow-up design/work items.Agentturns, the CLI/runtime tool loop, XML dispatcher prompt instructions, reusable prompt sections, and the channel orchestrator's non-native prompt assembly.enhancement,risk: high,size: M,agent:agent,agent:dispatcher,agent:loop,agent:prompt,agent: tests,channel:core.Validation Evidence (required)
cargo test, workspacecargo clippy --all-targets -- -D warnings, and./dev/ci.sh allwere not run locally because this is a focused high-risk slice with targeted runtime/channel tests plus CI coverage. After the CI lint failures, I moved the test-only import into the test module and verified the affected lint class locally withcargo clippy -p zeroclaw-channels --all-targets -- -D warnings.Security & Privacy Impact (required)
Yes/No for each. Answer any
Yeswith a 1–2 sentence explanation.No)No)No)No)Yes, describe the risk and mitigation: No new permissions, network calls, credential handling, or PII are introduced. The security impact is intended to be defensive: when no tools are effective, the model is no longer prompted to emit tool calls and no longer has tool-like output parsed as executable action.Compatibility (required)
Yes)No)NoorYesto either: No user action is required. Existing configured tools still work; this only changes behavior when the effective tool list for a turn is empty or when prompt descriptions previously drifted from filtered tool availability.Rollback (required for
risk: mediumandrisk: high)git revert <squash-commit-sha>.Supersede Attribution (required only when
Supersedes #is used)#<pr> by @<author>, one per line): N/ACo-authored-bytrailers added in commit messages for incorporated contributors? (No)No, why (inspiration-only, no direct code/design carry-over): No superseded PR is incorporated.