fix: orphaned tool_results and parallel tool_result merging in Worker

[NYX-privacy-ai]

Summary

Fixes two related bugs that cause the Anthropic API to return empty responses when the Worker executes tools via select_tools():

• Bug 1 — Orphaned tool_results: Worker::execution_loop never pushed ChatMessage::assistant_with_tool_calls() when using the select_tools() path. On the next LLM call, sanitize_tool_messages() sees tool_result messages with no matching assistant tool_calls, marks them as "orphaned", and rewrites them as user messages — confusing the LLM into returning empty responses. The respond_with_tools() path already does this correctly (lines 523-528); this patch applies the same pattern to select_tools().

• Bug 2 — Consecutive User messages for parallel tools: convert_messages() in rig_adapter.rs creates a separate RigMessage::User for each tool_result. The Anthropic API requires all tool results for a given assistant tool_use batch to be in a single User message. When multiple tools run in parallel, consecutive User messages violate the protocol and produce empty content arrays. This patch merges consecutive tool_result messages into a single User message.

Test plan

• Single tool execution via Worker (e.g., Polymarket heartbeat) — completes successfully
• Parallel tool execution via Worker (e.g., DeFi heartbeat calling positions + balance simultaneously) — both results merged, LLM responds with non-empty content
• cargo check --features libsql passes cleanly

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two critical bugs related to tool execution within the Worker, specifically when interacting with the Anthropic API. The changes ensure that tool results are properly contextualized by preceding assistant messages and that multiple parallel tool results are correctly grouped into a single user message. This stabilizes the tool execution flow, prevents misinterpretation by the LLM, and resolves instances of empty API responses, thereby improving the overall reliability of tool-using agents.

Highlights

• Orphaned Tool Results Fix: Corrected an issue in Worker::execution_loop where assistant_with_tool_calls() messages were not pushed when using the select_tools() path. This prevented tool_result messages from being incorrectly marked as "orphaned" by sanitize_tool_messages(), which previously confused the LLM into returning empty responses.
• Parallel Tool Result Merging: Implemented logic in rig_adapter.rs to merge consecutive tool_result messages into a single User message. This change adheres to the Anthropic API's requirement for all tool results from a given assistant tool_use batch to be in one User message, resolving issues with empty responses during parallel tool execution.

Changelog

• src/agent/worker.rs
  • Ensured assistant_with_tool_calls messages are pushed to the message history before executing tools via the select_tools() path.
• src/llm/rig_adapter.rs
  • Modified the convert_messages function to merge consecutive ToolResult messages into a single RigMessage::User to comply with Anthropic API requirements for parallel tool calls.

Activity

• The author has confirmed successful execution for single tool scenarios.
• The author has confirmed successful execution for parallel tool scenarios, with results correctly merged and non-empty LLM responses.
• The author has verified that cargo check --features libsql passes cleanly.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to fix two bugs related to tool execution: orphaned tool_results and incorrect merging of parallel tool results, improving compatibility with the Anthropic API. However, a high-severity security vulnerability was identified where sensitive tool parameters are leaked in job event logs and SSE broadcasts due to unredacted logging of raw tool parameters. Additionally, there are a couple of suggestions to refactor the new code for improved clarity and maintainability.

[gemini-code-assist]

The process_tool_result function logs raw tool parameters without redaction, leading to a high-severity security vulnerability where sensitive information (like credentials or API keys) can be leaked in job event logs and SSE broadcasts. It is crucial to use redact_params with the tool's sensitive_params() before logging to prevent this. Additionally, the logic for handling single and multiple tool selections is duplicated; consider simplifying this by always calling execute_tools_parallel to improve maintainability.

[gemini-code-assist]

Similar to the single tool execution path, the parallel tool execution path also calls process_tool_result with unredacted parameters, leading to potential leakage of sensitive information in job event logs and SSE broadcasts.

Ensure that tool parameters are redacted before logging in process_tool_result to prevent accidental exposure of secrets.

