fix(providers): restore dynamic model discovery and current Grok support - #10756
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: 5d4e5a50ee
ℹ️ 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 !is_reasoning_model { | ||
| if !is_reasoning_model && !supports_xai_effort { |
There was a problem hiding this comment.
When GOOSE_TEMPERATURE or another caller sets model_config.temperature, this condition silently drops it for every Grok 4.5, 4.3, and 3-mini request merely because the model supports reasoning_effort, including requests where no reasoning effort is supplied. These are still Chat Completions models, and the bundled catalog explicitly marks Grok 4.3 and 4.5 as supporting temperature (canonical_models.json lines 160713-160720 and 160744-160751), so the new default Grok 4.5 no longer honors the user's configured sampling behavior; restrict omission to models or parameter combinations that actually reject temperature.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d2c87e306
ℹ️ 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".
| Config::global() | ||
| .get_param::<bool>(KIMI_CONFIGURED_MARKER) | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
When Kimi Code has been configured once and then removed through the provider-config delete path, KimiCodeProvider::cleanup only deletes kimicode/token.json while the metadata delete path does not remove kimi_code_configured. This new refresh registration therefore keeps reporting Kimi as configured from the stale marker, so an inventory refresh will instantiate the provider and fetch_supported_models falls through to ensure_token, starting the device-code OAuth flow in the refresh path instead of skipping an unconfigured provider; either clear the marker on cleanup/delete or also require the token cache to exist here.
Useful? React with 👍 / 👎.
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
I think a good approach @jbg - sorry am pushing a few small tweaks to get it green, but I like it.
only downside would be if say grok 4.6 comes out - will detect it, but not the context so will default low to 128k -I guess that is better than just not working.
all I could think would be to somehow default to next known highest one, but that seems clumsy/risky, so probably ok (until there is uniform way to discover that as well)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4da7caab06
ℹ️ 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".
| || model_name.starts_with("grok-4.20") | ||
| || model_name.starts_with("grok-4-0709") | ||
| || model_name.starts_with("grok-4-fast-reasoning") | ||
| || model_name.starts_with("grok-4-1-fast-reasoning") |
There was a problem hiding this comment.
For the xAI models matched only by these branches (for example grok-4.20-0309-reasoning and grok-4-0709), supports_xai_reasoning_effort is false, and create_request_with_options only serializes reasoning_effort through that helper's branch. Marking them as is_reasoning_model() feeds the ACP session config (thinking_effort_values), so users can select Low/Medium/High/Max, but the selected value is silently dropped from the request; keep this predicate limited to models whose effort Goose can configure, or add a separate capability for fixed server-side reasoning.
Useful? React with 👍 / 👎.
2 participants
Summary
Context
Supersedes #10345.
The immediate Grok failure was not just a stale hardcoded list: the Goose 2.0 inventory registration defaulted refresh off, so xAI's existing
/modelsimplementation was never called and the picker always fell back to provider metadata. The same mismatch affected several other providers with dynamicfetch_supported_modelsimplementations.There was a second forward-compatibility issue: the default recommended-model filter discarded any provider model absent from the bundled canonical registry. This meant a successful API refresh could still hide newly released models until the catalog was regenerated. Unknown models are now retained; known non-text or tool-incompatible models remain filtered.
xAI behavior follows the current reasoning documentation and May 15 retirement guide.
Verification
cargo fmtgit diff --check