Handle malformed server-side tool call args correctly by feeding back to the model

[ScriptSmith]

No description provided.

[greptile-apps]

Greptile Summary

This PR addresses a class of silent failures where a server-side tool call (shell, web_search, file_search, MCP, tool_search) with unparseable arguments was either dropped — leaving the agentic loop in a false-completed state — or dispatched with null args, causing confusing downstream errors. The fix introduces a DetectedToolCall::invalid field and a corresponding execute() contract so each tool synthesizes a spec-shaped failure item and feeds the error back as a function_call_output, allowing the model to self-correct.

• DetectedToolCall gains an invalid: Option<String> field with constructor methods new()/invalid() and a shared invalid_arguments_text() helper; the detect() and execute() contracts are now documented on the trait.
• All five tool executors (shell, web_search, file_search, MCP, tool_search) are updated: parse functions return Result/detection enums instead of Option, invalid detections are routed to synthesized failure handles instead of being silently dropped, and the detect_in_chunk / MCP path preserves label/tool metadata on the invalid call so execute() can construct a well-formed failure item.
• Previous reviewed gaps (? abort inside the call.invalid guard in MCP, silent done-event drop in file_search) are addressed: the MCP guard uses unwrap_or_else instead of ?, and format_file_search_call_sse_event now returns Bytes unconditionally.

Confidence Score: 5/5

Safe to merge; the change is well-scoped and each tool's invalid-args path is covered by new tests, including the previously-flagged edge cases.

All six changed files follow the same pattern consistently. The two issues raised in previous review rounds (the ?-abort inside the MCP call.invalid guard and the silent done-event drop in file_search) are explicitly fixed here. The one remaining note — invalid_arguments_text using the generic "mcp" label instead of the already-computed specific tool name — is a minor quality-of-life improvement for model self-correction and does not affect correctness or loop continuity.

No files require special attention.

Important Files Changed

Filename	Overview
src/services/server_tools/mod.rs	Adds invalid: Option<String> field to DetectedToolCall, constructor methods new()/invalid(), and the shared invalid_arguments_text() helper; contracts for detect/execute are documented
src/services/mcp/executor.rs	Invalid-args guard added before the main binding-dispatch path; synthesize_failed_call refactored to accept &str server label; detect_in_chunk marks unparseable JSON invalid instead of silently substituting Value::Null; new tests cover both the normal-detect path and the public-constructor edge case
src/services/shell_tool.rs	Adds ShellCallDetection enum, truncate_for_error, and synthesize_invalid_args_handle; both parse failures and empty-commands cases are now surfaced as spec-shaped shell_call + shell_call_output events with exit_code 2 and the error in stderr
src/services/web_search_tool.rs	Adds WebSearchCallDetection enum and synthesize_web_search_invalid_handle; parse changed from Option to Result; invalid detections route through synthesize_web_search_invalid_handle in execute()
src/services/file_search_tool.rs	Adds FileSearchCallDetection enum and synthesize_file_search_invalid_handle; format_file_search_call_sse_event now returns Bytes unconditionally (fixing the previous-reviewed done-event drop); check_response_for_file_search preserves pre-PR behavior of skipping invalid calls
src/services/mcp/tool_search/mod.rs	Replaces the silent Value::Null fallback for unparseable arguments with DetectedToolCall::invalid(); adds an invalid-execution branch that emits a spec-shaped tool_search_call with status failed; call_id: null in emitted items is intentional per the tool-search spec for server-executed calls

Sequence Diagram

sequenceDiagram
    participant Model
    participant Orchestrator
    participant Tool as Tool Executor

    Model->>Orchestrator: SSE chunk with function_call (malformed args)
    Orchestrator->>Tool: "detect(chunk) → DetectedToolCall{invalid: Some(err)}"
    Orchestrator->>Tool: execute(call) [call.invalid is Some]
    Note over Tool: synthesize_*_invalid_handle
    Tool-->>Orchestrator: "ToolExecutionHandle{events: [in_progress, failed], result: Ok}"
    Orchestrator-->>Model: SSE: tool_call in_progress → failed
    Orchestrator->>Model: "Continuation: FunctionCallOutput{output: "Invalid arguments …"}"
    Model->>Orchestrator: Next turn with corrected tool call

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/services/mcp/executor.rs:501-507
**`invalid_arguments_text` uses the generic `"mcp"` label instead of the actual tool name**

`tool_name` is computed just a few lines earlier (from `__mcp_tool` in the arguments, falling back to `call.call_id`) and is already passed to `synthesize_failed_call` for the MCP call item itself. But the error text fed back to the model still says "Invalid arguments for tool `mcp`" instead of naming the specific tool (e.g., "Invalid arguments for tool `jira_create`"). The SSE event and the model-facing error therefore disagree on the tool name, making the model's self-correction loop marginally harder.

_{Reviews (3): Last reviewed commit: "Review fixes" | Re-trigger Greptile}

[ScriptSmith]

@greptile-apps