[gemini-code-assist]

This logic for merging tool results can be made more concise by using an if let chain. This removes the need for the separate should_merge variable and makes the code more idiomatic and readable.

                if let Some(RigMessage::User { content }) = history.last_mut()
                    && content.iter().all(|c| matches!(c, UserContent::ToolResult(_)))
                {
                    content.push(tool_result);
                } else {
                    history.push(RigMessage::User {
                        content: OneOrMany::one(tool_result),
                    });
                }

[zmanian]

Review: fix: orphaned tool_results and parallel tool_result merging in Worker

This PR bundles 7 commits fixing several distinct issues: orphaned tool_results, parallel tool_result merging, Telegram timeouts/session/formatting, idempotent state transitions, scheduler job cleanup, and empty API response handling. The core bug fixes (orphaned tool_results, tool_result merging) are correct and well-motivated. However, there are issues that should be addressed before merge.

Scope / PR Hygiene

This PR title says "orphaned tool_results and parallel tool_result merging" but actually contains 7 distinct fixes across 4 subsystems (worker, rig_adapter, telegram channel, scheduler, state machine). Consider splitting into separate PRs -- at minimum the Telegram changes (4 commits: timeout, session persistence, markdown-to-HTML, thread_id) are completely independent from the Worker/LLM fixes and should be their own PR.

Bug 1 Fix (orphaned tool_results) -- Correct

The fix in worker.rs to push ChatMessage::assistant_with_tool_calls() before executing tools in the select_tools() path mirrors what respond_with_tools() already does (lines 554-559). This is the right fix.

However, I notice the restructured code now has the assistant message push happening outside the if selections.len() == 1 / else branch -- meaning the reasoning text extraction uses only selections.first(). This is fine for the single-tool case but for parallel tools, all selections share the same reasoning prefix. Verify this matches the intent.

Bug 2 Fix (consecutive User messages) -- Correct with Nits

The merging logic in rig_adapter.rs is correct: consecutive tool_result messages are merged into a single RigMessage::User. The should_merge check verifying all existing content items are ToolResult is a good guard.

Nit: The two-step should_merge + if let pattern could be simplified with a single if let chain as Gemini noted. Not blocking.

Empty Response Fallback (rig_adapter.rs) -- Needs Discussion

The fallback for empty API responses (lines 449-475 and 514-548) catches rig-core errors containing "no message or tool call" or "(empty)" and returns a canned text response. Concerns:

1. String matching on error messages is fragile. If rig-core changes its error wording, this silently stops working. Consider matching on the error variant/type if rig-core exposes one, or at minimum add a comment noting the exact rig-core version this targets.

2. The fallback is duplicated in both complete() and complete_with_tools(). Extract a helper like fn handle_completion_result(result, model_name) -> Result<..> to avoid the duplication.

3. Token counts of 0 are misleading. Downstream cost tracking will undercount. Consider at least logging a warning about inaccurate token counts, or using None if the response types support it.

4. The canned response "Let me try a different approach" may confuse the agent into thinking the LLM genuinely wants to change strategy, potentially causing loops. A more neutral fallback or an explicit retry at this layer might be better.

Idempotent State Transitions (state.rs) -- Caution

Making can_transition_to(same_state) return true and short-circuiting in transition_to() is a reasonable fix for the double-completion race. However:

• This silently masks bugs. If code accidentally tries Pending -> Pending or Failed -> Failed, it will succeed silently instead of surfacing the logic error. Consider logging at debug or warn level when an idempotent no-op occurs, so it's visible during development.
• The guard in transition_to() returns Ok(()) before pushing to transitions -- good, that avoids polluting the history. But the comment says "nothing to do" while can_transition_to now returns true for same-state -- these two guards are redundant. The transition_to guard alone would suffice; the can_transition_to change makes the public API more permissive than necessary for external callers.

Scheduler Cleanup Watchdog (scheduler.rs) -- Works but Polling

