-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Remove dependency on goose-mcp from goose crate #6637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 25 commits
8cb23a4
ff112d5
e0ac05d
dcda387
8de46fb
b7695c7
b0d88db
d65721e
67632ff
a68b89a
005a565
08ad92f
de0b113
ccee0ed
0f1dfeb
097a6e0
ca9b2db
10278d3
76a115f
b4ad24c
0047f29
95dea77
c44dafb
db3e5e5
d6e4069
a4e5f16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ use super::types::SharedProvider; | |
| use crate::agents::extension::{Envs, ProcessExit}; | ||
| use crate::agents::extension_malware_check; | ||
| use crate::agents::mcp_client::{McpClient, McpClientTrait}; | ||
| use crate::builtin_extension::get_builtin_extension; | ||
| use crate::config::extensions::name_to_key; | ||
| use crate::config::search_path::SearchPaths; | ||
| use crate::config::{get_all_extensions, Config}; | ||
|
|
@@ -564,14 +565,9 @@ impl ExtensionManager { | |
| } | ||
| ExtensionConfig::Builtin { name, timeout, .. } => { | ||
| let timeout_duration = Duration::from_secs(timeout.unwrap_or(300)); | ||
| let normalized_name = name_to_key(name); | ||
|
|
||
| if !goose_mcp::BUILTIN_EXTENSIONS.contains_key(normalized_name.as_str()) { | ||
| return Err(ExtensionError::ConfigError(format!( | ||
| "Unknown builtin extension: {}", | ||
| name | ||
| ))); | ||
| } | ||
| let extension_fn = get_builtin_extension(name.as_str()).ok_or_else(|| { | ||
| ExtensionError::ConfigError(format!("Unknown builtin extension: {}", name)) | ||
| })?; | ||
|
||
|
|
||
| if let Some(container) = container { | ||
| let container_id = container.id(); | ||
|
|
@@ -580,6 +576,7 @@ impl ExtensionManager { | |
| 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") | ||
|
|
@@ -600,10 +597,6 @@ impl ExtensionManager { | |
| .await?; | ||
| Box::new(client) | ||
| } else { | ||
| let def = goose_mcp::BUILTIN_EXTENSIONS | ||
| .get(normalized_name.as_str()) | ||
| .unwrap(); | ||
|
|
||
| // Set GOOSE_WORKING_DIR in the current process for builtin extensions | ||
| // since they run in-process and read from std::env::var | ||
| if effective_working_dir.exists() && effective_working_dir.is_dir() { | ||
|
|
@@ -616,7 +609,7 @@ impl ExtensionManager { | |
|
|
||
| let (server_read, client_write) = tokio::io::duplex(65536); | ||
| let (client_read, server_write) = tokio::io::duplex(65536); | ||
| (def.spawn_server)(server_read, server_write); | ||
| extension_fn(server_read, server_write); | ||
| Box::new( | ||
| McpClient::connect( | ||
| (client_read, client_write), | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||||||||||||||||
| use once_cell::sync::Lazy; | ||||||||||||||||||||||||||
| use std::collections::HashMap; | ||||||||||||||||||||||||||
| use std::sync::RwLock; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| pub type SpawnServerFn = fn(tokio::io::DuplexStream, tokio::io::DuplexStream); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| static BUILTIN_REGISTRY: Lazy<RwLock<HashMap<&'static str, SpawnServerFn>>> = | ||||||||||||||||||||||||||
| Lazy::new(|| RwLock::new(HashMap::new())); | ||||||||||||||||||||||||||
|
Comment on lines
+7
to
+8
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// Register a builtin extension into the global registry | ||||||||||||||||||||||||||
| pub fn register_builtin_extension(name: &'static str, spawn_fn: SpawnServerFn) { | ||||||||||||||||||||||||||
| BUILTIN_REGISTRY.write().unwrap().insert(name, spawn_fn); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// Register multiple builtin extensions from a HashMap | ||||||||||||||||||||||||||
| pub fn register_builtin_extensions(extensions: HashMap<&'static str, SpawnServerFn>) { | ||||||||||||||||||||||||||
| let mut registry = BUILTIN_REGISTRY.write().unwrap(); | ||||||||||||||||||||||||||
| registry.extend(extensions); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// Get a copy of all registered builtin extensions | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| /// Get a copy of all registered builtin extensions | |
| /// Get a registered builtin extension by name |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_builtin_extensions() clones the entire registry, which forces an allocation even when callers only need a lookup; consider adding has_builtin_extension(name: &str) / get_builtin_extension(name: &str) helpers that do the lookup under the read lock to avoid cloning the full HashMap.
| } | |
| } | |
| /// Check if a builtin extension with the given name is registered | |
| pub fn has_builtin_extension(name: &str) -> bool { | |
| BUILTIN_REGISTRY.read().unwrap().contains_key(name) | |
| } | |
| /// Get the builtin extension spawn function for the given name, if registered | |
| pub fn get_builtin_extension(name: &str) -> Option<SpawnServerFn> { | |
| BUILTIN_REGISTRY.read().unwrap().get(name).copied() | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExtensionConfig::Builtinlookup now uses the rawname, but builtin keys are normalized elsewhere (and previously usedname_to_key), so configs like "Developer"/"developer" may regress; lookup should use the normalized key (or ensure registration uses the same normalization).