feat(acp): add provider config picker for sessions#869
Merged
Conversation
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Remove the unused provider-only AgentSelector and add regression coverage for launch payloads, queued-session drain selection handoff, resume payloads, stale value fallback, and unavailable selector fallback. 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: b84afdafd6
ℹ️ 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".
Comment on lines
+1619
to
+1620
| let discovery = fresh_cached_acp_config_discovery(session.provider.as_deref()).or_else(|| { | ||
| latest_acp_config_discovery_from_session_metadata( |
There was a problem hiding this comment.
When resuming without an explicit selection (for example queued follow-ups or the timeline Resume action), this checks the provider-wide discovery cache before the session's own config metadata. That cache is populated by the initial/default-model picker discovery and is not keyed by the stored model, so a session saved with model=opus plus an effort value that only appears after switching to Opus can have its effort sanitized against the default model's options, dropped, and then persisted as a truncated selection. Prefer the session metadata or a model-specific discovery when the stored selection includes a model.
Useful? React with 👍 / 👎.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Selecting a provider dismissed the whole picker dropdown, so the model
and effort columns for the new provider could never be reached. Mark the
provider, model, and effort radio items closeOnSelect={false} so the
popover behaves like a settings panel, and clear the stale discovery
config when a new discovery starts so the now-visible columns show the
loading state instead of the previous provider's options.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
The trigger button showed provider · model · effort, which overflowed and hid the values that actually change. The provider icon already identifies the provider, so remove its text from the trigger label in both pickers, fall back to the provider name only while no model or effort labels are available, and surface the provider name in the trigger's hover title instead. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Style the fixed model and effort picker like the new-session dialog control and place it before the attachment button in the existing-session composer. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Close the existing-session model and effort picker when Return is pressed after the focused option has handled selection. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Make up and down arrow navigation activate picker values immediately, and make Return dismiss the picker without selecting the focused item. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Changing the model made the effort column blink through a spinner row even though the refreshed options are usually the same, and the trigger button swapped its chevron for a spinner on every reload. Retain the previous effort selector and render it as muted disabled options while the model-specific options load, replacing the retained-width placeholder, and only show the trigger spinner before any model or effort content is available. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The trigger label values fly in and out on change but the button width snapped to the new content size. Render a hidden max-content sizer copy of the trigger parts, observe its size, and set an explicit transitioned width on the visible label so the button eases between content widths. Flex shrink still applies to the explicit width, so tight rows truncate exactly as before. Also deduplicate the trigger label markup between the dropdown and static button branches with a shared snippet. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Address the code review feedback on the config picker series: - Export is_config_selection_unavailable_error from acp-client, built on the same string constants the driver's error producers use, and drop the app-side substring copy so rewording an error message can't silently disable the stale-selection cleanup. - Persist the resume config selection before spawning the session run in both the Tauri command and web server resume paths, so an instant unavailable-config failure can't have its selection cleanup overwritten by a late re-persist of the stale value. - Stop failing resumed prompts when an agent that advertised config options on session/new omits them from session/load. The selection was applied when the session was created and lives in the agent's session state, so skip the re-apply with a warning instead of erroring the first follow-up after an app restart. - Keep the effort column usable when model-specific discovery fails: the new-session picker restores the previous model's effort options and the follow-up composer falls back to the non-model-specific options, instead of the column disappearing until the provider or working directory changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Typing in a composer next to the config picker made the trigger's icon and border shimmer. The app's 13px rem base gives the trigger fractional-pixel geometry, and partial repaints of the surrounding layer re-rasterize those between-device-pixel edges with slightly different anti-aliasing each pass. Promote the trigger to its own compositing layer with translateZ(0) so it only re-rasterizes when its own content changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Compact panes are too narrow to fit the config picker, attach button, text input, and send button in a single composer row. Let the compact input area wrap, order the hashtag input first as its own full-width row, and push the send/stop button to the right edge of the controls row beneath it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a model/effort config picker for ACP providers, letting users choose a provider configuration when launching a session and adjust it on follow-up prompts.
AcpConfigPicker,AcpConfigPickerShell,AcpFixedConfigPicker) with full keyboard navigation, used in the new-session modal and chat follow-up controlsacp_config_selectioncolumn, migration 0020) and apply it before prompting, including on session resumeacp-clientand thread the selection through session launch, queueing, and the web serverdoctorTest plan
cargo testcovers config normalization, selection persistence, store migrations, and session command regressions🤖 Generated with Claude Code