fix: activate custom provider after adding via configure#9213
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 551c1868a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let model_items: Vec<_> = provider_config | ||
| .models | ||
| .iter() | ||
| .map(|m| (m.name.as_str(), m.name.as_str(), "")) | ||
| .collect(); | ||
| cliclack::select("Which model should be the default?") | ||
| .items(&model_items) | ||
| .interact()? |
There was a problem hiding this comment.
If the user enters only commas/spaces in the model prompt (e.g. ", ,"), the current validator still accepts it, models becomes empty after trimming/filtering, and this new activation path reaches the select("Which model should be the default?") call with zero items. In that case, activation fails at runtime after the provider has already been created, leaving users in a partial-success state. This regression is introduced by the new post-create activation logic and is triggered specifically when provider_config.models is empty.
Useful? React with 👍 / 👎.
After adding a custom provider through 'goose configure → Custom Providers → Add', the provider was saved to disk but not set as the active provider in config.yaml. Users had to separately navigate to 'Configure Providers' to select it. Now prompts the user to set the new provider as active immediately after creation, and if confirmed, writes GOOSE_PROVIDER and GOOSE_MODEL to config.yaml. Closes #6692 Signed-off-by: Douwe Osinga <douwe@squareup.com>
DOsinga
force-pushed
the
fix/activate-custom-provider-6692
branch
from
551c186 to
1df2d3e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1df2d3e4af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Ok(model) = cliclack::select("Which model should be the default?") | ||
| .items(&model_items) | ||
| .interact() | ||
| { |
There was a problem hiding this comment.
The new activation step silently ignores any error from select(...).interact() by using if let Ok(model), so cancellation/interrupts (for example Esc/Ctrl+C) or terminal I/O errors now fall through to a successful Custom provider added outro without writing GOOSE_PROVIDER/GOOSE_MODEL. This creates a partial-success state introduced by this commit: provider creation succeeds, but activation may be skipped with no feedback. Please handle the error path explicitly (or propagate it) so users don’t get a false success signal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
well, it silently didn't activate before, so this is no worse. up to you whether to deal with this
2 participants
After adding a custom provider through
goose configure → Custom Providers → Add, the provider was saved to disk but not set as the active provider in config.yaml. Users had to separately navigate to "Configure Providers" to select it.Now prompts the user to set the new provider as active immediately after creation, and if confirmed, writes
GOOSE_PROVIDERandGOOSE_MODELto config.yaml. If only one model was entered, it's auto-selected; if multiple, the user picks the default.Closes #6692