perf(tools): remove unconditional params clone in shared execution (fix #893) (#926)

* perf(tools): remove unconditional params clone in shared execution

* Update src/tools/execute.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* chore(fmt): apply rustfmt in worker container tool execution

* fix(tools): restore owned param call sites

* fix(tools): pass normalized_params to tool.execute() instead of raw params

The ownership refactor accidentally passed the un-coerced `params` to
`tool.execute()` while validation ran against the coerced
`normalized_params`. This meant tools received un-normalized input
(e.g. stringified JSON arrays instead of actual arrays). Since
`normalized_params` is owned and unused after the execute call, passing
it directly achieves the original zero-clone goal without breaking
parameter coercion.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(tools): update empty-tool-name test for owned params signature

Adapts the test_execute_empty_tool_name_returns_not_found test (added
on staging) to pass owned Value instead of &Value, matching the new
execute_tool_with_safety signature.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
Nige
2026-03-22 17:48:02 -07:00
committed by GitHub
co-authored by gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> [email protected] <[email protected]> Claude Opus 4.6
parent 969b559e2a
commit 3e73dbe615
4 changed files with 27 additions and 22 deletions
+8 -1
View File
@@ -915,7 +915,14 @@ pub(super) async fn execute_chat_tool_standalone(
params: &serde_json::Value,
job_ctx: &crate::context::JobContext,
) -> Result<String, Error> {
crate::tools::execute::execute_tool_with_safety(tools, safety, tool_name, params, job_ctx).await
crate::tools::execute::execute_tool_with_safety(
tools,
safety,
tool_name,
params.clone(),
job_ctx,
)
.await
}
/// Parsed auth result fields for emitting StatusUpdate::AuthRequired.
+1 -5
View File
@@ -549,11 +549,7 @@ impl Scheduler {
// Delegate to shared tool execution pipeline
let output_str = crate::tools::execute::execute_tool_with_safety(
&tools,
&safety,
tool_name,
&normalized_params,
&job_ctx,
&tools, &safety, tool_name, params, &job_ctx,
)
.await?;
+10 -13
View File
@@ -19,7 +19,7 @@ pub async fn execute_tool_with_safety(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
params: &serde_json::Value,
params: serde_json::Value,
job_ctx: &JobContext,
) -> Result<String, Error> {
if tool_name.is_empty() {
@@ -35,7 +35,7 @@ pub async fn execute_tool_with_safety(
name: tool_name.to_string(),
})?;
let normalized_params = prepare_tool_params(tool.as_ref(), params);
let normalized_params = prepare_tool_params(tool.as_ref(), &params);
// Validate tool parameters
let validation = safety.validator().validate_tool_params(&normalized_params);
@@ -63,10 +63,7 @@ pub async fn execute_tool_with_safety(
// Execute with per-tool timeout
let timeout = tool.execution_timeout();
let start = std::time::Instant::now();
let result = tokio::time::timeout(timeout, async {
tool.execute(normalized_params.clone(), job_ctx).await
})
.await;
let result = tokio::time::timeout(timeout, tool.execute(normalized_params, job_ctx)).await;
let elapsed = start.elapsed();
match &result {
@@ -149,7 +146,7 @@ pub async fn execute_tool_simple(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
params: &serde_json::Value,
params: serde_json::Value,
job_ctx: &JobContext,
) -> Result<String, String> {
execute_tool_with_safety(tools, safety, tool_name, params, job_ctx)
@@ -308,7 +305,7 @@ mod tests {
&registry,
&safety,
"",
&serde_json::json!({}),
serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -331,7 +328,7 @@ mod tests {
let params = serde_json::json!({"message": "hello"});
let result =
execute_tool_with_safety(&registry, &safety, "echo", &params, &test_job_ctx()).await;
execute_tool_with_safety(&registry, &safety, "echo", params, &test_job_ctx()).await;
assert!(result.is_ok(), "Echo tool should succeed");
let output = result.unwrap();
@@ -350,7 +347,7 @@ mod tests {
&registry,
&safety,
"nonexistent",
&serde_json::json!({}),
serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -373,7 +370,7 @@ mod tests {
&registry,
&safety,
"fail_tool",
&serde_json::json!({}),
serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -397,7 +394,7 @@ mod tests {
&registry,
&safety,
"slow_tool",
&serde_json::json!({}),
serde_json::json!({}),
&test_job_ctx(),
)
.await;
@@ -425,7 +422,7 @@ mod tests {
&registry,
&safety,
"array_echo",
&serde_json::json!({"values": "[\"1\", \"2\", 3]"}),
serde_json::json!({"values": "[\"1\", \"2\", 3]"}),
&test_job_ctx(),
)
.await
+8 -3
View File
@@ -462,9 +462,14 @@ impl LoopDelegate for ContainerDelegate {
..Default::default()
};
let result =
execute_tool_simple(&self.tools, &self.safety, &tc.name, &tc.arguments, &job_ctx)
.await;
let result = execute_tool_simple(
&self.tools,
&self.safety,
&tc.name,
tc.arguments.clone(),
&job_ctx,
)
.await;
self.post_event(
"tool_result",