Repository navigation
feat(cli): /model with no argument opens a cross-provider model picker
#9658
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 1 commit
2ccfac9
3a28adf
6be91f4
3025873
f7eaa6a
5e857b3
ab9e2ae
08ac2ff
966cb54
13b18cf
fafb968
395a8ff
e5eef0e
544ad3b
02ddfe5
0c94765
3d817fc
a0d37b5
d975b3e
0bc763e
78ab68c
9c94804
6536ab7
d9d2493
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -806,11 +806,13 @@ impl CliSession { | |
| let current_model_name = current_model_config.model_name.clone(); | ||
|
|
||
| if model.is_none() { | ||
| output::goose_mode_message(&format!( | ||
| "Current session model: '{}' (provider '{}')", | ||
| current_model_name, current_provider_name | ||
| )); | ||
| return Ok(()); | ||
| return self | ||
| .pick_and_switch_model( | ||
| ¤t_provider_name, | ||
| ¤t_model_config, | ||
| ¤t_model_name, | ||
| ) | ||
| .await; | ||
| } | ||
|
|
||
| let model_name = model.unwrap_or_default().trim(); | ||
|
|
@@ -866,6 +868,146 @@ impl CliSession { | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Bare `/model` (no argument): present one searchable menu of models across | ||
| /// every configured provider (local LM Studio, NVIDIA, etc.) and switch the | ||
| /// live session to the chosen provider + model. | ||
| async fn pick_and_switch_model( | ||
| &self, | ||
| current_provider_name: &str, | ||
| current_model_config: &goose::model::ModelConfig, | ||
| current_model_name: &str, | ||
| ) -> Result<()> { | ||
| use std::time::Duration; | ||
|
|
||
| let _ = cliclack::log::info(format!( | ||
| "Current model: '{current_model_name}' (provider '{current_provider_name}'). Gathering models from your configured providers…" | ||
| )); | ||
|
|
||
| let all = goose::providers::providers().await; | ||
| let mut entries: Vec<(String, String, String)> = Vec::new(); | ||
| let mut seen: std::collections::HashSet<String> = std::collections::HashSet::new(); | ||
|
|
||
| for (meta, _ptype) in &all { | ||
| let include = | ||
| meta.name.as_str() == current_provider_name || provider_is_configured(meta); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In a normal install with no AWS credentials, this predicate still treats Amazon Bedrock as configured because its required Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 3a28adf. A built-in default is now treated as configuration, not a credential: a provider that authenticates with a secret (API key / bearer token) must have at least one real secret value to be offered in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user has already signed in to an OAuth-backed provider such as ChatGPT Codex or xAI OAuth and the current session is on a different provider, the usable credential lives in the provider's token cache rather than in Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Real gap, but it belongs in the per-provider follow-up (#9822) rather than this metadata predicate. The usable credential for OAuth-backed providers lives in their token cache, which only |
||
| if !include || !seen.insert(meta.name.clone()) { | ||
| continue; | ||
| } | ||
|
|
||
| let seed = if meta.default_model.trim().is_empty() { | ||
| "placeholder".to_string() | ||
| } else { | ||
| meta.default_model.clone() | ||
| }; | ||
| let model_config = match goose::model::ModelConfig::new(&seed) { | ||
| Ok(c) => c.with_canonical_limits(&meta.name), | ||
| Err(_) => continue, | ||
| }; | ||
| let temp_provider = match goose::providers::create(&meta.name, model_config, Vec::new()) | ||
| .await | ||
| { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| let _ = | ||
| cliclack::log::warning(format!("Skipping '{}': {e}", meta.display_name)); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| match tokio::time::timeout( | ||
| Duration::from_secs(15), | ||
| temp_provider.fetch_supported_models(), | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a configured Claude Code provider is installed but its Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. When the 15s list timeout cancels |
||
| .await | ||
| { | ||
| Ok(Ok(models)) if !models.is_empty() => { | ||
| for m in models { | ||
| let mut label = format!("{} ▸ {}", meta.display_name, m); | ||
| if meta.name.as_str() == current_provider_name | ||
| && m.as_str() == current_model_name | ||
| { | ||
| label.push_str(" (current)"); | ||
| } | ||
| entries.push((label, meta.name.clone(), m)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in The picker now asks each provider whether a model pick is meaningful via a new |
||
| } | ||
| } | ||
| Ok(Ok(_)) => { | ||
| let _ = cliclack::log::warning(format!( | ||
| "'{}' returned no models", | ||
| meta.display_name | ||
| )); | ||
| } | ||
| Ok(Err(e)) => { | ||
| let _ = cliclack::log::warning(format!( | ||
| "Could not list models for '{}': {e}", | ||
| meta.display_name | ||
| )); | ||
| } | ||
| Err(_) => { | ||
| let _ = cliclack::log::warning(format!( | ||
| "Timed out listing models for '{}'", | ||
| meta.display_name | ||
| )); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if entries.is_empty() { | ||
| output::render_error( | ||
| "No models available from any configured provider. Check that LM Studio is running or that provider keys are set.", | ||
| ); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| entries.sort_by(|a, b| { | ||
| let a_current = a.1.as_str() == current_provider_name; | ||
| let b_current = b.1.as_str() == current_provider_name; | ||
| b_current | ||
| .cmp(&a_current) | ||
| .then_with(|| a.0.to_lowercase().cmp(&b.0.to_lowercase())) | ||
| }); | ||
|
|
||
| let labels: Vec<String> = entries.iter().map(|(l, _, _)| l.clone()).collect(); | ||
| let chosen_label: String = if labels.len() > 12 { | ||
| crate::commands::configure::interactive_model_search(&labels)? | ||
| } else { | ||
| let items: Vec<(String, String, &str)> = | ||
| labels.iter().map(|l| (l.clone(), l.clone(), "")).collect(); | ||
| cliclack::select("Select a model:") | ||
| .items(&items) | ||
| .interact()? | ||
| }; | ||
|
|
||
| let (_, chosen_provider, chosen_model) = | ||
| match entries.into_iter().find(|(l, _, _)| l == &chosen_label) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When two rows render the same label, such as a custom/declarative provider using the same display name and model id as another provider, cliclack returns only that label string and this lookup always picks the first matching entry. In that scenario selecting the later row switches to the wrong provider/model; use a stable unique item value (or include the provider name in the value/label) so the selected row maps back to the intended entry. Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| Some(e) => e, | ||
| None => return Ok(()), | ||
| }; | ||
|
|
||
| if chosen_provider.as_str() == current_provider_name | ||
| && chosen_model.as_str() == current_model_name | ||
| { | ||
| output::goose_mode_message(&format!("Session already using model '{chosen_model}'")); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let new_model_config = | ||
| build_switched_model_config(&chosen_provider, &chosen_model, current_model_config)?; | ||
| let extensions = self.agent.get_extension_configs().await; | ||
| let new_provider = goose::providers::create(&chosen_provider, new_model_config, extensions) | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("Failed to create provider: {e}"))?; | ||
| self.agent | ||
| .update_provider(new_provider, &self.session_id) | ||
| .await?; | ||
| let mode = self.agent.goose_mode().await; | ||
| self.agent.update_goose_mode(mode, &self.session_id).await?; | ||
| output::goose_mode_message(&format!( | ||
| "Session model switched to '{chosen_model}' (provider '{chosen_provider}')" | ||
| )); | ||
| Ok(()) | ||
| } | ||
|
|
||
| async fn handle_plan_mode(&mut self, options: input::PlanCommandOptions) -> Result<()> { | ||
| self.run_mode = RunMode::Plan; | ||
| output::render_enter_plan_mode(); | ||
|
|
@@ -2162,6 +2304,19 @@ fn format_elapsed_time(duration: std::time::Duration) -> String { | |
| } | ||
| } | ||
|
|
||
| /// A provider is offered in the `/model` menu when all of its required config | ||
| /// keys are resolvable (via env var, stored secret/param, or a built-in default). | ||
| /// This keeps unconfigured cloud providers out of the list while always including | ||
| /// local (no-auth) providers like LM Studio and any provider whose key is set. | ||
| fn provider_is_configured(meta: &goose::providers::base::ProviderMetadata) -> bool { | ||
| let config = Config::global(); | ||
| meta.config_keys.iter().filter(|k| k.required).all(|k| { | ||
| std::env::var(&k.name).is_ok() | ||
| || k.default.is_some() | ||
| || config.get(&k.name, k.secret).is_ok() | ||
| }) | ||
| } | ||
|
|
||
| fn build_switched_model_config( | ||
| provider_name: &str, | ||
| model_name: &str, | ||
|
|
||
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.
When the current provider is Claude Code, Gemini CLI, or an ACP provider, this new no-argument path enters the picker before the existing
provider.manages_own_context()/-acpchecks below run. Those providers keep the authoritative conversation state in the subprocess, which is why/model <name>is rejected; allowing the picker to switch away from them can make the next provider continue from only Goose's partial transcript and lose hidden CLI/ACP context. Apply the same guard before opening the picker for these current providers.Useful? React with 👍 / 👎.
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.
Fixed in
ab9e2ae. The-acpandmanages_own_context()guards now run inhandle_modelbefore themodel.is_none()dispatch, so bare/modelis rejected for a context-managed current provider exactly like/model <name>already was. The picker can no longer switch away from Claude Code / Gemini CLI / an ACP provider and strand the conversation state they hold in their subprocess.