From ff3ad4349dd3732617ec1ee59330bbad5761bed3 Mon Sep 17 00:00:00 2001 From: Zane <75694352+zanesq@users.noreply.github.com> Date: Tue, 17 Feb 2026 09:32:12 -0800 Subject: [PATCH] working_dir usage more clear in add_extension (#6958) --- crates/goose/src/agents/extension_manager.rs | 139 ++++++++++-------- .../modal/ProviderConfiguationModal.tsx | 3 +- ui/desktop/src/sandbox/proxy.test.ts | 9 +- 3 files changed, 78 insertions(+), 73 deletions(-) diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 57f5a784a4..0c9d645248 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -535,10 +535,6 @@ impl ExtensionManager { return Ok(()); } - // Resolve working_dir: explicit > current_dir - let effective_working_dir = - working_dir.unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); - let mut temp_dir = None; let client: Box = match &config { @@ -577,6 +573,74 @@ impl ExtensionManager { ) .await? } + ExtensionConfig::Builtin { name, timeout, .. } => { + let timeout_duration = Duration::from_secs(timeout.unwrap_or(300)); + let normalized_name = name_to_key(name); + let extension_fn = + get_builtin_extension(normalized_name.as_str()).ok_or_else(|| { + ExtensionError::ConfigError(format!("Unknown builtin extension: {}", name)) + })?; + + if let Some(container) = container { + let container_id = container.id(); + tracing::info!( + container = %container_id, + builtin = %name, + "Starting builtin extension inside Docker container" + ); + let normalized_name = name_to_key(name); + let command = Command::new("docker").configure(|command| { + command + .arg("exec") + .arg("-i") + .arg(container_id) + .arg("goose") + .arg("mcp") + .arg(&normalized_name); + }); + + let effective_working_dir = working_dir + .clone() + .unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); + + let capabilities = GooseMcpClientCapabilities { + mcpui: self.capabilities.mcpui, + }; + + let client = child_process_client( + command, + timeout, + self.provider.clone(), + Some(&effective_working_dir), + Some(container_id.to_string()), + self.client_name.clone(), + capabilities, + ) + .await?; + Box::new(client) + } else { + // Non-containerized builtin runs in-process via duplex channels. + // Working directory is passed per-request via call_tool metadata, not here. + let (server_read, client_write) = tokio::io::duplex(65536); + let (client_read, server_write) = tokio::io::duplex(65536); + extension_fn(server_read, server_write); + + let capabilities = GooseMcpClientCapabilities { + mcpui: self.capabilities.mcpui, + }; + + Box::new( + McpClient::connect( + (client_read, client_write), + timeout_duration, + self.provider.clone(), + self.client_name.clone(), + capabilities, + ) + .await?, + ) + } + } ExtensionConfig::Stdio { cmd, args, @@ -619,6 +683,9 @@ impl ExtensionManager { }) }; + let effective_working_dir = working_dir + .clone() + .unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); let capabilities = GooseMcpClientCapabilities { mcpui: self.capabilities.mcpui, }; @@ -634,65 +701,6 @@ impl ExtensionManager { .await?; Box::new(client) } - ExtensionConfig::Builtin { name, timeout, .. } => { - let timeout_duration = Duration::from_secs(timeout.unwrap_or(300)); - let normalized_name = name_to_key(name); - let extension_fn = - get_builtin_extension(normalized_name.as_str()).ok_or_else(|| { - ExtensionError::ConfigError(format!("Unknown builtin extension: {}", name)) - })?; - - if let Some(container) = container { - let container_id = container.id(); - tracing::info!( - container = %container_id, - builtin = %name, - "Starting builtin extension inside Docker container" - ); - let normalized_name = name_to_key(name); - let command = Command::new("docker").configure(|command| { - command - .arg("exec") - .arg("-i") - .arg(container_id) - .arg("goose") - .arg("mcp") - .arg(&normalized_name); - }); - - let capabilities = GooseMcpClientCapabilities { - mcpui: self.capabilities.mcpui, - }; - let client = child_process_client( - command, - timeout, - self.provider.clone(), - Some(&effective_working_dir), - Some(container_id.to_string()), - self.client_name.clone(), - capabilities, - ) - .await?; - Box::new(client) - } else { - let (server_read, client_write) = tokio::io::duplex(65536); - let (client_read, server_write) = tokio::io::duplex(65536); - extension_fn(server_read, server_write); - let capabilities = GooseMcpClientCapabilities { - mcpui: self.capabilities.mcpui, - }; - Box::new( - McpClient::connect( - (client_read, client_write), - timeout_duration, - self.provider.clone(), - self.client_name.clone(), - capabilities, - ) - .await?, - ) - } - } ExtensionConfig::Platform { name, .. } => { let normalized_key = name_to_key(name); let def = PLATFORM_EXTENSIONS @@ -724,6 +732,11 @@ impl ExtensionManager { command.arg("python").arg(file_path.to_str().unwrap()); }); + // Compute working_dir for InlinePython (runs as child process via uvx) + let effective_working_dir = working_dir + .clone() + .unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); + let capabilities = GooseMcpClientCapabilities { mcpui: self.capabilities.mcpui, }; diff --git a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx index 9a48b396e9..a2f1fc6892 100644 --- a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx +++ b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx @@ -107,8 +107,7 @@ export default function ProviderConfigurationModal({ Object.entries(configValues) .filter( ([_k, entry]) => - !!entry.value || - (entry.serverValue != null && typeof entry.serverValue === 'string') + !!entry.value || (entry.serverValue != null && typeof entry.serverValue === 'string') ) .map(([k, entry]) => [ k, diff --git a/ui/desktop/src/sandbox/proxy.test.ts b/ui/desktop/src/sandbox/proxy.test.ts index 12879e30c0..fcf9116093 100644 --- a/ui/desktop/src/sandbox/proxy.test.ts +++ b/ui/desktop/src/sandbox/proxy.test.ts @@ -190,14 +190,7 @@ describe('checkBlocked', () => { }); it('does not block loopback by default, blocks when opted in', async () => { - const allowed = await checkBlocked( - 'localhost', - 8080, - blocked, - noLD, - noLDCache, - defaultOptions - ); + const allowed = await checkBlocked('localhost', 8080, blocked, noLD, noLDCache, defaultOptions); expect(allowed.blocked).toBe(false); const blocked_ = await checkBlocked('localhost', 8080, blocked, noLD, noLDCache, {