fix(providers): enable inventory refresh for OpenRouter model picker - #10641
Conversation
Register OpenRouterProvider with refresh_only inventory so the model picker fetches all models dynamically from the OpenRouter API instead of falling back to the 10-entry KNOWN_MODELS static list. Also add skip_canonical_filtering so all models returned by the OpenRouter API are surfaced without being filtered against the canonical registry (mirroring the LiteLLM fix from aaif-goose#10489). Fixes aaif-goose#10631
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abb9ea09ae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The fetch_supported_models comment always said "only models with tool support" but the filter was never implemented. Now checks supported_parameters contains "tools" per OpenRouter API docs before including a model in the inventory list.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37dd89e786
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Extract fetch_model_entries helper and move tool-support filtering into a fetch_recommended_models override that receives the toolshim flag. When toolshim is enabled all models are included; when disabled only models with supported_parameters containing tools are returned.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdff3948ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
bdff394 to
44fbeab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2784cb9bf7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| true | ||
| } | ||
|
|
||
| async fn fetch_recommended_models(&self, toolshim: bool) -> Result<Vec<String>, ProviderError> { |
There was a problem hiding this comment.
Keep OpenRouter fetch_supported_models populated
Because this moves the OpenRouter API catalogue fetch from fetch_supported_models to only fetch_recommended_models, any caller that asks for supported models now falls back to the trait default Ok(vec![]); for example the ACP ProviderSupportedModelsListRequest handler in crates/goose/src/acp/server/providers.rs:481-484 still calls fetch_supported_models() directly. In that path, configured OpenRouter users will see an empty supported-models response even though the API has models, so keep a fetch_supported_models override and have the recommended-model filtering layer build on top of it.
Useful? React with 👍 / 👎.
michaelneale
left a comment
There was a problem hiding this comment.
LGTM - one Q about the secret for openrouter but I think it is fine.
* origin/main: (24 commits) upgrade to rmcp 2.0 (#10584) chore: opus5 support (including adaptive thinking) (#10686) fix(desktop): use shell working directory on initial launch (#10655) fix(providers): enable inventory refresh for OpenRouter model picker (#10641) Add configurable GOOSE_DOCS_ROOT for air-gapped docs access (#10294) fix(i18n): fix Korean translation errors, untranslated entries, and terminology consistency (#10667) fix(acp): preserve read lines and reload tool output (#10662) fix(acp): surface tool call output in CLI and Desktop (#10654) fix overlong function names in provider requests (#10659) fix (desktop): preserve ACP tool call update fields (#10653) feat: support latest Gemini models (#10630) Merge commit from fork feat (acp): Gate tool-call label enrichment on ACP client capability (#10644) fix(session): use configured session manager for tool summaries (#10628) refactor (acp): simplify ACP tool-call handling and fix chain summaries (#10599) fix: omit disabled OpenRouter reasoning (#10517) enhance the uniffi API layer (#10427) remove sampling from smoke test (#10639) rebuild canonical model db (#10633) fix(ui): clear stale pending ACP connection after terminal recovery failure (#10552) ... # Conflicts: # ui/desktop/src/i18n/messages/ko.json
Fixes #10631
Summary
Register OpenRouterProvider with refresh_only inventory so the model picker fetches all models dynamically from the OpenRouter API instead of falling back to the 10-entry KNOWN_MODELS static list.
Also add skip_canonical_filtering so all models returned by the OpenRouter API are surfaced without being filtered against the canonical registry (mirroring the LiteLLM fix from #10489).
Testing
manual
Screenshots/Demos (for UX changes)
test.mov