fix(acp): resolve a stale pinned model value by its hint-stripped id - #942
Conversation
Resuming a session pinned to Fable aborted the turn before the prompt was ever sent, with "Selected ACP model value 'claude-fable-5[1m]' is no longer available for config option 'model'". The Fable picker row is internally inconsistent: its value is `claude-fable-5[1m]`, but it resolves to the bare `claude-fable-5`. The CLI records the resolved id in the transcript, so `session/load` rebuilds the picker keyed on that id and the `[1m]` value we stored is gone from the list. `ensure_config_option_has_value` then failed unconditionally, killing the turn. Opus and Sonnet round-trip fine — `opus[1m]` resolves to `claude-opus-5[1m]`, carrying the suffix through — so this only bit Fable. Fall back to matching the stored value's base id when the value itself is missing, and send that live id instead of the stale one. This keeps the pin an explicit assertion rather than leaving the model to whatever the agent restores from its own transcript. The fallback is one-way: a stored bare id never matches a hinted row, so a pin is never silently promoted to a context window the user did not choose. Covers the resume path end to end, which the existing tests missed — `resolves_stale_config_id_by_category` covered a stale config *id*, but nothing covered a stale *value*. Signed-off-by: Matt Toohey <contact@matttoohey.com>
![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: a1e5c7734f
ℹ️ 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".
| '{}'; falling back to '{base_value_id}'", | ||
| option.id | ||
| ); | ||
| return Ok(base_value_id.to_string()); |
There was a problem hiding this comment.
After this fallback succeeds, apply_or_record_session_config_options records the agent response containing only the bare value as the latest config_options_update, while the stored AcpConfigSelection still contains the hinted ID. On the next resume after an app restart (when the discovery cache is empty), resolve_resume_acp_config_selection uses that metadata and sanitize_acp_config_value_selection requires an exact value-ID match, so it drops and persists away the model pin before starting the run. The normalized ID therefore needs to be persisted, or the resume sanitizer must recognize this equivalence, otherwise the fix only preserves the pin for one resumed run.
Useful? React with 👍 / 👎.
| if let Some(base_value_id) = strip_config_value_hint(value_id) { | ||
| if select_option_has_value(option, base_value_id) == Some(true) { |
There was a problem hiding this comment.
This fallback runs for every selection category and treats any trailing bracketed portion as a removable hint, although ACP value IDs are opaque and the observed compatibility case is specifically the Claude model [1m] suffix. If, for example, a stale effort or another model has a meaningful ID such as high[fast] while the live selector also offers high, this now silently applies a different value instead of reporting the stored selection as unavailable. Restrict the workaround to the affected model/suffix case or otherwise verify provider-specific equivalence before stripping.
Useful? React with 👍 / 👎.
1 participant
Resuming a session pinned to Fable aborted the turn before the prompt was ever sent, with
Selected ACP model value 'claude-fable-5[1m]' is no longer available for config option 'model'.Why
The Fable picker row is internally inconsistent: its value is
claude-fable-5[1m], but it resolves to the bareclaude-fable-5. The CLI records the resolved id in the transcript, sosession/loadrebuilds the picker keyed on that id and the[1m]value we stored is gone from the list.ensure_config_option_has_valuethen failed unconditionally, killing the turn.Opus and Sonnet round-trip fine —
opus[1m]resolves toclaude-opus-5[1m], carrying the suffix through — so this only bit Fable.What changed
ensure_config_option_has_valuebecomesresolve_config_option_value, which returns the value to actually send rather than just validating. On an exact miss it falls back to the stored value's hint-stripped base id (claude-fable-5[1m]→claude-fable-5) and sends that live id instead of the stale one, logging a warning.resolve_session_config_option_selectionnow returns aResolvedConfigSelection { config_id, value_id }so the resolved value flows through toSetSessionConfigOptionRequest.Re-sending the resolved id keeps the pin an explicit assertion rather than leaving the model to whatever the agent restores from its own transcript. The fallback is deliberately one-way: a stored bare id never matches a hinted row, so a pin is never silently promoted to a context window the user did not choose.
Tests
Three new tests, including the resume path end to end — which the existing suite missed.
resolves_stale_config_id_by_categorycovered a stale config id, but nothing covered a stale value.resolves_hint_stripped_config_value_when_the_pinned_value_is_gonedoes_not_promote_a_bare_config_value_to_a_hinted_oneapplies_hint_stripped_model_value_when_load_drops_the_hint(fullsession/load→ re-apply flow against a test agent)