diff --git a/crates/tools/src/exec.rs b/crates/tools/src/exec.rs index b7903daf6..43d7ad63b 100644 --- a/crates/tools/src/exec.rs +++ b/crates/tools/src/exec.rs @@ -396,7 +396,7 @@ impl AgentTool for ExecTool { obj.insert( "node".to_string(), serde_json::json!({ - "type": "string", + "type": ["string", "null"], "description": "Node name or ID to run on. Omit to use the session's default node." }), ); @@ -436,18 +436,21 @@ impl AgentTool for ExecTool { .and_then(|v| v.as_str()) .filter(|s| !s.trim().is_empty()) .map(String::from); + let clear_default_node = params.get("node").is_some_and(serde_json::Value::is_null); // Determine the effective node reference, distinguishing model-supplied // values from the admin-configured default. When no nodes are connected: // - Model-hallucinated values are silently dropped (fall through to local). // - A configured `default_node` produces a clear error so the admin knows // the intended remote host is unavailable. let node_ref = if let Some(provider) = &self.node_provider { - if provider.has_connected_nodes() { + if clear_default_node { + None + } else if provider.has_connected_nodes() { match model_node.or_else(|| self.default_node.clone()) { Some(node_ref) => Some(node_ref), None => provider.default_node_ref().await, } - } else if let Some(ref dn) = self.default_node { + } else if !clear_default_node && let Some(ref dn) = self.default_node { return Err(Error::message(format!( "default node '{dn}' is configured but no nodes are currently connected" )) diff --git a/crates/tools/src/exec/tests.rs b/crates/tools/src/exec/tests.rs index bc75fc13a..68cf0e325 100644 --- a/crates/tools/src/exec/tests.rs +++ b/crates/tools/src/exec/tests.rs @@ -1049,6 +1049,59 @@ impl NodeExecProvider for DisconnectedNodeProvider { } } +/// Connected provider used to verify explicit node clearing does not route remotely. +struct ConnectedNodeProvider; + +#[async_trait] +impl NodeExecProvider for ConnectedNodeProvider { + async fn exec_on_node( + &self, + _node_id: &str, + _command: &str, + _timeout_secs: u64, + _cwd: Option<&str>, + _env: Option<&HashMap>, + ) -> anyhow::Result { + unreachable!("explicit null node must use local execution"); + } + + async fn resolve_node_id(&self, _node_ref: &str) -> Option { + Some("connected-node".into()) + } + + fn has_connected_nodes(&self) -> bool { + true + } + + async fn default_node_ref(&self) -> Option { + Some("connected-node".into()) + } +} + +#[tokio::test] +async fn test_exec_null_node_clears_configured_default() { + let temp_dir = tempfile::tempdir().unwrap(); + let tool = ExecTool { + working_dir: Some(temp_dir.path().to_path_buf()), + ..Default::default() + } + .with_node_provider( + Arc::new(ConnectedNodeProvider), + Some("configured-node".into()), + ); + + let result = tool + .execute(serde_json::json!({ + "command": "echo local", + "node": null + })) + .await + .unwrap(); + + assert_eq!(result["stdout"].as_str().unwrap().trim(), "local"); + assert_eq!(result["exit_code"], 0); +} + #[tokio::test] async fn test_exec_ignores_node_param_when_no_nodes_connected() { let temp_dir = tempfile::tempdir().unwrap(); @@ -1123,6 +1176,14 @@ async fn test_exec_schema_hides_node_when_no_nodes_connected() { ); } +#[tokio::test] +async fn test_exec_schema_allows_explicit_null_node_when_connected() { + let tool = ExecTool::default().with_node_provider(Arc::new(ConnectedNodeProvider), None); + + let schema = tool.parameters_schema(); + assert_eq!(schema["properties"]["node"]["type"], serde_json::json!(["string", "null"])); +} + #[tokio::test] async fn test_exec_errors_when_default_node_configured_but_disconnected() { let temp_dir = tempfile::tempdir().unwrap();