tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 32 Packages Projects Releases Wiki Activity

feat(cli): let --with-extension name its extension (#11127)

Browse Source 
Signed-off-by: kirp@umich.edu <kirp@umich.edu>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
tic-top
2026-08-19 00:17:58 +00:00
committed by GitHub
parent 3fac8f23cf
commit 9f941fbfc5
4 changed files with 361 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose-cli/src/cli.rs
+1 -1
View File
@@ -176,7 +176,7 @@ pub struct ExtensionOptions {
long = "with-extension",
value_name = "COMMAND",
help = "Add stdio extensions (can be specified multiple times)",
long_help = "Add stdio extensions from full commands with environment variables. Can be specified multiple times. Format: 'ENV1=val1 ENV2=val2 command args...'",
long_help = "Add stdio extensions from full commands with environment variables. Can be specified multiple times. Format: '[name:]ENV1=val1 ENV2=val2 command args...'. Without the optional name, the extension is named after the command, which is the launcher for anything started through one ('npx', 'python', 'uvx', ...); extensions that would end up sharing a name are instead named after their full command line.",
action = clap::ArgAction::Append
)]
pub extensions: Vec<String>,
crates/goose-cli/src/session/builder.rs
+226 -12
View File
@@ -1,9 +1,10 @@
use crate::cli::StreamableHttpOptions;
use super::output;
use super::CliSession;
use super::{derive_extension_name_from_command, split_extension_name_prefix, CliSession};
use console::style;
use goose::agents::{Agent, Container, ExtensionError};
use goose::config::extensions::name_to_key;
use goose::config::resolve_extensions_for_new_session;
use goose::config::{Config, ExtensionConfig, GooseMode};
use goose::model_config::model_config_from_user_config;
@@ -12,7 +13,7 @@ use goose::recipe::Recipe;
use goose::session::session_manager::SessionType;
use goose::session::EnabledExtensionsState;
use rustyline::EditMode;
use std::collections::BTreeSet;
use std::collections::{BTreeSet, HashMap, HashSet};
use std::process;
use std::sync::Arc;
use tokio::task::JoinSet;
@@ -28,11 +29,68 @@ fn truncate_with_ellipsis(s: &str, max_len: usize) -> String {
}
}
// Plain String rather than `ExtensionError`: the only variant this function
// ever constructs is `ConfigError(String)`, but clippy's `result_large_err`
// sizes an error type by its largest variant, and `ExtensionError` carries a
// `ClientError`/`ClientInitializeError` far past the 128-byte default
// threshold. Callers wrap this back into `ExtensionError::ConfigError`.
fn disambiguate_stdio_extension_names(
extensions: &mut [(String, ExtensionConfig)],
renameable: &HashSet<usize>,
) -> Result<(), String> {
let mut counts: HashMap<String, usize> = HashMap::new();
// Only entries the caller marked fixed (explicitly named, or not a CLI
// stdio extension at all) can make this an error — a fixed name colliding
// with a renameable one is exactly the case the loop below resolves by
// renaming the renameable side, not a real conflict.
let mut fixed_counts: HashMap<String, usize> = HashMap::new();
for (idx, (_, config)) in extensions.iter().enumerate() {
let key = config.key();
*counts.entry(key.clone()).or_default() += 1;
if !renameable.contains(&idx) {
*fixed_counts.entry(key).or_default() += 1;
}
}
let duplicate_fixed_names = extensions.iter().enumerate().find(|(index, (_, config))| {
!renameable.contains(index) && fixed_counts.get(&config.key()).copied().unwrap_or(0) > 1
});
if let Some((_, (_, config))) = duplicate_fixed_names {
return Err(format!(
"extension name '{}' is already in use",
config.name()
));
}
let mut taken: HashSet<String> = counts.keys().cloned().collect();
for (idx, (_, config)) in extensions.iter_mut().enumerate() {
if !renameable.contains(&idx) || counts.get(&config.key()).copied().unwrap_or(0) < 2 {
continue;
}
let ExtensionConfig::Stdio {
name, cmd, args, ..
} = config
else {
continue;
};
let derived = derive_extension_name_from_command(cmd, args);
let mut candidate = derived.clone();
let mut suffix = 2;
while candidate.is_empty() || taken.contains(&name_to_key(&candidate)) {
candidate = format!("{}_{}", derived, suffix);
suffix += 1;
}
taken.insert(name_to_key(&candidate));
*name = candidate;
}
Ok(())
}
fn parse_cli_flag_extensions(
extensions: &[String],
streamable_http_extensions: &[StreamableHttpOptions],
builtins: &[String],
) -> Vec<(String, ExtensionConfig)> {
) -> Vec<(String, ExtensionConfig, bool)> {
let mut extensions_to_load = Vec::new();
for (idx, ext_str) in extensions.iter().enumerate() {
@@ -40,7 +98,8 @@ fn parse_cli_flag_extensions(
Ok(config) => {
let hint = truncate_with_ellipsis(ext_str, EXTENSION_HINT_MAX_LEN);
let label = format!("stdio #{}({})", idx + 1, hint);
extensions_to_load.push((label, config));
let explicitly_named = split_extension_name_prefix(ext_str).0.is_some();
extensions_to_load.push((label, config, !explicitly_named));
}
Err(e) => {
eprintln!(
@@ -59,13 +118,13 @@ fn parse_cli_flag_extensions(
let config = CliSession::parse_streamable_http_extension(&opts.url, opts.timeout);
let hint = truncate_with_ellipsis(&opts.url, EXTENSION_HINT_MAX_LEN);
let label = format!("http #{}({})", idx + 1, hint);
extensions_to_load.push((label, config));
extensions_to_load.push((label, config, false));
}
for builtin_str in builtins {
let configs = CliSession::parse_builtin_extensions(builtin_str);
for config in configs {
extensions_to_load.push((config.name(), config));
extensions_to_load.push((config.name(), config, false));
}
}
@@ -549,16 +608,34 @@ async fn collect_extension_configs(
&session_config.builtins,
);
let mut all: Vec<ExtensionConfig> = configured_extensions;
let mut all: Vec<(String, ExtensionConfig)> = configured_extensions
.into_iter()
.map(|config| (config.name(), config))
.collect();
if !session_config.no_profile && !session_config.resume && recipe_extensions.is_none() {
let project_root = std::env::current_dir().ok();
all.extend(goose::plugins::mcp_servers::enabled_plugin_mcp_servers(
project_root.as_deref(),
));
all.extend(
goose::plugins::mcp_servers::enabled_plugin_mcp_servers(project_root.as_deref())
.into_iter()
.map(|config| (config.name(), config)),
);
}
all.extend(cli_flag_extensions.into_iter().map(|(_, cfg)| cfg));
Ok(all)
let cli_start = all.len();
let renameable = cli_flag_extensions
.iter()
.enumerate()
.filter_map(|(index, (_, _, renameable))| renameable.then_some(cli_start + index))
.collect();
all.extend(
cli_flag_extensions
.into_iter()
.map(|(label, config, _)| (label, config)),
);
disambiguate_stdio_extension_names(&mut all, &renameable)
.map_err(ExtensionError::ConfigError)?;
Ok(all.into_iter().map(|(_, config)| config).collect())
}
async fn resolve_and_load_extensions(
@@ -832,6 +909,143 @@ mod tests {
use goose::session::SessionManager;
use tempfile::TempDir;
fn stdio_names(extensions: &[&str]) -> Vec<String> {
let parsed = parse_cli_flag_extensions(
&extensions.iter().map(|s| s.to_string()).collect::<Vec<_>>(),
&[],
&[],
);
let renameable = parsed
.iter()
.enumerate()
.filter_map(|(index, (_, _, renameable))| renameable.then_some(index))
.collect();
let mut configs: Vec<_> = parsed
.into_iter()
.map(|(label, config, _)| (label, config))
.collect();
disambiguate_stdio_extension_names(&mut configs, &renameable).unwrap();
configs
.into_iter()
.map(|(_, config)| config.name())
.collect()
}
#[test]
fn test_colliding_launcher_names_fall_back_to_the_command_line() {
assert_eq!(
stdio_names(&[
"npx -y @modelcontextprotocol/server-memory",
"npx -y @modelcontextprotocol/server-filesystem",
]),
vec!["server-memory".to_string(), "server-filesystem".to_string()]
);
}
#[test]
fn test_explicit_names_survive_a_collision() {
assert_eq!(
stdio_names(&["python -m word_mcp", "memory:python -m memory_mcp"]),
vec!["python".to_string(), "memory".to_string()]
);
assert_eq!(
stdio_names(&[
"word:python -m word_mcp",
"python -m a_mcp",
"python -m b_mcp",
]),
vec![
"word".to_string(),
"python_m_a_mcp".to_string(),
"python_m_b_mcp".to_string(),
]
);
}
#[test]
fn test_normalized_names_are_disambiguated() {
assert_eq!(
stdio_names(&["MyTool --server a", "mytool --server b"]),
vec!["mytool_server_a".to_string(), "mytool_server_b".to_string()]
);
}
#[test]
fn test_cli_name_is_disambiguated_against_configured_extension() {
let mut extensions = vec![
(
"configured".to_string(),
CliSession::parse_stdio_extension("npx configured-server").unwrap(),
),
(
"cli".to_string(),
CliSession::parse_stdio_extension("npx cli-server").unwrap(),
),
];
disambiguate_stdio_extension_names(&mut extensions, &HashSet::from([1])).unwrap();
assert_eq!(extensions[0].1.name(), "npx");
assert_eq!(extensions[1].1.name(), "npx_cli-server");
}
#[test]
fn test_fixed_counted_per_key_not_globally() {
// `duplicate_fixed_names` must count *fixed entries sharing a key*,
// not "is there more than one fixed entry at all" -- otherwise a
// fixed entry with a unique name, sitting alongside a fixed+renameable
// collision on a different key, would wrongly trip the same error.
let mut extensions = vec![
(
"configured a".to_string(),
// Fixed, name "word" -- unique, no collision with anything.
CliSession::parse_stdio_extension("word:npx server-a").unwrap(),
),
(
"configured b".to_string(),
// Fixed, name "npx" -- collides with the renameable entry below.
CliSession::parse_stdio_extension("npx server-b").unwrap(),
),
(
"cli".to_string(),
// Renameable, also defaults to "npx".
CliSession::parse_stdio_extension("npx cli-server").unwrap(),
),
];
disambiguate_stdio_extension_names(&mut extensions, &HashSet::from([2])).unwrap();
assert_eq!(extensions[0].1.name(), "word");
assert_eq!(extensions[1].1.name(), "npx");
assert_eq!(extensions[2].1.name(), "npx_cli-server");
}
#[test]
fn test_duplicate_explicit_names_are_rejected() {
let mut extensions = vec![
(
"first".to_string(),
CliSession::parse_stdio_extension("memory:npx first").unwrap(),
),
(
"second".to_string(),
CliSession::parse_stdio_extension("Memory:npx second").unwrap(),
),
];
let error =
disambiguate_stdio_extension_names(&mut extensions, &HashSet::new()).unwrap_err();
assert!(error
.to_string()
.contains("extension name 'memory' is already in use"));
}
#[test]
fn test_identical_commands_still_get_distinct_names() {
assert_eq!(
stdio_names(&["python -m word_mcp", "python -m word_mcp"]),
vec![
"python_m_word_mcp".to_string(),
"python_m_word_mcp_2".to_string()
]
);
}
fn test_config(temp_dir: &TempDir) -> Config {
Config::new_with_file_secrets(
temp_dir.path().join("config.yaml"),
crates/goose-cli/src/session/mod.rs
+130 -7
View File
@@ -70,6 +70,59 @@ const SHELL_STATUS_FALLBACK_WIDTH: usize = 120;
const SHELL_STATUS_MAX_LINES: usize = 3;
const SHELL_STATUS_RESERVED_WIDTH: usize = 2;
/// Upper bound on a *derived* extension name. Tools are exposed to the model as
/// `{extension}__{tool}`, and several providers cap tool name length, so a name
/// built out of a whole command line has to be clipped.
const DERIVED_EXTENSION_NAME_MAX_LEN: usize = 32;
pub(crate) fn split_extension_name_prefix(extension_command: &str) -> (Option<String>, &str) {
let Some((candidate, rest)) = extension_command.split_once(':') else {
return (None, extension_command);
};
let looks_like_name = candidate.chars().count() >= 2
&& candidate
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-');
if !looks_like_name || rest.trim().is_empty() || rest.starts_with("//") {
return (None, extension_command);
}
(Some(name_to_key(candidate)), rest)
}
pub(crate) fn derive_extension_name_from_command(cmd: &str, args: &[String]) -> String {
let basename = std::path::Path::new(cmd)
.file_name()
.and_then(|f| f.to_str())
.unwrap_or(cmd);
let joined = std::iter::once(basename)
.chain(args.iter().map(String::as_str))
.map(|token| token.trim_start_matches('-'))
.collect::<Vec<_>>()
.join("_");
let mut key = String::with_capacity(joined.len());
for c in name_to_key(&joined).chars() {
if c == '_' && key.ends_with('_') {
continue;
}
key.push(c);
}
let key = key.trim_matches('_');
if key.chars().count() <= DERIVED_EXTENSION_NAME_MAX_LEN {
return key.to_string();
}
let tail: String = key
.chars()
.skip(key.chars().count() - DERIVED_EXTENSION_NAME_MAX_LEN)
.collect();
// Drop the leading fragment of whatever token the clip landed inside.
match tail.split_once('_') {
Some((_, rest)) if !rest.is_empty() => rest.to_string(),
_ => tail,
}
}
fn planner_provider_messages(plan_messages: &Conversation) -> Conversation {
// The planner prompt has no turn-context instructions; drop the blocks.
let projected_messages: Vec<Message> = plan_messages
@@ -337,9 +390,14 @@ impl CliSession {
}
/// Parse a stdio extension command string into an ExtensionConfig
/// Format: "ENV1=val1 ENV2=val2 command args..."
/// Format: "[name:]ENV1=val1 ENV2=val2 command args..."
///
/// Without the optional `name:` prefix the extension is named after the
/// basename of the command, which is the launcher rather than the server
/// whenever one is used (`npx`, `python -m ...`, `uvx`, ...).
pub fn parse_stdio_extension(extension_command: &str) -> Result<ExtensionConfig> {
let mut parts = goose::utils::split_command_args(extension_command)?;
let (explicit_name, command) = split_extension_name_prefix(extension_command);
let mut parts = goose::utils::split_command_args(command)?;
let mut envs = HashMap::new();
while let Some(part) = parts.first() {
@@ -356,11 +414,13 @@ impl CliSession {
}
let cmd = parts.remove(0);
let name = std::path::Path::new(&cmd)
.file_name()
.and_then(|f| f.to_str())
.unwrap_or("unnamed")
.to_string();
let name = explicit_name.unwrap_or_else(|| {
std::path::Path::new(&cmd)
.file_name()
.and_then(|f| f.to_str())
.unwrap_or("unnamed")
.to_string()
});
Ok(ExtensionConfig::Stdio {
name,
@@ -2963,6 +3023,69 @@ mod tests {
assert!(CliSession::parse_stdio_extension("").is_err());
}
fn stdio_name(input: &str) -> String {
CliSession::parse_stdio_extension(input).unwrap().name()
}
#[test]
fn test_parse_stdio_extension_explicit_name() {
assert_eq!(stdio_name("word:python -m word_mcp"), "word");
assert_eq!(stdio_name("Word-One:python -m word_mcp"), "word-one");
let absolute = CliSession::parse_stdio_extension("memory:/usr/local/bin/mcp").unwrap();
assert_eq!(absolute.name(), "memory");
assert!(matches!(
absolute,
ExtensionConfig::Stdio { cmd, .. } if cmd == "/usr/local/bin/mcp"
));
let config = CliSession::parse_stdio_extension("memory:API_KEY=k npx -y srv").unwrap();
let ExtensionConfig::Stdio {
name,
cmd,
args,
envs,
..
} = config
else {
panic!("expected a stdio extension");
};
assert_eq!(name, "memory");
assert_eq!(cmd, "npx");
assert_eq!(args, vec!["-y".to_string(), "srv".to_string()]);
assert_eq!(envs.get_env().get("API_KEY").map(String::as_str), Some("k"));
}
#[test_case("C:\\Program Files\\srv.exe --stdio" ; "windows_drive_letter")]
#[test_case("srv://not-a-name" ; "url_like_command")]
#[test_case("npx -y pkg:latest" ; "colon_after_a_space")]
#[test_case("word:" ; "empty_command")]
fn test_split_extension_name_prefix_leaves_command_alone(input: &str) {
let (name, rest) = split_extension_name_prefix(input);
assert_eq!(name, None);
assert_eq!(rest, input);
}
#[test]
fn test_derive_extension_name_from_command() {
assert_eq!(
derive_extension_name_from_command("python", &["-m".into(), "word_mcp".into()]),
"python_m_word_mcp"
);
assert_eq!(
derive_extension_name_from_command(
"npx",
&[
"-y".into(),
"@modelcontextprotocol/server-filesystem".into()
],
),
"server-filesystem"
);
assert_eq!(
derive_extension_name_from_command("/usr/local/bin/srv", &[]),
"srv"
);
}
#[test]
fn test_build_switched_model_config_rebuilds_target_model_settings() {
let _guard = env_lock::lock_env([
documentation/docs/guides/goose-cli-commands.md
+4 -1
View File
@@ -215,7 +215,7 @@ Start or resume interactive chat sessions.
- **`--max-turns <NUMBER>`**: Set the maximum number of turns allowed without user input (default: 1000)
**Extension Options:**
- **`--with-extension <command>`**: Add stdio extensions
- **`--with-extension <command>`**: Add stdio extensions. Format: `[name:]ENV1=val1 command args...`. Without the optional `name:`, the extension is named after the command — which is the launcher, not the server, for anything started through one (`npx`, `python -m ...`, `uvx`). That name prefixes every tool the server exposes (`npx__search`), and extensions started by the same launcher would otherwise collide, so goose names colliding ones after their full command line instead. Give an explicit name to control it: `--with-extension "memory:npx -y @modelcontextprotocol/server-memory"` exposes `memory__search`.
- **`--with-streamable-http-extension <url>`**: Add remote extensions over Streamable HTTP
- **`--with-builtin <id>`**: Enable built-in extensions (e.g., 'developer', 'computercontroller')
@@ -244,6 +244,9 @@ goose session --resume --session-id 20251108_2 --fork --edit --history
# Start with extensions
goose session --with-extension "npx -y @modelcontextprotocol/server-memory"
# Name an extension explicitly (tools become memory__*, not npx__*)
goose session --with-extension "memory:npx -y @modelcontextprotocol/server-memory"
goose session --with-builtin developer
goose session --with-streamable-http-extension "http://localhost:8080/mcp"