From 94601423e8aa375ef94154751fdbc30460e4dd28 Mon Sep 17 00:00:00 2001 From: Darkatse <39111369+Darkatse@users.noreply.github.com> Date: Sun, 13 Sep 2026 06:56:02 -0400 Subject: [PATCH] fix(agent): normalize tool arguments and recover invalid calls Accept null and empty arguments as an empty object, and validate argument shape once before dispatch. Malformed calls consume their budget and return recoverable tool errors. Normalize provider replay while preserving rejected input in audit storage. Include runtime and protocol regression coverage and the tool argument contract. Validation: pnpm run check. --- docs/Agent/ToolSystem.md | 9 ++ .../contract_tests/agent_runtime/execution.rs | 93 +++++++++++ .../services/agent_model_gateway/decode.rs | 14 +- .../services/agent_model_gateway/encode.rs | 6 +- .../src/services/agent_model_gateway/tests.rs | 6 +- .../delegation/await_tool.rs | 5 +- .../delegation/delegate_tool.rs | 5 +- .../delegation/handoff_tool.rs | 3 +- .../delegation/list_tool.rs | 10 +- .../delegation/task_return_tool.rs | 18 +-- .../model_turn_display.rs | 6 +- .../agent_runtime_service/tool_execution.rs | 53 +++++-- .../agent_tools/chat/read_messages.rs | 13 +- .../src/services/agent_tools/chat/search.rs | 13 +- .../src/services/agent_tools/common.rs | 4 - .../src/services/agent_tools/dice/roll.rs | 22 +-- .../src/services/agent_tools/dispatcher.rs | 33 ++-- .../src/services/agent_tools/skill/list.rs | 4 +- .../src/services/agent_tools/skill/read.rs | 16 +- .../src/services/agent_tools/skill/script.rs | 17 +-- .../skill/script/tests/execution.rs | 12 +- .../agent_tools/skill/script/tests/mod.rs | 14 +- .../skill/script/tests/workspace.rs | 28 +++- .../src/services/agent_tools/skill/search.rs | 16 +- .../agent_tools/workspace/apply_patch.rs | 14 +- .../services/agent_tools/workspace/args.rs | 4 +- .../services/agent_tools/workspace/commit.rs | 15 +- .../services/agent_tools/workspace/finish.rs | 7 +- .../agent_tools/workspace/list_files.rs | 15 +- .../agent_tools/workspace/read_file.rs | 14 +- .../agent_tools/workspace/search_files.rs | 14 +- .../services/agent_tools/workspace/tests.rs | 144 +++++++++--------- .../agent_tools/workspace/write_file.rs | 13 +- .../agent_tools/world_info/read_activated.rs | 13 +- .../payload/claude/messages.rs | 8 +- .../payload/claude/tools.rs | 2 +- .../payload/gemini_interactions.rs | 2 +- .../payload/makersuite.rs | 2 +- .../chat_completion_service/payload/mod.rs | 60 ++++++++ .../payload/openai_responses.rs | 15 +- .../payload/tool_calls.rs | 24 +-- .../src/services/tool_request_gate.rs | 6 +- src-tauri/crates/tt-domain/src/models/tool.rs | 124 ++++++++++++++- 43 files changed, 546 insertions(+), 370 deletions(-) diff --git a/docs/Agent/ToolSystem.md b/docs/Agent/ToolSystem.md index 52d70115..11e55ded 100644 --- a/docs/Agent/ToolSystem.md +++ b/docs/Agent/ToolSystem.md @@ -27,6 +27,14 @@ Invocation 使用的工具快照包含描述、参数、名称映射及预算上 聊天工具读取 Run 输入对应的历史范围；文件读取使用 1-based 行号，聊天消息索引使用 0-based。较长文本可以分段读取。完整工具集合会按 Profile 收窄，return-mode 子 Agent 使用 `task.return` 作为结束工具。 +## 参数 + +一次调用携带一组具名参数。线上要么是 JSON object，要么是它的 JSON 字符串编码，「没有参数」在各家实现里有多种等价写法（字段缺失、`null`、`""`、`"null"`、`"{}"`）。`ToolArguments` 是唯一的换算位置：先剥掉字符串编码，再用同一条规则判断，因此两种编码得到同一个参数集合，全可选参数的工具可以直接无参调用。 + +既不是 object、也不属于上述空集合写法的内容（截断的 JSON、数组、标量）原样保留。runtime 在预算计入之后、分派之前拒绝这类调用，返回可恢复的 `tool.invalid_arguments` 并回显开头一段原文；工具本身只接收已经确定为 object 的参数。 + +各渠道回放统一使用对象参数：合法对象保持原值，非法参数回放为 `{}`，拒绝原因由配对的 tool error 承载。持久化仍保留非法参数原文。 + ## 结果如何进入下一轮 `AgentToolResult` 包含调用 ID、工具 ID、文本、结构化结果、错误信息和资源引用。runtime 把它作为 tool message 追加到模型上下文，并保存调用记录。 @@ -47,5 +55,6 @@ Skill 提供按需读取的工作方法、材料和脚本。它沿用相同的 - [registry.rs](../../src-tauri/crates/tt-application/src/services/agent_tools/registry.rs)：内置目录。 - [policy.rs](../../src-tauri/crates/tt-application/src/services/agent_tools/policy.rs)：Invocation 工具快照。 +- [tool.rs](../../src-tauri/crates/tt-domain/src/models/tool.rs)：`ToolArguments` 的线上编码换算规则。 - [tool_request_gate.rs](../../src-tauri/crates/tt-application/src/services/tool_request_gate.rs)：调用检查与预算。 - [tool_execution.rs](../../src-tauri/crates/tt-application/src/services/agent_runtime_service/tool_execution.rs)：分派和记录。 diff --git a/src-tauri/crates/tauritavern/src/app/contract_tests/agent_runtime/execution.rs b/src-tauri/crates/tauritavern/src/app/contract_tests/agent_runtime/execution.rs index 01f0fa43..03a287b5 100644 --- a/src-tauri/crates/tauritavern/src/app/contract_tests/agent_runtime/execution.rs +++ b/src-tauri/crates/tauritavern/src/app/contract_tests/agent_runtime/execution.rs @@ -1,4 +1,5 @@ use super::*; +use tt_domain::models::tool::ToolId; use tt_domain::models::upstream_failure::{UPSTREAM_NETWORK_TIMEOUT, UpstreamFailure}; #[tokio::test] @@ -354,6 +355,98 @@ async fn agent_runtime_returns_missing_chat_reads_to_the_agent() { let _ = fs::remove_dir_all(root).await; } +#[tokio::test] +async fn agent_runtime_normalizes_empty_arguments_and_recovers_from_invalid_arguments() { + let root = temp_root("agent-tool-arguments"); + let mut malformed = model_tool_call("call_invalid", "workspace_list_files", json!({})); + malformed["function"]["arguments"] = json!(r#"{"path":"#); + let fixture = agent_runtime_fixture_with_responses( + &root, + vec![ + model_tool_response(vec![ + // The helper encodes this as the wire string "null". + model_tool_call("call_empty", "workspace_list_files", Value::Null), + malformed, + ]), + model_tool_response(vec![ + model_tool_call("call_over_budget", "workspace_list_files", json!({})), + model_tool_call( + "call_write", + "workspace_write_file", + json!({ "path": "output/main.md", "content": "continued after invalid arguments" }), + ), + model_tool_call("call_finish", "workspace_finish", json!({})), + ]), + ], + ); + let mut profile = resolve_contract_profile(&fixture).await; + profile.tools.max_rounds = 2; + profile + .tools + .max_calls_per_tool + .insert(ToolId::builtin("workspace.list_files").unwrap(), 2); + let run = contract_run("run_arguments", AgentRunPresentation::Background, &profile); + fixture.agent_repository.create_run(&run).await.unwrap(); + let request = chat_request("list files and write an output"); + let prompt_snapshot = json!({ "chatCompletionPayload": request.payload.clone() }); + let (_cancel_sender, mut cancel_receiver) = watch::channel(false); + + fixture + .service + .execute_agent_loop_run_inner( + &run.id, + prompt_snapshot, + request, + profile, + &mut cancel_receiver, + ) + .await + .expect("argument errors remain recoverable"); + + let requests = fixture.model_gateway.requests().await; + let results = requests[1] + .messages + .iter() + .flat_map(|message| &message.parts) + .filter_map(|part| match part { + AgentModelContentPart::ToolResult { result } => Some(result), + _ => None, + }) + .collect::>(); + assert_eq!(results.len(), 2); + assert_eq!(results[0].call_id, "call_empty"); + assert!(!results[0].is_error); + assert!(results[0].structured["entries"].is_array()); + assert_eq!( + results[1].error_code.as_deref(), + Some("tool.invalid_arguments") + ); + assert!(results[1].content.contains(r#"{"path":"#)); + + let events = read_agent_events(&fixture.agent_repository, &run.id).await; + assert!(events.iter().any(|event| { + event.event_type == "tool_call_failed" + && event.payload["callId"] == "call_over_budget" + && event.payload["errorCode"] == "agent.tool_budget_exhausted" + })); + let artifact = fixture + .agent_repository + .read_text(&run.id, &WorkspacePath::parse("output/main.md").unwrap()) + .await + .unwrap(); + assert_eq!(artifact.text, "continued after invalid arguments"); + assert_eq!( + fixture + .agent_repository + .load_run(&run.id) + .await + .unwrap() + .status, + AgentRunStatus::Completed + ); + fs::remove_dir_all(root).await.unwrap(); +} + #[tokio::test] async fn agent_runtime_duplicate_tool_call_id_preserves_first_audit_facts() { let root = temp_root("agent-duplicate-tool-call-id"); diff --git a/src-tauri/crates/tt-application/src/services/agent_model_gateway/decode.rs b/src-tauri/crates/tt-application/src/services/agent_model_gateway/decode.rs index cd245320..5f4a30af 100644 --- a/src-tauri/crates/tt-application/src/services/agent_model_gateway/decode.rs +++ b/src-tauri/crates/tt-application/src/services/agent_model_gateway/decode.rs @@ -7,7 +7,7 @@ use crate::services::chat_completion_service::exchange::{ use tt_domain::models::agent::{ AgentModelContentPart, AgentModelMessage, AgentModelResponse, AgentModelRole, AgentModelTool, }; -use tt_domain::models::tool::ToolInvocation; +use tt_domain::models::tool::{ToolArguments, ToolInvocation}; #[cfg(any(test, feature = "test-support"))] pub fn decode_chat_completion_response( @@ -201,7 +201,7 @@ fn parse_tool_call( )) })?; let arguments = - parse_tool_call_arguments(function.get("arguments").or_else(|| function.get("args"))); + ToolArguments::decode(function.get("arguments").or_else(|| function.get("args"))); Ok(ToolInvocation { call_id: id.to_string(), @@ -222,16 +222,6 @@ pub(super) fn model_tool_for_alias<'a>( tools.iter().find(|tool| tool.model_alias == alias) } -fn parse_tool_call_arguments(value: Option<&Value>) -> Value { - match value { - Some(Value::String(raw)) => { - serde_json::from_str::(raw).unwrap_or_else(|_| Value::String(raw.to_string())) - } - Some(Value::Null) | None => Value::Object(Map::new()), - Some(value) => value.clone(), - } -} - fn extract_text_from_message(message: &Map) -> String { text_from_value(message.get("content")).unwrap_or_default() } diff --git a/src-tauri/crates/tt-application/src/services/agent_model_gateway/encode.rs b/src-tauri/crates/tt-application/src/services/agent_model_gateway/encode.rs index a156a102..aae01cac 100644 --- a/src-tauri/crates/tt-application/src/services/agent_model_gateway/encode.rs +++ b/src-tauri/crates/tt-application/src/services/agent_model_gateway/encode.rs @@ -290,10 +290,6 @@ fn encode_openai_tool_call( let model_alias = model_tool_for_id(&call.tool_id, tools) .map(|tool| tool.model_alias.as_str()) .ok_or_else(|| tool_history_not_advertised(&call.tool_id))?; - let arguments = serde_json::to_string(&call.arguments).map_err(|error| { - ApplicationError::ValidationError(format!("agent.tool_call_serialize_failed: {error}")) - })?; - let mut object = Map::new(); object.insert("id".to_string(), Value::String(call.call_id.clone())); object.insert("type".to_string(), Value::String("function".to_string())); @@ -301,7 +297,7 @@ fn encode_openai_tool_call( "function".to_string(), json!({ "name": model_alias, - "arguments": arguments, + "arguments": call.arguments.encode_for_replay(), }), ); diff --git a/src-tauri/crates/tt-application/src/services/agent_model_gateway/tests.rs b/src-tauri/crates/tt-application/src/services/agent_model_gateway/tests.rs index cd4acb4e..9ff6acca 100644 --- a/src-tauri/crates/tt-application/src/services/agent_model_gateway/tests.rs +++ b/src-tauri/crates/tt-application/src/services/agent_model_gateway/tests.rs @@ -15,7 +15,7 @@ use tt_domain::models::agent::{ AgentModelContentPart, AgentModelMessage, AgentModelRequest, AgentModelRole, AgentModelTool, AgentToolResult, }; -use tt_domain::models::tool::{ToolChoice, ToolId, ToolInvocation, ToolProviderId}; +use tt_domain::models::tool::{ToolArguments, ToolChoice, ToolId, ToolInvocation, ToolProviderId}; use tt_ports::repositories::chat_completion_repository::{ CHAT_COMPLETION_PROVIDER_STATE_FIELD, ChatCompletionNormalizationReport, ChatCompletionSource, }; @@ -509,7 +509,9 @@ fn openai_responses_continuation_sends_only_new_tool_results() { call: ToolInvocation { call_id: "call_1".to_string(), tool_id: ToolId::builtin("workspace.write_file").unwrap(), - arguments: json!({"path":"output/main.md","content":"hi"}), + arguments: ToolArguments::decode(Some( + &json!({"path":"output/main.md","content":"hi"}), + )), provider_metadata: Value::Null, }, }, diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/await_tool.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/await_tool.rs index c06409ad..72952bbb 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/await_tool.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/await_tool.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use std::time::{Duration, Instant}; use serde::{Deserialize, Serialize}; -use serde_json::{Value, json}; +use serde_json::{Map, Value, json}; use super::rendering::{DelegatedResultContinuationHint, render_await_content}; use super::task_status::task_is_terminal; @@ -68,6 +68,7 @@ impl AgentRuntimeService { &self, prepared: &PreparedInvocation, call: &ToolInvocation, + args: &Map, committed_count: usize, cancel: &mut AgentCancelReceiver, ) -> Result { @@ -76,7 +77,7 @@ impl AgentRuntimeService { let profile = &prepared.profile; let parent_tools = &prepared.request.tools; let started = Instant::now(); - let args = match serde_json::from_value::(call.arguments.clone()) { + let args = match serde_json::from_value::(Value::Object(args.clone())) { Ok(args) => args, Err(error) => { return Ok(tool_error_outcome( diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/delegate_tool.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/delegate_tool.rs index 00352076..fe9f55a9 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/delegate_tool.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/delegate_tool.rs @@ -1,7 +1,7 @@ use std::time::Instant; use serde::Deserialize; -use serde_json::{Value, json}; +use serde_json::{Map, Value, json}; use super::policy::validate_subagent_target; use super::tool_error::tool_error_outcome; @@ -29,11 +29,12 @@ impl AgentRuntimeService { run_id: &str, invocation_id: &str, call: &ToolInvocation, + args: &Map, profile: &ResolvedAgentProfile, _cancel: &AgentCancelReceiver, ) -> Result { let started = Instant::now(); - let args = match serde_json::from_value::(call.arguments.clone()) { + let args = match serde_json::from_value::(Value::Object(args.clone())) { Ok(args) => args, Err(error) => { return Ok(tool_error_outcome( diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/handoff_tool.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/handoff_tool.rs index 8f9641f8..272f3df3 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/handoff_tool.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/handoff_tool.rs @@ -36,10 +36,11 @@ impl AgentRuntimeService { run_id: &str, invocation_id: &str, call: &ToolInvocation, + args: &Map, profile: &ResolvedAgentProfile, ) -> Result { let started = Instant::now(); - let args = match serde_json::from_value::(call.arguments.clone()) { + let args = match serde_json::from_value::(Value::Object(args.clone())) { Ok(args) => args, Err(error) => { return Ok(tool_error_outcome( diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/list_tool.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/list_tool.rs index 0b175e92..ea3aedde 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/list_tool.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/list_tool.rs @@ -48,18 +48,10 @@ impl AgentRuntimeService { pub(in crate::services::agent_runtime_service) async fn dispatch_agent_list_tool( &self, call: &ToolInvocation, + args: &Map, profile: &ResolvedAgentProfile, ) -> Result { let started = Instant::now(); - let Some(args) = call.arguments.as_object() else { - return Ok(tool_error_outcome( - call, - "tool.invalid_arguments", - "arguments must be an object", - started.elapsed().as_millis(), - )); - }; - let purpose = match agent_list_purpose(args) { Ok(purpose) => purpose, Err(message) => { diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/task_return_tool.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/task_return_tool.rs index 4689e427..2c5068d7 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/task_return_tool.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/delegation/task_return_tool.rs @@ -23,6 +23,7 @@ impl AgentRuntimeService { run_id: &str, invocation_id: &str, call: &ToolInvocation, + args: &Map, exit_policy: AgentInvocationExitPolicy, profile: &ResolvedAgentProfile, ) -> Result { @@ -35,14 +36,6 @@ impl AgentRuntimeService { started.elapsed().as_millis(), )); } - let Some(args) = call.arguments.as_object() else { - return Ok(tool_error_outcome( - call, - "tool.invalid_arguments", - "arguments must be an object", - started.elapsed().as_millis(), - )); - }; let summary = match required_trimmed_string(args, "summary") { Ok(summary) => summary, Err(message) => { @@ -83,7 +76,7 @@ impl AgentRuntimeService { } let result_ref = WorkspacePath::parse(format!("agent-results/{invocation_id}.json"))?; let summary_ref = task_result_summary_path(&task.workspace_key)?; - let result_payload = match normalize_task_return_arguments(&call.arguments, profile) { + let result_payload = match normalize_task_return_arguments(args, profile) { Ok(arguments) => arguments, Err(error) => { return Ok(tool_error_outcome( @@ -230,13 +223,10 @@ impl TaskReturnArgumentError { } fn normalize_task_return_arguments( - arguments: &Value, + arguments: &Map, profile: &ResolvedAgentProfile, ) -> Result { - let Some(args) = arguments.as_object() else { - return Ok(arguments.clone()); - }; - let mut args = args.clone(); + let mut args = arguments.clone(); let Some(artifacts_value) = args.get("artifacts") else { return Ok(Value::Object(args)); }; diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/model_turn_display.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/model_turn_display.rs index f90fbb5a..df2537ac 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/model_turn_display.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/model_turn_display.rs @@ -281,7 +281,7 @@ fn string_field(value: &Value, key: &str) -> Option { mod tests { use super::*; use tt_domain::models::agent::{AgentModelMessage, AgentModelRole}; - use tt_domain::models::tool::{ToolId, ToolInvocation, ToolProviderId}; + use tt_domain::models::tool::{ToolArguments, ToolId, ToolInvocation, ToolProviderId}; #[test] fn model_turn_projection_preserves_canonical_tool_identity() { @@ -295,7 +295,7 @@ mod tests { vec![ToolInvocation { call_id: "call_mcp".to_string(), tool_id: tool_id.clone(), - arguments: Value::Null, + arguments: ToolArguments::empty(), provider_metadata: Value::Null, }], ); @@ -361,7 +361,7 @@ mod tests { ToolInvocation { call_id: "call_1".to_string(), tool_id: ToolId::builtin("workspace.write_file").unwrap(), - arguments: json!({}), + arguments: ToolArguments::empty(), provider_metadata: Value::Null, } } diff --git a/src-tauri/crates/tt-application/src/services/agent_runtime_service/tool_execution.rs b/src-tauri/crates/tt-application/src/services/agent_runtime_service/tool_execution.rs index 47cee2d1..40d1be98 100644 --- a/src-tauri/crates/tt-application/src/services/agent_runtime_service/tool_execution.rs +++ b/src-tauri/crates/tt-application/src/services/agent_runtime_service/tool_execution.rs @@ -1,6 +1,6 @@ use std::time::Instant; -use serde_json::{Value, json}; +use serde_json::{Map, Value, json}; use sha2::{Digest, Sha256}; use tokio_util::sync::CancellationToken; @@ -129,6 +129,19 @@ impl AgentRuntimeService { return Err(error); } + // Charge the budget before rejecting arguments so malformed calls cannot retry for free. + let args = match tool_invocation.arguments.as_map() { + Ok(args) => args, + Err(message) => { + return Ok(recoverable_tool_error( + tool_invocation, + "tool.invalid_arguments", + &message, + started.elapsed().as_millis(), + )); + } + }; + let call = tool_invocation; if exit_policy == AgentInvocationExitPolicy::RunFinishAllowed { self.transition_status(run_id, AgentRunStatus::DispatchingTool) @@ -162,29 +175,43 @@ impl AgentRuntimeService { started.elapsed().as_millis(), )) } else if builtin_name == Some(AGENT_LIST) { - self.dispatch_agent_list_tool(call, profile).await + self.dispatch_agent_list_tool(call, args, profile).await } else if builtin_name == Some(AGENT_DELEGATE) { Box::pin(self.dispatch_agent_delegate_tool( run_id, invocation_id, call, + args, profile, cancel, )) .await } else if builtin_name == Some(AGENT_AWAIT) { - self.dispatch_agent_await_tool(prepared, call, commit_ledger.explicit_count(), cancel) - .await + self.dispatch_agent_await_tool( + prepared, + call, + args, + commit_ledger.explicit_count(), + cancel, + ) + .await } else if builtin_name == Some(AGENT_HANDOFF) { - self.dispatch_agent_handoff_tool(run_id, invocation_id, call, profile) + self.dispatch_agent_handoff_tool(run_id, invocation_id, call, args, profile) .await } else if builtin_name == Some(TASK_RETURN) { - self.dispatch_task_return_tool(run_id, invocation_id, call, exit_policy, profile) - .await + self.dispatch_task_return_tool( + run_id, + invocation_id, + call, + args, + exit_policy, + profile, + ) + .await } else if !call.tool_id.is_builtin() { // The MCP service reports sent-but-unconfirmed calls explicitly. started_tool = false; - let outcome = self.call_mcp_tool(call, cancel).await?; + let outcome = self.call_mcp_tool(call, args, cancel).await?; started_tool = true; match outcome { McpCallOutcome::KnownResponse(response) => Ok(AgentToolDispatchOutcome { @@ -226,6 +253,7 @@ impl AgentRuntimeService { .dispatch_with_model_workspace_repository( run_id, call, + args, session, profile, &workspace_repository, @@ -233,7 +261,7 @@ impl AgentRuntimeService { .await } else { self.tool_dispatcher - .dispatch(run_id, call, session, profile) + .dispatch(run_id, call, args, session, profile) .await }; @@ -373,6 +401,7 @@ impl AgentRuntimeService { async fn call_mcp_tool( &self, call: &ToolInvocation, + args: &Map, cancel: &mut super::AgentCancelReceiver, ) -> Result { let cancellation = CancellationToken::new(); @@ -392,7 +421,7 @@ impl AgentRuntimeService { }; let outcome = self .mcp_service - .call_permitted_tool(&call.tool_id, call.arguments.clone(), cancellation) + .call_permitted_tool(&call.tool_id, Value::Object(args.clone()), cancellation) .await; if let Some(watcher) = watcher { watcher.abort(); @@ -793,7 +822,7 @@ mod tests { use tt_domain::models::agent::AgentToolResult; use tt_domain::models::agent::WorkspacePath; use tt_domain::models::tool::{ - InvocationToolSnapshot, ToolBinding, ToolDescriptor, ToolId, ToolInvocation, + InvocationToolSnapshot, ToolArguments, ToolBinding, ToolDescriptor, ToolId, ToolInvocation, ToolProviderId, ToolSnapshotId, }; @@ -807,7 +836,7 @@ mod tests { let invocation = ToolInvocation { call_id: "call_1".to_string(), tool_id: ToolId::builtin("workspace.finish").unwrap(), - arguments: Value::Null, + arguments: ToolArguments::empty(), provider_metadata: Value::Null, }; let result = AgentToolResult { diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/chat/read_messages.rs b/src-tauri/crates/tt-application/src/services/agent_tools/chat/read_messages.rs index 86b231a8..1786b06f 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/chat/read_messages.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/chat/read_messages.rs @@ -9,7 +9,7 @@ use super::{ chat_unavailable_message, role_as_str, visible_total_messages, }; use crate::errors::ApplicationError; -use crate::services::agent_tools::common::{ensure_only_args, object_args, tool_error}; +use crate::services::agent_tools::common::{ensure_only_args, tool_error}; use crate::services::agent_tools::dispatcher::AgentToolEffect; use tt_domain::errors::DomainError; use tt_domain::frozen_macros::{FrozenMacros, MAX_EXPANDED_TEXT_BYTES}; @@ -68,18 +68,9 @@ pub(in crate::services::agent_tools) async fn read_messages( group_chat_repository: &dyn GroupChatRepository, run_id: &str, call: &ToolInvocation, + args: &Map, macros: &FrozenMacros, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; if let Err(message) = ensure_only_args(args, &["messages"]) { return Ok(( tool_error(call, "tool.invalid_arguments", &message), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/chat/search.rs b/src-tauri/crates/tt-application/src/services/agent_tools/chat/search.rs index ea55dded..fb1594fe 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/chat/search.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/chat/search.rs @@ -9,7 +9,7 @@ use super::{ }; use crate::errors::ApplicationError; use crate::services::agent_tools::common::{ - object_args, optional_usize_arg, required_trimmed_string_arg, tool_error, + optional_usize_arg, required_trimmed_string_arg, tool_error, }; use crate::services::agent_tools::dispatcher::AgentToolEffect; use tt_domain::errors::DomainError; @@ -51,18 +51,9 @@ pub(in crate::services::agent_tools) async fn search( group_chat_repository: &dyn GroupChatRepository, run_id: &str, call: &ToolInvocation, + args: &Map, macros: &Arc, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let query = match required_trimmed_string_arg(args, "query") { Some(query) => query.to_string(), None => { diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/common.rs b/src-tauri/crates/tt-application/src/services/agent_tools/common.rs index 2835d1e6..9eb44b40 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/common.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/common.rs @@ -5,10 +5,6 @@ use super::structured::{ToolErrorStructured, structured_value}; use tt_domain::models::agent::AgentToolResult; use tt_domain::models::tool::ToolInvocation; -pub(super) fn object_args(call: &ToolInvocation) -> Option<&Map> { - call.arguments.as_object() -} - pub(super) fn required_trimmed_string_arg<'a>( args: &'a Map, key: &str, diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/dice/roll.rs b/src-tauri/crates/tt-application/src/services/agent_tools/dice/roll.rs index 5b56859d..12fe29a3 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/dice/roll.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/dice/roll.rs @@ -1,9 +1,10 @@ use rand::RngExt; use serde::Serialize; +use serde_json::{Map, Value}; use super::{MAX_ABS_MODIFIER, MAX_DICE, MAX_SIDES}; use crate::errors::ApplicationError; -use crate::services::agent_tools::common::{object_args, required_trimmed_string_arg, tool_error}; +use crate::services::agent_tools::common::{required_trimmed_string_arg, tool_error}; use crate::services::agent_tools::dispatcher::AgentToolEffect; use crate::services::agent_tools::structured::structured_value; use tt_domain::models::agent::AgentToolResult; @@ -32,17 +33,8 @@ struct DiceRollStructured<'a> { pub(in crate::services::agent_tools) async fn roll( call: &ToolInvocation, + args: &Map, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let Some(formula) = required_trimmed_string_arg(args, "formula") else { return Ok(( tool_error(call, "tool.invalid_arguments", "formula is required"), @@ -224,7 +216,7 @@ mod tests { use serde_json::{Value, json}; use super::{MAX_DICE, MAX_SIDES, parse_formula}; - use tt_domain::models::tool::{ToolId, ToolInvocation}; + use tt_domain::models::tool::{ToolArguments, ToolId, ToolInvocation}; #[test] fn reject_invalid_or_unbounded_formulas() { @@ -251,11 +243,13 @@ mod tests { let call = ToolInvocation { call_id: "call_dice".to_string(), tool_id: ToolId::builtin("dice.roll").unwrap(), - arguments: json!({ "formula": "1d1+2" }), + arguments: ToolArguments::decode(Some(&json!({ "formula": "1d1+2" }))), provider_metadata: Value::Null, }; - let (result, effect) = super::roll(&call).await.expect("dice roll result"); + let (result, effect) = super::roll(&call, call.arguments.as_map().unwrap()) + .await + .expect("dice roll result"); assert!(matches!( effect, diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/dispatcher.rs b/src-tauri/crates/tt-application/src/services/agent_tools/dispatcher.rs index eada9c39..e6a6687e 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/dispatcher.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/dispatcher.rs @@ -1,6 +1,8 @@ use std::sync::Arc; use std::time::Instant; +use serde_json::{Map, Value}; + use super::chat; use super::dice; use super::session::AgentToolSession; @@ -96,12 +98,14 @@ impl AgentToolDispatcher { &self, run_id: &str, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, profile: &ResolvedAgentProfile, ) -> Result { self.dispatch_with_model_workspace_repository( run_id, call, + args, session, profile, self.workspace_repository.as_ref(), @@ -113,6 +117,7 @@ impl AgentToolDispatcher { &self, run_id: &str, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, profile: &ResolvedAgentProfile, model_workspace_repository: &dyn WorkspaceRepository, @@ -126,6 +131,7 @@ impl AgentToolDispatcher { self.group_chat_repository.as_ref(), run_id, call, + args, &session.frozen_macros, ) .await? @@ -137,6 +143,7 @@ impl AgentToolDispatcher { self.group_chat_repository.as_ref(), run_id, call, + args, &session.frozen_macros, ) .await? @@ -145,15 +152,15 @@ impl AgentToolDispatcher { // WorldInfo activation is a hidden run input fact, not a model-visible // workspace file; invocation workspace policy must not gate this read. let prompt_snapshot = self.read_run_prompt_snapshot(run_id).await?; - world_info::read_activated(&prompt_snapshot, call)? + world_info::read_activated(&prompt_snapshot, call, args)? } - dice::DICE_ROLL => dice::roll(call).await?, + dice::DICE_ROLL => dice::roll(call, args).await?, skill::SKILL_LIST => skill::list(call, session, profile).await?, skill::SKILL_SEARCH => { - skill::search(self.skill_service.as_ref(), call, session, profile).await? + skill::search(self.skill_service.as_ref(), call, args, session, profile).await? } skill::SKILL_READ => { - skill::read(self.skill_service.as_ref(), call, session, profile).await? + skill::read(self.skill_service.as_ref(), call, args, session, profile).await? } skill::SKILL_SCRIPT => { let prompt_snapshot = self.read_run_prompt_snapshot(run_id).await?; @@ -166,30 +173,34 @@ impl AgentToolDispatcher { prompt_snapshot, }, call, + args, session, profile, ) .await? } workspace::WORKSPACE_LIST_FILES => { - workspace::list_files(model_workspace_repository, run_id, call).await? + workspace::list_files(model_workspace_repository, run_id, call, args).await? } workspace::WORKSPACE_SEARCH_FILES => { - workspace::search_files(model_workspace_repository, run_id, call).await? + workspace::search_files(model_workspace_repository, run_id, call, args).await? } workspace::WORKSPACE_READ_FILE => { - workspace::read_file(model_workspace_repository, run_id, call, session).await? + workspace::read_file(model_workspace_repository, run_id, call, args, session) + .await? } workspace::WORKSPACE_WRITE_FILE => { - workspace::write_file(model_workspace_repository, run_id, call, session).await? + workspace::write_file(model_workspace_repository, run_id, call, args, session) + .await? } workspace::WORKSPACE_APPLY_PATCH => { - workspace::apply_patch(model_workspace_repository, run_id, call, session).await? + workspace::apply_patch(model_workspace_repository, run_id, call, args, session) + .await? } workspace::WORKSPACE_COMMIT => { - workspace::commit(model_workspace_repository, run_id, call, profile).await? + workspace::commit(model_workspace_repository, run_id, call, args, profile).await? } - workspace::WORKSPACE_FINISH => workspace::finish(call)?, + workspace::WORKSPACE_FINISH => workspace::finish(call, args)?, other => { return Err(ApplicationError::InternalError(format!( "tool.dispatch_handler_missing: admitted builtin tool `builtin:{other}` has no execution handler" diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/list.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/list.rs index df5583f2..828d5402 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/list.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/list.rs @@ -114,7 +114,7 @@ mod tests { use super::{build_result, skill_is_visible}; use tt_domain::models::agent::profile::AgentSkillPolicy; use tt_domain::models::skill::{SkillIndexEntry, SkillScope, SkillSourceRef}; - use tt_domain::models::tool::{ToolId, ToolInvocation}; + use tt_domain::models::tool::{ToolArguments, ToolId, ToolInvocation}; #[test] fn wildcard_deny_hides_skills_even_when_visible_allows_all() { @@ -141,7 +141,7 @@ mod tests { let call = ToolInvocation { call_id: "call_skill_list".to_string(), tool_id: ToolId::builtin("skill.list").unwrap(), - arguments: json!({}), + arguments: ToolArguments::empty(), provider_metadata: Value::Null, }; diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/read.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/read.rs index 1dee3d09..625c88ee 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/read.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/read.rs @@ -1,7 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::super::common::{ - ensure_only_args, object_args, optional_usize_arg, required_trimmed_string_arg, tool_error, + ensure_only_args, optional_usize_arg, required_trimmed_string_arg, tool_error, }; use super::super::dispatcher::AgentToolEffect; use super::super::session::AgentToolSession; @@ -31,19 +32,10 @@ struct SkillReadStructured<'a> { pub(in crate::services::agent_tools) async fn read( skill_service: &SkillService, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, profile: &ResolvedAgentProfile, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; if let Err(message) = ensure_only_args(args, &["name", "path", "start_line", "line_count"]) { return Ok(( tool_error(call, "tool.invalid_arguments", &message), @@ -58,7 +50,7 @@ pub(in crate::services::agent_tools) async fn read( }; let path = args .get("path") - .and_then(serde_json::Value::as_str) + .and_then(Value::as_str) .map(str::trim) .filter(|value| !value.is_empty()) .unwrap_or("SKILL.md"); diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script.rs index 87031925..648a3ecc 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script.rs @@ -2,9 +2,7 @@ use std::collections::HashMap; use serde_json::{Map, Value, json}; -use super::super::common::{ - ensure_only_args, object_args, required_trimmed_string_arg, tool_error, -}; +use super::super::common::{ensure_only_args, required_trimmed_string_arg, tool_error}; use super::super::dispatcher::AgentToolEffect; use super::super::session::AgentToolSession; use super::super::workspace::workspace_access_policy; @@ -44,6 +42,7 @@ pub(in crate::services::agent_tools) struct ScriptContext<'a> { pub(in crate::services::agent_tools) async fn script( context: ScriptContext<'_>, call: &ToolInvocation, + args: &Map, session: &AgentToolSession, profile: &ResolvedAgentProfile, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { @@ -54,16 +53,6 @@ pub(in crate::services::agent_tools) async fn script( run_id, prompt_snapshot, } = context; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; if let Err(message) = ensure_only_args(args, &["skill", "script", "args"]) { return Ok(( tool_error(call, "tool.invalid_arguments", &message), @@ -83,7 +72,7 @@ pub(in crate::services::agent_tools) async fn script( )); }; let script_args = match args.get("args") { - None => Value::Object(serde_json::Map::new()), + None => Value::Object(Map::new()), Some(value) if value.is_object() => value.clone(), Some(_) => { return Ok(( diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/execution.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/execution.rs index b7366c37..a2ee1b19 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/execution.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/execution.rs @@ -106,6 +106,7 @@ async fn success_builds_result_and_passes_workspace_context() { profile.workspace.visible_roots = vec!["profile-only".to_string()]; profile.workspace.writable_roots = vec!["profile-only".to_string()]; + let tool_call = call(json!({ "skill": "demo", "script": "helper", "args": { "n": 7 } })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -122,7 +123,8 @@ async fn success_builds_result_and_passes_workspace_context() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper", "args": { "n": 7 } })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -163,6 +165,7 @@ async fn module_snapshot_contains_only_script_modules() { }); let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, _) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -182,7 +185,8 @@ async fn module_snapshot_contains_only_script_modules() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -230,6 +234,7 @@ async fn frozen_host_context_is_passed_to_engine() { } }); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, _) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -246,7 +251,8 @@ async fn frozen_host_context_is_passed_to_engine() { run_id: "run-1", prompt_snapshot, }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/mod.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/mod.rs index 5aff9d9a..a0d694dc 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/mod.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/mod.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use async_trait::async_trait; -use serde_json::{Value, json}; +use serde_json::{Map, Value, json}; use tokio::sync::Mutex; use super::*; @@ -26,7 +26,7 @@ use tt_domain::models::skill::{ SkillScope, SkillScopeFilter, SkillScopeRetargetRequest, SkillScopeRetargetResult, SkillSearchRequest, SkillSearchResult, SkillWriteRequest, }; -use tt_domain::models::tool::ToolId; +use tt_domain::models::tool::{ToolArguments, ToolId}; use tt_ports::repositories::skill_repository::SkillRepository; use tt_ports::repositories::workspace_repository::{ WorkspaceAppendResult, WorkspaceEntry, WorkspaceEntryKind, WorkspaceFile, WorkspaceFileList, @@ -506,11 +506,15 @@ fn call(arguments: Value) -> ToolInvocation { ToolInvocation { call_id: "call_skill_script".to_string(), tool_id: ToolId::builtin("skill.run_script").unwrap(), - arguments, + arguments: ToolArguments::decode(Some(&arguments)), provider_metadata: Value::Null, } } +fn call_args(call: &ToolInvocation) -> &Map { + call.arguments.as_map().expect("test arguments are objects") +} + fn empty_prompt_snapshot() -> Value { json!({ "worldInfoActivation": { "entries": [] }, @@ -529,6 +533,7 @@ async fn run_with_repo_and_outcome( outcome, requests: Mutex::new(Vec::new()), }); + let tool_call = call(arguments); script( ScriptContext { skill_service: &SkillService::new(Arc::new(repo)), @@ -543,7 +548,8 @@ async fn run_with_repo_and_outcome( run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(arguments), + &tool_call, + call_args(&tool_call), &session, &profile, ) diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/workspace.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/workspace.rs index ca7b4916..22dd443a 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/workspace.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/script/tests/workspace.rs @@ -23,6 +23,7 @@ async fn script_writes_produce_workspace_file_written_effect() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -33,7 +34,8 @@ async fn script_writes_produce_workspace_file_written_effect() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -95,6 +97,7 @@ async fn multiple_files_written_produce_batch_effect() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -105,7 +108,8 @@ async fn multiple_files_written_produce_batch_effect() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -165,6 +169,7 @@ async fn write_outside_writable_roots_is_rejected_before_any_disk_write() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -175,7 +180,8 @@ async fn write_outside_writable_roots_is_rejected_before_any_disk_write() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -223,6 +229,7 @@ async fn existing_file_write_uses_snapshot_sha_guard() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -233,7 +240,8 @@ async fn existing_file_write_uses_snapshot_sha_guard() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -280,6 +288,7 @@ async fn stale_conflict_fails_without_side_effects() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -290,7 +299,8 @@ async fn stale_conflict_fails_without_side_effects() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -338,6 +348,7 @@ async fn mid_batch_failure_preserves_already_written_files_in_effect() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -348,7 +359,8 @@ async fn mid_batch_failure_preserves_already_written_files_in_effect() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) @@ -391,6 +403,7 @@ async fn truncated_workspace_snapshot_returns_tool_error() { let session = session_with_skill("demo"); let profile = profile(true); + let tool_call = call(json!({ "skill": "demo", "script": "helper" })); let (result, effect) = script( ScriptContext { skill_service: &SkillService::new(Arc::new(FakeSkillRepo { @@ -407,7 +420,8 @@ async fn truncated_workspace_snapshot_returns_tool_error() { run_id: "run-1", prompt_snapshot: empty_prompt_snapshot(), }, - &call(json!({ "skill": "demo", "script": "helper" })), + &tool_call, + call_args(&tool_call), &session, &profile, ) diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/skill/search.rs b/src-tauri/crates/tt-application/src/services/agent_tools/skill/search.rs index a68e7b68..4ab9b0d0 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/skill/search.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/skill/search.rs @@ -1,8 +1,7 @@ use serde::Serialize; +use serde_json::{Map, Value}; -use super::super::common::{ - object_args, optional_usize_arg, required_trimmed_string_arg, tool_error, -}; +use super::super::common::{optional_usize_arg, required_trimmed_string_arg, tool_error}; use super::super::dispatcher::AgentToolEffect; use super::super::session::AgentToolSession; use super::list::skill_is_visible; @@ -51,19 +50,10 @@ struct SkillSearchHitStructured<'a> { pub(in crate::services::agent_tools) async fn search( skill_service: &SkillService, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, profile: &ResolvedAgentProfile, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let Some(name) = required_trimmed_string_arg(args, "name") else { return Ok(( tool_error(call, "tool.invalid_arguments", "name is required"), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/apply_patch.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/apply_patch.rs index 43f02008..79846cfb 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/apply_patch.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/apply_patch.rs @@ -1,7 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::args::{ - classify_workspace_io_error, ensure_writable_workspace_path, object_args, optional_bool_arg, + classify_workspace_io_error, ensure_writable_workspace_path, optional_bool_arg, parse_workspace_path, required_raw_string_arg, required_trimmed_string_arg, tool_error, }; use super::policy::workspace_access_policy; @@ -31,19 +32,10 @@ pub(in crate::services::agent_tools) async fn apply_patch( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let Some(path) = required_trimmed_string_arg(args, "path") else { return Ok(( tool_error(call, "tool.invalid_arguments", "path is required"), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/args.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/args.rs index 662b7125..8cbf3897 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/args.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/args.rs @@ -6,8 +6,8 @@ use tt_domain::models::agent::{AgentToolResult, WorkspacePath}; use tt_domain::models::tool::ToolInvocation; pub(super) use crate::services::agent_tools::common::{ - WORKSPACE_PATH_IS_DIRECTORY_CODE, ensure_only_args, object_args, optional_bool_arg, - optional_usize_arg, required_raw_string_arg, required_trimmed_string_arg, tool_error, + WORKSPACE_PATH_IS_DIRECTORY_CODE, ensure_only_args, optional_bool_arg, optional_usize_arg, + required_raw_string_arg, required_trimmed_string_arg, tool_error, workspace_path_is_directory_message, }; diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/commit.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/commit.rs index 48ee5296..6fc0b652 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/commit.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/commit.rs @@ -1,8 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::args::{ - ensure_visible_workspace_path, object_args, parse_workspace_path, required_trimmed_string_arg, - tool_error, + ensure_visible_workspace_path, parse_workspace_path, required_trimmed_string_arg, tool_error, }; use super::policy::workspace_access_policy; use crate::errors::ApplicationError; @@ -26,12 +26,11 @@ pub(in crate::services::agent_tools) async fn commit( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, profile: &ResolvedAgentProfile, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let args = object_args(call); - let path = args - .and_then(|args| required_trimmed_string_arg(args, "path")) + let path = required_trimmed_string_arg(args, "path") .unwrap_or(profile.output.message_body_path.as_str()); let path = match parse_workspace_path(path) { Ok(path) => path, @@ -41,7 +40,7 @@ pub(in crate::services::agent_tools) async fn commit( return Ok((error.into_tool_result(call), AgentToolEffect::None)); } - let mode = match args.and_then(|args| required_trimmed_string_arg(args, "mode")) { + let mode = match required_trimmed_string_arg(args, "mode") { Some("append") => AgentChatCommitMode::Append, Some("replace") | None => AgentChatCommitMode::Replace, Some(other) => { @@ -55,9 +54,7 @@ pub(in crate::services::agent_tools) async fn commit( )); } }; - let reason = args - .and_then(|args| required_trimmed_string_arg(args, "reason")) - .map(str::to_string); + let reason = required_trimmed_string_arg(args, "reason").map(str::to_string); Ok(( AgentToolResult { diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/finish.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/finish.rs index 91bd7d18..7cae27c6 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/finish.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/finish.rs @@ -1,4 +1,5 @@ use serde::Serialize; +use serde_json::{Map, Value}; use crate::errors::ApplicationError; use tt_domain::models::agent::AgentToolResult; @@ -15,16 +16,14 @@ struct WorkspaceFinishStructured<'a> { pub(in crate::services::agent_tools) fn finish( call: &ToolInvocation, + args: &Map, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let args = call.arguments.as_object(); let result = AgentToolResult { call_id: call.call_id.clone(), tool_id: call.tool_id.clone(), content: "Finished the Agent run.".to_string(), structured: structured_value(WorkspaceFinishStructured { - reason: args - .and_then(|args| args.get("reason")) - .and_then(serde_json::Value::as_str), + reason: args.get("reason").and_then(Value::as_str), }), is_error: false, error_code: None, diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/list_files.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/list_files.rs index 55b796b0..8417d494 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/list_files.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/list_files.rs @@ -1,8 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::args::{ - ensure_visible_workspace_path, object_args, optional_list_path_arg, optional_usize_arg, - tool_error, + ensure_visible_workspace_path, optional_list_path_arg, optional_usize_arg, tool_error, }; use super::policy::workspace_access_policy; use super::render::{filter_visible_entries, render_file_list}; @@ -34,18 +34,9 @@ pub(in crate::services::agent_tools) async fn list_files( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let path = match optional_list_path_arg(args, "path") { Ok(path) => path, Err(message) => { diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/read_file.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/read_file.rs index 28bfd055..e3b5e4a8 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/read_file.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/read_file.rs @@ -1,7 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::args::{ - classify_workspace_io_error, ensure_only_args, ensure_visible_workspace_path, object_args, + classify_workspace_io_error, ensure_only_args, ensure_visible_workspace_path, optional_usize_arg, parse_workspace_path, required_trimmed_string_arg, tool_error, }; use super::policy::workspace_access_policy; @@ -31,19 +32,10 @@ pub(in crate::services::agent_tools) async fn read_file( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; if let Err(message) = ensure_only_args(args, &["path", "start_line", "line_count"]) { return Ok(( tool_error(call, "tool.invalid_arguments", &message), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/search_files.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/search_files.rs index a115a363..0aa7e2a7 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/search_files.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/search_files.rs @@ -1,7 +1,8 @@ use serde::Serialize; +use serde_json::{Map, Value}; use super::args::{ - ensure_visible_workspace_path, object_args, optional_list_path_arg, optional_usize_arg, + ensure_visible_workspace_path, optional_list_path_arg, optional_usize_arg, required_trimmed_string_arg, tool_error, }; use super::policy::{WorkspaceAccessPolicy, workspace_access_policy}; @@ -64,18 +65,9 @@ pub(in crate::services::agent_tools) async fn search_files( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let Some(query) = required_trimmed_string_arg(args, "query") else { return Ok(( tool_error(call, "tool.invalid_arguments", "query is required"), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/tests.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/tests.rs index a731b5e2..dbe89e7e 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/tests.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/tests.rs @@ -3,7 +3,7 @@ use std::sync::Mutex; use async_trait::async_trait; use chrono::Utc; -use serde_json::json; +use serde_json::{Map, Value, json}; use sha2::{Digest, Sha256}; use super::args::classify_workspace_io_error; @@ -19,7 +19,7 @@ use tt_domain::models::agent::{ WorkspaceInputManifest, WorkspaceManifest, WorkspacePath, WorkspacePersistentChangeSet, WorkspaceRootCommit, WorkspaceRootLifecycle, WorkspaceRootMount, WorkspaceRootScope, }; -use tt_domain::models::tool::{ToolId, ToolInvocation}; +use tt_domain::models::tool::{ToolArguments, ToolId, ToolInvocation}; use tt_ports::repositories::workspace_repository::{ WorkspaceAppendResult, WorkspaceEntry, WorkspaceFile, WorkspaceFileList, WorkspaceRepository, WorkspaceWriteGuard, @@ -69,7 +69,7 @@ fn make_test_tool_call(name: &str) -> ToolInvocation { ToolInvocation { call_id: "call_test".to_string(), tool_id: ToolId::builtin(name).unwrap(), - arguments: json!({}), + arguments: ToolArguments::empty(), provider_metadata: json!({}), } } @@ -92,7 +92,7 @@ async fn workspace_read_invalid_path_returns_canonical_tool_error() { let mut session = AgentToolSession::default(); let call = workspace_call("workspace.read_file", json!({ "path": "../secrets.json" })); - let (result, effect) = read_file(&repository, "run", &call, &mut session) + let (result, effect) = read_file(&repository, "run", &call, call_args(&call), &mut session) .await .expect("invalid model path must remain recoverable"); @@ -124,7 +124,7 @@ async fn workspace_read_hidden_path_returns_recoverable_tool_error() { json!({ "path": "input/prompt_snapshot.json" }), ); - let (result, effect) = read_file(&repository, "run", &call, &mut session) + let (result, effect) = read_file(&repository, "run", &call, call_args(&call), &mut session) .await .expect("hidden model path must remain recoverable"); @@ -150,7 +150,7 @@ async fn workspace_read_defaults_to_a_preview_for_oversized_files() { let mut session = AgentToolSession::default(); let call = workspace_call("workspace.read_file", json!({ "path": "output/large.md" })); - let (result, _) = read_file(&repository, "run", &call, &mut session) + let (result, _) = read_file(&repository, "run", &call, call_args(&call), &mut session) .await .expect("large read should return a preview"); @@ -169,7 +169,7 @@ async fn workspace_read_keeps_a_large_single_line_out_of_the_next_model_request( let mut session = AgentToolSession::default(); let call = workspace_call("workspace.read_file", json!({ "path": "output/large.txt" })); - let (result, _) = read_file(&repository, "run", &call, &mut session) + let (result, _) = read_file(&repository, "run", &call, call_args(&call), &mut session) .await .expect("large read should return a preview"); @@ -191,7 +191,7 @@ async fn workspace_write_root_returns_recoverable_tool_error() { }), ); - let (result, effect) = write_file(&repository, "run", &call, &mut session) + let (result, effect) = write_file(&repository, "run", &call, call_args(&call), &mut session) .await .expect("non-writable model path must remain recoverable"); @@ -212,16 +212,18 @@ async fn workspace_write_existing_file_requires_prior_read() { let repository = TestWorkspaceRepository::with_file("output/main.md", "old text"); let mut session = AgentToolSession::default(); + let write_call = workspace_call( + "workspace.write_file", + json!({ + "path": "output/main.md", + "content": "new text", + }), + ); let (result, _) = write_file( &repository, "run", - &workspace_call( - "workspace.write_file", - json!({ - "path": "output/main.md", - "content": "new text", - }), - ), + &write_call, + call_args(&write_call), &mut session, ) .await @@ -242,19 +244,20 @@ async fn workspace_write_existing_file_requires_prior_read() { ); let read_call = workspace_call("workspace.read_file", json!({ "path": "output/main.md" })); - read_file(&repository, "run", &read_call, &mut session) - .await - .expect("read file"); + read_file( + &repository, + "run", + &read_call, + call_args(&read_call), + &mut session, + ) + .await + .expect("read file"); let (result, effect) = write_file( &repository, "run", - &workspace_call( - "workspace.write_file", - json!({ - "path": "output/main.md", - "content": "new text", - }), - ), + &write_call, + call_args(&write_call), &mut session, ) .await @@ -275,17 +278,37 @@ async fn workspace_patch_partial_failure_requires_full_read_before_retry() { let repository = TestWorkspaceRepository::with_file("output/main.md", "alpha beta\ngamma"); let mut session = AgentToolSession::default(); + let partial_read = workspace_call( + "workspace.read_file", + json!({ + "path": "output/main.md", + "start_line": 1, + "line_count": 1 + }), + ); + let full_read = workspace_call("workspace.read_file", json!({ "path": "output/main.md" })); + let missing_patch = workspace_call( + "workspace.apply_patch", + json!({ + "path": "output/main.md", + "old_string": "delta", + "new_string": "omega" + }), + ); + let patch = workspace_call( + "workspace.apply_patch", + json!({ + "path": "output/main.md", + "old_string": "alpha", + "new_string": "omega" + }), + ); + read_file( &repository, "run", - &workspace_call( - "workspace.read_file", - json!({ - "path": "output/main.md", - "start_line": 1, - "line_count": 1 - }), - ), + &partial_read, + call_args(&partial_read), &mut session, ) .await @@ -294,14 +317,8 @@ async fn workspace_patch_partial_failure_requires_full_read_before_retry() { let (result, _) = apply_patch( &repository, "run", - &workspace_call( - "workspace.apply_patch", - json!({ - "path": "output/main.md", - "old_string": "delta", - "new_string": "omega" - }), - ), + &missing_patch, + call_args(&missing_patch), &mut session, ) .await @@ -311,21 +328,9 @@ async fn workspace_patch_partial_failure_requires_full_read_before_retry() { Some("workspace.patch_requires_full_read") ); - let (result, _) = apply_patch( - &repository, - "run", - &workspace_call( - "workspace.apply_patch", - json!({ - "path": "output/main.md", - "old_string": "alpha", - "new_string": "omega" - }), - ), - &mut session, - ) - .await - .expect("patch blocked after partial failure"); + let (result, _) = apply_patch(&repository, "run", &patch, call_args(&patch), &mut session) + .await + .expect("patch blocked after partial failure"); assert_eq!( result.error_code.as_deref(), Some("workspace.patch_requires_full_read") @@ -334,26 +339,15 @@ async fn workspace_patch_partial_failure_requires_full_read_before_retry() { read_file( &repository, "run", - &workspace_call("workspace.read_file", json!({ "path": "output/main.md" })), + &full_read, + call_args(&full_read), &mut session, ) .await .expect("full read"); - let (result, _) = apply_patch( - &repository, - "run", - &workspace_call( - "workspace.apply_patch", - json!({ - "path": "output/main.md", - "old_string": "alpha", - "new_string": "omega" - }), - ), - &mut session, - ) - .await - .expect("patch after full read"); + let (result, _) = apply_patch(&repository, "run", &patch, call_args(&patch), &mut session) + .await + .expect("patch after full read"); assert!(!result.is_error); assert_eq!( @@ -370,11 +364,15 @@ fn workspace_call(name: &str, arguments: serde_json::Value) -> ToolInvocation { ToolInvocation { call_id: format!("call_{}", name.replace('.', "_")), tool_id: ToolId::builtin(name).unwrap(), - arguments, + arguments: ToolArguments::decode(Some(&arguments)), provider_metadata: serde_json::Value::Null, } } +fn call_args(call: &ToolInvocation) -> &Map { + call.arguments.as_map().expect("test arguments are objects") +} + struct TestWorkspaceRepository { files: Mutex>, } diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/write_file.rs b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/write_file.rs index b0cab2d6..9073a0cb 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/workspace/write_file.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/workspace/write_file.rs @@ -2,7 +2,7 @@ use serde::Serialize; use serde_json::{Map, Value}; use super::args::{ - classify_workspace_io_error, ensure_writable_workspace_path, object_args, parse_workspace_path, + classify_workspace_io_error, ensure_writable_workspace_path, parse_workspace_path, required_raw_string_arg, required_trimmed_string_arg, tool_error, }; use super::policy::workspace_access_policy; @@ -34,19 +34,10 @@ pub(in crate::services::agent_tools) async fn write_file( workspace_repository: &dyn WorkspaceRepository, run_id: &str, call: &ToolInvocation, + args: &Map, session: &mut AgentToolSession, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { let policy = workspace_access_policy(workspace_repository, run_id).await?; - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let Some(path) = required_trimmed_string_arg(args, "path") else { return Ok(( tool_error(call, "tool.invalid_arguments", "path is required"), diff --git a/src-tauri/crates/tt-application/src/services/agent_tools/world_info/read_activated.rs b/src-tauri/crates/tt-application/src/services/agent_tools/world_info/read_activated.rs index f7c0c577..0f15b32e 100644 --- a/src-tauri/crates/tt-application/src/services/agent_tools/world_info/read_activated.rs +++ b/src-tauri/crates/tt-application/src/services/agent_tools/world_info/read_activated.rs @@ -6,7 +6,7 @@ use super::{ MAX_WORLDINFO_TOTAL_READ_CHARS, }; use crate::errors::ApplicationError; -use crate::services::agent_tools::common::{object_args, tool_error}; +use crate::services::agent_tools::common::tool_error; use crate::services::agent_tools::dispatcher::AgentToolEffect; use tt_domain::models::agent::AgentToolResult; use tt_domain::models::tool::ToolInvocation; @@ -104,17 +104,8 @@ struct RenderedEntry { pub(in crate::services::agent_tools) fn read_activated( prompt_snapshot: &Value, call: &ToolInvocation, + args: &Map, ) -> Result<(AgentToolResult, AgentToolEffect), ApplicationError> { - let Some(args) = object_args(call) else { - return Ok(( - tool_error( - call, - "tool.invalid_arguments", - "arguments must be an object", - ), - AgentToolEffect::None, - )); - }; let request = match parse_request(args) { Ok(request) => request, Err(message) => { diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/messages.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/messages.rs index f59beda2..49d3dd6b 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/messages.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/messages.rs @@ -133,11 +133,9 @@ pub(super) fn convert_messages( if use_tools { blocks.extend(convert_openai_tool_calls_to_claude_blocks(&tool_calls)); } else { - blocks.extend( - tool_calls.iter().map(|call| { - normalize_claude_text_block(&call.arguments.to_string()) - }), - ); + blocks.extend(tool_calls.iter().map(|call| { + normalize_claude_text_block(&call.arguments.encode_for_replay()) + })); } blocks } else { diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/tools.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/tools.rs index a4dd4be9..5b8e6c03 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/tools.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/claude/tools.rs @@ -15,7 +15,7 @@ pub(super) fn convert_openai_tool_calls_to_claude_blocks( "type": "tool_use", "id": tool_call.id, "name": tool_call.name, - "input": tool_call.arguments, + "input": tool_call.arguments.to_replay_object(), }) }) .collect() diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/gemini_interactions.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/gemini_interactions.rs index c51694f1..c26d8f3f 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/gemini_interactions.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/gemini_interactions.rs @@ -414,7 +414,7 @@ fn build_function_call_step(tool_call: &OpenAiToolCall) -> Value { "type": "function_call", "id": tool_call.id.clone(), "name": tool_call.name.clone(), - "arguments": tool_call.arguments.clone(), + "arguments": tool_call.arguments.to_replay_object(), }); if let Some(signature) = tool_call.signature.as_deref() diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/makersuite.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/makersuite.rs index fc246816..ff75f339 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/makersuite.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/makersuite.rs @@ -723,7 +723,7 @@ fn convert_openai_tool_calls_to_parts( .map(|tool_call| { let mut function_call = json!({ "name": tool_call.name, - "args": tool_call.arguments, + "args": tool_call.arguments.to_replay_object(), }); if supports_function_call_ids { function_call["id"] = Value::String(tool_call.id.clone()); diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/mod.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/mod.rs index f75c673c..f6215b1b 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/mod.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/mod.rs @@ -107,6 +107,66 @@ mod tests { use super::build_payload; use tt_ports::repositories::chat_completion_repository::ChatCompletionSource; + #[test] + fn converted_tool_call_history_replays_unusable_arguments_as_an_empty_object() { + let error = "## Tool error\n\nRequest rejected"; + for arguments in ["null", "[1,2]", r#"{"path":"#] { + for (format, arguments_path, error_path) in [ + ("openai_responses", "/input/1/arguments", "/input/2/output"), + ( + "claude_messages", + "/messages/1/content/0/input", + "/messages/2/content/0/content", + ), + ( + "gemini_generate_content", + "/contents/1/parts/0/functionCall/args", + "/contents/2/parts/0/functionResponse/response/content", + ), + ( + "gemini_interactions", + "/input/1/arguments", + "/input/2/result/0/text", + ), + ] { + let payload = json!({ + "chat_completion_source": "custom", + "custom_api_format": format, + "model": "test-model", + "tools": [{ "type": "function", "function": { + "name": "read_file", "parameters": { "type": "object" } + } }], + "messages": [ + { "role": "user", "content": "Read a file" }, + { "role": "assistant", "tool_calls": [{ + "id": "call_1", "type": "function", + "function": { "name": "read_file", "arguments": arguments } + }] }, + { "role": "tool", "tool_call_id": "call_1", "content": error } + ] + }); + let (_, upstream) = build_payload( + ChatCompletionSource::Custom, + payload.as_object().unwrap().clone(), + ) + .unwrap(); + let replay = upstream + .pointer(arguments_path) + .unwrap_or_else(|| panic!("{format} missing {arguments_path}: {upstream}")); + let replay = match replay.as_str() { + Some(encoded) => serde_json::from_str::(encoded).unwrap(), + None => replay.clone(), + }; + assert_eq!(replay, json!({}), "{format}: {arguments}"); + assert_eq!( + upstream.pointer(error_path), + Some(&json!(error)), + "{format}" + ); + } + } + } + #[test] fn claude_leaves_additional_body_overrides_to_service_layer() { let payload = json!({ diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/openai_responses.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/openai_responses.rs index 305ef6d3..1f68747f 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/openai_responses.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/openai_responses.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use serde_json::{Map, Number, Value, json}; use crate::errors::ApplicationError; +use tt_domain::models::tool::ToolArguments; use tt_ports::repositories::chat_completion_repository::CHAT_COMPLETION_PROVIDER_STATE_FIELD; use super::content_parts::{InputPart, MediaPart, MediaSource, parse_openai_chat_content}; @@ -483,7 +484,7 @@ fn assistant_function_calls( "Assistant tool_call function is missing name".to_string(), ) })?; - let arguments = function_call_arguments(function.get("arguments"))?; + let arguments = ToolArguments::decode(function.get("arguments")).encode_for_replay(); Ok(ResponsesFunctionCall { call_id, @@ -494,18 +495,6 @@ fn assistant_function_calls( .collect() } -fn function_call_arguments(value: Option<&Value>) -> Result { - match value { - Some(Value::String(arguments)) => Ok(arguments.clone()), - Some(Value::Null) | None => Ok("{}".to_string()), - Some(value) => serde_json::to_string(value).map_err(|error| { - ApplicationError::ValidationError(format!( - "Assistant tool_call arguments are not serializable: {error}" - )) - }), - } -} - fn message_native_openai_responses_output( message: &Map, ) -> Result>, ApplicationError> { diff --git a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/tool_calls.rs b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/tool_calls.rs index 08a69a9a..91497197 100644 --- a/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/tool_calls.rs +++ b/src-tauri/crates/tt-application/src/services/chat_completion_service/payload/tool_calls.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use serde_json::{Map, Value, json}; +use tt_domain::models::tool::ToolArguments; use crate::errors::ApplicationError; @@ -12,7 +13,7 @@ const DEFAULT_TOOL_NAME: &str = "tool"; pub(super) struct OpenAiToolCall { pub id: String, pub name: String, - pub arguments: Value, + pub arguments: ToolArguments, pub signature: Option, } @@ -37,9 +38,8 @@ pub(super) fn extract_openai_tool_calls(value: Option<&Value>) -> Vec) -> Value { - let Some(value) = value else { - return Value::Object(Map::new()); - }; - - match value { - Value::String(raw) => { - serde_json::from_str::(raw).unwrap_or_else(|_| Value::String(raw.to_string())) - } - Value::Null => Value::Object(Map::new()), - other => other.clone(), - } -} - fn strict_tool_call_ids( value: Option<&Value>, message_index: usize, @@ -249,7 +235,7 @@ mod tests { assert_eq!(calls[0].id, "call_1"); assert_eq!(calls[0].name, "weather"); assert_eq!(calls[0].signature.as_deref(), Some("sig_1")); - assert_eq!(calls[0].arguments["city"], "Paris"); + assert_eq!(calls[0].arguments.as_map().unwrap()["city"], "Paris"); } #[test] diff --git a/src-tauri/crates/tt-application/src/services/tool_request_gate.rs b/src-tauri/crates/tt-application/src/services/tool_request_gate.rs index 030d66fc..33287d51 100644 --- a/src-tauri/crates/tt-application/src/services/tool_request_gate.rs +++ b/src-tauri/crates/tt-application/src/services/tool_request_gate.rs @@ -118,8 +118,8 @@ pub(crate) enum ToolRequestGateError { mod tests { use serde_json::{Value, json}; use tt_domain::models::tool::{ - InvocationToolSnapshot, ToolBinding, ToolChoice, ToolDescriptor, ToolId, ToolInvocation, - ToolProviderId, ToolSnapshotId, ToolTurnContract, + InvocationToolSnapshot, ToolArguments, ToolBinding, ToolChoice, ToolDescriptor, ToolId, + ToolInvocation, ToolProviderId, ToolSnapshotId, ToolTurnContract, }; use super::{ToolRequestGate, ToolRequestGateError}; @@ -145,7 +145,7 @@ mod tests { ToolInvocation { call_id: call_id.to_string(), tool_id, - arguments: json!({}), + arguments: ToolArguments::empty(), provider_metadata: Value::Null, } } diff --git a/src-tauri/crates/tt-domain/src/models/tool.rs b/src-tauri/crates/tt-domain/src/models/tool.rs index 3c0d07bf..707181d5 100644 --- a/src-tauri/crates/tt-domain/src/models/tool.rs +++ b/src-tauri/crates/tt-domain/src/models/tool.rs @@ -4,7 +4,7 @@ use std::{ }; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; -use serde_json::Value; +use serde_json::{Map, Value}; use crate::errors::DomainError; @@ -485,12 +485,93 @@ pub enum ToolChoice { Specific(ToolId), } +const INVALID_ARGUMENTS_QUOTE_CHARS: usize = 200; + +/// Invalid arguments retain their raw text in storage but replay as `{}`. +/// The paired tool error carries the rejection back to the model. +#[derive(Debug, Clone, PartialEq)] +pub enum ToolArguments { + Object(Map), + Invalid(String), +} + +impl ToolArguments { + pub fn empty() -> Self { + Self::Object(Map::new()) + } + + pub fn decode(raw: Option<&Value>) -> Self { + match raw { + None | Some(Value::Null) => Self::empty(), + Some(Value::String(text)) if text.trim().is_empty() => Self::empty(), + Some(Value::String(text)) => match serde_json::from_str::(text) { + Ok(Value::Object(map)) => Self::Object(map), + Ok(Value::Null) => Self::empty(), + _ => Self::Invalid(text.clone()), + }, + Some(Value::Object(map)) => Self::Object(map.clone()), + Some(value) => Self::Invalid(value.to_string()), + } + } + + pub fn as_map(&self) -> Result<&Map, String> { + match self { + Self::Object(map) => Ok(map), + Self::Invalid(raw) => Err(format!( + "arguments must be a JSON object; send `{{}}` when the call takes no arguments. Received: `{}`", + quote_invalid_arguments(raw) + )), + } + } + + pub fn encode_for_replay(&self) -> String { + match self { + Self::Object(map) => { + serde_json::to_string(map).expect("JSON argument objects are serializable") + } + Self::Invalid(_) => "{}".to_string(), + } + } + + pub fn to_replay_object(&self) -> Value { + match self { + Self::Object(map) => Value::Object(map.clone()), + Self::Invalid(_) => Value::Object(Map::new()), + } + } +} + +impl Serialize for ToolArguments { + fn serialize(&self, serializer: S) -> Result { + match self { + Self::Object(map) => map.serialize(serializer), + Self::Invalid(raw) => serializer.serialize_str(raw), + } + } +} + +impl<'de> Deserialize<'de> for ToolArguments { + fn deserialize>(deserializer: D) -> Result { + Ok(match Value::deserialize(deserializer)? { + Value::Object(map) => Self::Object(map), + value => Self::decode(Some(&value)), + }) + } +} + +fn quote_invalid_arguments(raw: &str) -> String { + match raw.char_indices().nth(INVALID_ARGUMENTS_QUOTE_CHARS) { + Some((index, _)) => format!("{}…", &raw[..index]), + None => raw.to_string(), + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct ToolInvocation { pub call_id: String, pub tool_id: ToolId, - pub arguments: Value, + pub arguments: ToolArguments, #[serde(default)] pub provider_metadata: Value, } @@ -586,8 +667,9 @@ mod tests { use serde_json::json; use super::{ - InvocationToolSnapshot, ToolBinding, ToolCatalog, ToolChoice, ToolDescriptionOverride, - ToolDescriptor, ToolId, ToolProviderId, ToolSnapshotId, ToolTurnContract, + InvocationToolSnapshot, ToolArguments, ToolBinding, ToolCatalog, ToolChoice, + ToolDescriptionOverride, ToolDescriptor, ToolId, ToolProviderId, ToolSnapshotId, + ToolTurnContract, }; use crate::errors::DomainError; @@ -779,6 +861,40 @@ mod tests { assert!(ToolTurnContract::all(&empty, ToolChoice::Required).is_err()); } + #[test] + fn both_wire_encodings_of_a_value_decode_to_the_same_arguments() { + let object = ToolArguments::decode(Some(&json!({ "depth": 2 }))); + for (raw, expected) in [ + (None, ToolArguments::empty()), + (Some(json!(null)), ToolArguments::empty()), + (Some(json!("null")), ToolArguments::empty()), + (Some(json!(" ")), ToolArguments::empty()), + (Some(json!("{}")), ToolArguments::empty()), + (Some(json!(r#"{"depth":2}"#)), object.clone()), + (Some(json!([1, 2])), ToolArguments::Invalid("[1,2]".into())), + (Some(json!("[1,2]")), ToolArguments::Invalid("[1,2]".into())), + ( + Some(json!(r#"{"path":"#)), + ToolArguments::Invalid(r#"{"path":"#.into()), + ), + ] { + assert_eq!(ToolArguments::decode(raw.as_ref()), expected, "{raw:?}"); + } + } + + #[test] + fn invalid_arguments_are_stored_verbatim_but_replay_as_an_empty_object() { + let invalid = ToolArguments::decode(Some(&json!("x".repeat(500)))); + + assert_eq!( + serde_json::to_value(&invalid).unwrap(), + json!("x".repeat(500)) + ); + assert_eq!(invalid.encode_for_replay(), "{}"); + let message = invalid.as_map().unwrap_err(); + assert!(message.contains('…') && message.len() < 400, "{message}"); + } + #[test] fn tool_catalog_rejects_duplicate_ids() { let id = ToolId::builtin("workspace.finish").unwrap();