The 2-hour hard ceiling with 1-second polling is functional but spawns a long-lived task per job that wakes every second. For a system with MAX_PARALLEL_JOBS=5 this is fine, but consider:

• Using tokio::time::timeout on the JoinHandle instead of polling, which would be zero-cost until the timeout fires.
• The max_job_lifetime is hardcoded. Consider making it configurable via AgentConfig or at least a named constant.

Telegram Changes

These look correct but are out of scope for the PR title:

• Timeout reduction (30->25s): Sound reasoning to stay under the 30s WASM callback limit.
• chat_id as thread_id: Correct fix for session persistence. Good find.
• Markdown-to-HTML converter: Functional but has edge cases:
  • Nested markdown like **bold *and italic* text** will not convert correctly -- the bold pass runs first and the italic pass won't find the inner * correctly after <b> tags are inserted.
  • The escape_html runs before inline markdown conversion for non-heading lines, which means < in user content won't interfere with generated tags. Good.
  • The heading detection (starts_with("### ")) should handle #### and deeper. Currently trim_start_matches('#') handles it but only ###, ##, # prefixes are checked -- a line starting with #### falls through to convert_inline_markdown.
  • No tests for the markdown converter. Given this is a WASM channel, unit tests would be valuable.

max_tokens bump (1024 -> 2048)

Doubling max_tokens in select_tools() is reasonable but should be documented in the commit message with evidence (was truncation observed? how often?). This doubles cost per tool-selection call.

Missing

• No regression tests for the orphaned tool_results fix or the tool_result merging fix. Per project rules, every bug fix must include a test. The [skip-regression-check] escape valve exists but should be justified.
• The process_tool_result logging concern from Gemini's review (parameter redaction) appears to be pre-existing, not introduced by this PR, but worth confirming.

Summary

The core fixes (Bug 1 and Bug 2) are correct and important. Request changes for:

1. Add regression tests for the two core bugs (or justify [skip-regression-check])
2. Add a log line when idempotent state transition no-op occurs
3. Consider extracting the duplicated empty-response fallback in rig_adapter
4. Split Telegram changes into a separate PR (or rename this PR to reflect actual scope)

[ilblackdragon]

Thanks @NYX-privacy-ai for identifying the MCP client startup persistence bug! Your analysis was spot-on — startup-loaded clients were being dropped after tool registration, leaving the ExtensionManager unable to track their connection status.

We extracted the MCP client fix into a standalone PR (#1509) which has been merged. The other changes in this PR (notification context injection, thread idle timeout) need different approaches, so we're leaving this PR open for now if you'd like to rework those separately.

Appreciate the contribution! 🙏

[ilblackdragon]

Updating status on this PR — all the useful fixes have been merged separately:

• Orphaned tool_results + parallel merging (the bugs in the PR title) → merged via fix(worker): prevent orphaned tool_results and fix parallel merging #1069
• MCP client startup persistence → merged via fix: persist startup-loaded MCP clients in ExtensionManager #1509

The two remaining changes in this diff are:

1. Notification context injection (agent_loop.rs) — The underlying problem is real (routine notifications leave no trace in the user's chat thread, so the LLM lacks context when the user replies). However, the approach of injecting synthetic start_turn()/complete_turn() pairs has issues: it clobbers thread state, pollutes the context window with fabricated turns, and hardcodes "telegram" as default channel. This needs a different design — likely persisting the notification as a system-level message in the conversation history.

2. Thread idle timeout (session_manager.rs) — The existing compaction system (MoveToWorkspace / Summarize / Truncate) already handles unbounded context growth in long-lived threads. A 30-minute hard timeout would break mid-conversation context (e.g., user takes a lunch break) and introduces a TOCTOU race in the lock handling.

Given that all actionable fixes are merged, this PR can be closed. If you'd like to pursue the notification context gap (#1 above), happy to collaborate on a proper design in a new issue/PR.