From a88febc15eb0b8e150b85ccfea5aa080d62a25c7 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 19 Apr 2026 08:39:16 +0900 Subject: [PATCH] =?UTF-8?q?=E5=BC=95=E6=95=B0=E3=81=AA=E3=81=97=E3=81=A7To?= =?UTF-8?q?olCall=E3=81=99=E3=82=8B=E3=81=A8=E6=A7=8B=E9=80=A0=E3=82=A8?= =?UTF-8?q?=E3=83=A9=E3=83=BC=E3=81=AB=E3=81=AA=E3=82=8B=E5=95=8F=E9=A1=8C?= =?UTF-8?q?=E3=81=AE=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 1 - .../llm_client/scheme/anthropic/request.rs | 8 +- .../src/llm_client/scheme/gemini/request.rs | 7 +- .../src/llm_client/scheme/openai/request.rs | 7 +- crates/llm-worker/src/llm_client/types.rs | 64 ++++++++++++++++ .../src/timeline/tool_call_collector.rs | 25 ++++++- crates/llm-worker/src/worker.rs | 8 +- tickets/tool-call-empty-args-null.md | 75 ------------------- 8 files changed, 104 insertions(+), 91 deletions(-) delete mode 100644 tickets/tool-call-empty-args-null.md diff --git a/TODO.md b/TODO.md index 413cd4e2..974d878a 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,4 @@ - [ ] テスト設計 → [tickets/test-design.md](tickets/test-design.md) -- [ ] 引数なし tool 呼び出しで `arguments = "null"` が記録される不具合 → [tickets/tool-call-empty-args-null.md](tickets/tool-call-empty-args-null.md) - [ ] ツール設計 - [ ] Bash ツール (Permission 層と統合) → [tickets/bash-tool.md](tickets/bash-tool.md) - [ ] Compact の改善（要約品質 + 挙動詳細） → [tickets/compact-improvements.md](tickets/compact-improvements.md) diff --git a/crates/llm-worker/src/llm_client/scheme/anthropic/request.rs b/crates/llm-worker/src/llm_client/scheme/anthropic/request.rs index ff9f748e..44ecd4f0 100644 --- a/crates/llm-worker/src/llm_client/scheme/anthropic/request.rs +++ b/crates/llm-worker/src/llm_client/scheme/anthropic/request.rs @@ -6,7 +6,7 @@ use serde::Serialize; use crate::llm_client::{ Request, - types::{ContentPart, Item, Role, ToolDefinition}, + types::{ContentPart, Item, Role, ToolDefinition, parse_tool_arguments}, }; use super::AnthropicScheme; @@ -170,9 +170,9 @@ impl AnthropicScheme { }); } - // Parse arguments JSON string to Value - let input = serde_json::from_str(arguments) - .unwrap_or_else(|_| serde_json::Value::Object(serde_json::Map::new())); + // Parse arguments JSON string to Value (defensive: normalize + // non-object / legacy "null" payloads to {} so Anthropic API accepts it) + let input = parse_tool_arguments(arguments); pending_assistant_parts.push(AnthropicContentPart::ToolUse { id: call_id.clone(), diff --git a/crates/llm-worker/src/llm_client/scheme/gemini/request.rs b/crates/llm-worker/src/llm_client/scheme/gemini/request.rs index 822d8d23..d0f3b5fe 100644 --- a/crates/llm-worker/src/llm_client/scheme/gemini/request.rs +++ b/crates/llm-worker/src/llm_client/scheme/gemini/request.rs @@ -7,7 +7,7 @@ use serde_json::Value; use crate::llm_client::{ Request, - types::{Item, Role, ToolDefinition}, + types::{Item, Role, ToolDefinition, parse_tool_arguments}, }; use super::GeminiScheme; @@ -244,9 +244,8 @@ impl GeminiScheme { }); } - // Parse arguments - let args = serde_json::from_str(arguments) - .unwrap_or_else(|_| Value::Object(serde_json::Map::new())); + // Parse arguments (normalize non-object / legacy "null" payloads to {}) + let args = parse_tool_arguments(arguments); pending_model_parts.push(GeminiPart::FunctionCall { function_call: GeminiFunctionCall { diff --git a/crates/llm-worker/src/llm_client/scheme/openai/request.rs b/crates/llm-worker/src/llm_client/scheme/openai/request.rs index 7b25a04c..2aa71c21 100644 --- a/crates/llm-worker/src/llm_client/scheme/openai/request.rs +++ b/crates/llm-worker/src/llm_client/scheme/openai/request.rs @@ -7,7 +7,7 @@ use serde_json::Value; use crate::llm_client::{ Request, - types::{Item, Role, ToolDefinition}, + types::{Item, Role, ToolDefinition, parse_tool_arguments}, }; use super::OpenAIScheme; @@ -201,12 +201,15 @@ impl OpenAIScheme { arguments, .. } => { + // Normalize non-object / legacy "null" payloads to "{}" so + // OpenAI gets a valid JSON object string. + let normalized_args = parse_tool_arguments(arguments).to_string(); pending_tool_calls.push(OpenAIToolCall { id: call_id.clone(), r#type: "function".to_string(), function: OpenAIToolCallFunction { name: name.clone(), - arguments: arguments.clone(), + arguments: normalized_args, }, }); } diff --git a/crates/llm-worker/src/llm_client/types.rs b/crates/llm-worker/src/llm_client/types.rs index 412958ac..495318a2 100644 --- a/crates/llm-worker/src/llm_client/types.rs +++ b/crates/llm-worker/src/llm_client/types.rs @@ -317,6 +317,19 @@ impl Item { } } +/// Parse a ToolCall `arguments` string into a JSON object. +/// +/// Tool call arguments must be a JSON object at the provider API level +/// (Anthropic rejects non-object `tool_use.input`). This helper normalizes +/// anything that is not a JSON object — empty string, the literal `"null"`, +/// arrays, scalars, or parse failures — to an empty object `{}`. +pub fn parse_tool_arguments(arguments: &str) -> serde_json::Value { + match serde_json::from_str::(arguments) { + Ok(value) if value.is_object() => value, + _ => serde_json::Value::Object(serde_json::Map::new()), + } +} + // ============================================================================ // Content Parts - Components within message items // ============================================================================ @@ -583,3 +596,54 @@ impl RequestConfig { self } } + +#[cfg(test)] +mod parse_tool_arguments_tests { + use super::parse_tool_arguments; + use serde_json::{Value, json}; + + fn empty_object() -> Value { + Value::Object(serde_json::Map::new()) + } + + #[test] + fn empty_string_normalizes_to_object() { + assert_eq!(parse_tool_arguments(""), empty_object()); + } + + #[test] + fn literal_null_normalizes_to_object() { + // 既存セッションに残っている "null" が resume 時に復旧できること + assert_eq!(parse_tool_arguments("null"), empty_object()); + } + + #[test] + fn array_normalizes_to_object() { + assert_eq!(parse_tool_arguments("[1, 2, 3]"), empty_object()); + } + + #[test] + fn scalar_normalizes_to_object() { + assert_eq!(parse_tool_arguments("42"), empty_object()); + assert_eq!(parse_tool_arguments("\"str\""), empty_object()); + assert_eq!(parse_tool_arguments("true"), empty_object()); + } + + #[test] + fn invalid_json_normalizes_to_object() { + assert_eq!(parse_tool_arguments("{not json"), empty_object()); + } + + #[test] + fn valid_object_passes_through() { + assert_eq!( + parse_tool_arguments(r#"{"city":"Tokyo","days":3}"#), + json!({"city": "Tokyo", "days": 3}), + ); + } + + #[test] + fn empty_object_passes_through() { + assert_eq!(parse_tool_arguments("{}"), empty_object()); + } +} diff --git a/crates/llm-worker/src/timeline/tool_call_collector.rs b/crates/llm-worker/src/timeline/tool_call_collector.rs index e81a6b6f..3cc73520 100644 --- a/crates/llm-worker/src/timeline/tool_call_collector.rs +++ b/crates/llm-worker/src/timeline/tool_call_collector.rs @@ -5,6 +5,7 @@ use crate::{ handler::{Handler, ToolUseBlockEvent, ToolUseBlockKind}, + llm_client::types::parse_tool_arguments, tool::ToolCall, }; use std::sync::{Arc, Mutex}; @@ -84,8 +85,7 @@ impl Handler for ToolCallCollector { // ブロック完了時にToolCallを確定 if let (Some(id), Some(name)) = (scope.current_id.take(), scope.current_name.take()) { - let input = serde_json::from_str(&scope.input_json_buffer) - .unwrap_or(serde_json::Value::Null); + let input = parse_tool_arguments(&scope.input_json_buffer); let tool_call = ToolCall { id, name, input }; @@ -123,6 +123,27 @@ mod tests { assert_eq!(calls[0].input["city"], "Tokyo"); } + #[test] + fn test_collect_empty_buffer_returns_object() { + // 引数なしツール呼び出し: input_json_delta が一度も来ないケース + let collector = ToolCallCollector::new(); + let mut timeline = Timeline::new(); + timeline.on_tool_use_block(collector.clone()); + + timeline.dispatch(&Event::tool_use_start(0, "tool_empty", "ListPods")); + timeline.dispatch(&Event::tool_use_stop(0)); + + let calls = collector.take_collected(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].id, "tool_empty"); + assert_eq!(calls[0].name, "ListPods"); + assert!(calls[0].input.is_object()); + assert_eq!( + calls[0].input, + serde_json::Value::Object(serde_json::Map::new()) + ); + } + #[test] fn test_collect_multiple_tool_calls() { let collector = ToolCallCollector::new(); diff --git a/crates/llm-worker/src/worker.rs b/crates/llm-worker/src/worker.rs index 05913760..5cedf79a 100644 --- a/crates/llm-worker/src/worker.rs +++ b/crates/llm-worker/src/worker.rs @@ -16,7 +16,10 @@ use crate::{ DefaultInterceptor, Interceptor, PostToolAction, PreRequestAction, PreToolAction, PromptAction, ToolCallInfo, ToolResultInfo, TurnEndAction, }, - llm_client::{ClientError, ConfigWarning, LlmClient, Request, RequestConfig, ToolDefinition}, + llm_client::{ + ClientError, ConfigWarning, LlmClient, Request, RequestConfig, ToolDefinition, + types::parse_tool_arguments, + }, state::{Locked, Mutable, WorkerState}, timeline::event::{ErrorEvent, StatusEvent, UsageEvent}, timeline::{TextBlockCollector, Timeline, ToolCallCollector}, @@ -573,8 +576,7 @@ impl Worker { } = item { if !answered_call_ids.contains(call_id) { - let input = serde_json::from_str(arguments) - .unwrap_or_else(|_| serde_json::Value::Object(serde_json::Map::new())); + let input = parse_tool_arguments(arguments); pending_calls.push(ToolCall { id: call_id.clone(), name: name.clone(), diff --git a/tickets/tool-call-empty-args-null.md b/tickets/tool-call-empty-args-null.md deleted file mode 100644 index c4d2f7c4..00000000 --- a/tickets/tool-call-empty-args-null.md +++ /dev/null @@ -1,75 +0,0 @@ -# 引数なし tool 呼び出しで `arguments = "null"` が記録される不具合 - -## 背景 - -引数を取らないツール（例: `ListPods`）を Anthropic の Claude が呼び出したとき、次ターンで履歴を送り返す際に Anthropic API が以下のエラーで 400 を返す： - -``` -messages.N.content.0.tool_use.input: Input should be a valid dictionary -``` - -実環境で `cargo run -p pod` + TUI / API 経由で `ListPods` を呼ぶと再現する。セッション jsonl には tool 呼び出しが以下の形で記録されている： - -```json -{"type":"tool_call","call_id":"toolu_...","name":"ListPods","arguments":"null"} -``` - -`arguments` が `"null"` 文字列になっており、次ターンで Anthropic に送る `tool_use.input` が JSON `null` として serialize されてしまうことが原因。 - -## 原因 - -`crates/llm-worker/src/timeline/tool_call_collector.rs:87-88`: - -```rust -let input = serde_json::from_str(&scope.input_json_buffer) - .unwrap_or(serde_json::Value::Null); -``` - -Anthropic は引数なしのツール呼び出しでは `input_json_delta` を一度も送らない。その結果 `input_json_buffer` が空文字 `""` のまま stop イベントに到達し、`from_str("")` が失敗して `Value::Null` に fallback する。 - -この `Null` が `worker.rs:499` で `Item::tool_call_json(..., Value::Null)` として履歴に保存され、`Value::Null.to_string()` = `"null"` が `arguments` フィールドに残る。 - -次ターンで `anthropic/request.rs:174-175` が history → request body 変換する際： - -```rust -let input = serde_json::from_str(arguments) - .unwrap_or_else(|_| serde_json::Value::Object(serde_json::Map::new())); -``` - -`"null"` は valid JSON として parse 成功するため fallback が効かず、`Value::Null` のまま `tool_use.input` に入り API に送信される。Anthropic の tool_use.input は object 必須なので拒否される。 - -## 修正方針 - -ルート修正を `tool_call_collector.rs` に入れる。引数なし / パース失敗の場合は `Value::Object(Map::new())`（= `{}`）にする： - -```rust -let input = if scope.input_json_buffer.is_empty() { - serde_json::Value::Object(serde_json::Map::new()) -} else { - serde_json::from_str(&scope.input_json_buffer) - .unwrap_or_else(|_| serde_json::Value::Object(serde_json::Map::new())) -}; -``` - -加えて防御層として `anthropic/request.rs:174` で parse 結果が object でない場合も `{}` に正規化する。既に `arguments = "null"` として保存済みの古いセッションが resume されたときに回復できるようにするため。 - -## 影響範囲 - -- `crates/llm-worker/src/timeline/tool_call_collector.rs`: 空バッファ時の default を `Value::Object` に -- `crates/llm-worker/src/llm_client/scheme/anthropic/request.rs`: parse 結果が非 object の場合の正規化 -- `crates/llm-worker/src/worker.rs:576-577` の同様の parse でも非 object を正規化（防御） -- OpenAI / Gemini の request.rs でも同等の問題があるか確認、必要なら同じ修正を入れる - -## 完了条件 - -- 引数なしツール（`ListPods` など）を呼んだ直後のセッション jsonl で `arguments` が `"{}"` になる -- 同一セッション内で引数なしツールを呼んでから次ターンを開始しても 400 エラーが出ない -- `"arguments":"null"` が残っている既存セッションを resume しても 400 エラーが出ない -- `tool_call_collector.rs` に「空バッファ → `{}`」を検証するテストを追加 -- 該当パスの回帰テストを単体テストで担保 - -## 範囲外 - -- LLM 側が「無意味に `null` を `input` に入れてくる」ケースの検出・警告（そもそもプロバイダから来ないはず） -- OpenAI / Gemini の同等検証（確認だけ行い、問題あれば別チケット化） -- 既存セッション jsonl の自動修復スクリプト（resume 時の defensive 正規化で十分）