feat(macos): per-profile provider-connection picker#30271
Conversation
Closes audit finding #5 (profile editor cannot bind to a specific provider connection). The daemon schema already accepts `provider_connection` on `ProfileEntry` (see `assistant/src/config/schemas/llm.ts`); this PR plumbs the field through the macOS model + editor. UX: - New "Connection" sub-dropdown renders below Provider when the selected provider has at least one active connection. First option is "Any active <provider> connection" (value=""), preserving the daemon's legacy first-active-wins behavior. Subsequent options are the active connections, labelled by `label` (fallback: `name`). - Changing the Provider clears the connection binding so a stale name never survives a provider switch. - A "Not found" warning badge surfaces when the saved binding no longer matches any active connection (most often: a connection was disabled or deleted outside the editor). Wire shape: snake_case `provider_connection` for the JSON key (matches the Zod schema); camelCase `providerConnection` for the Swift property. `preservedJSON` exclusion list updated so the snake_case key doesn't double-write. Connections are fetched once by `InferenceProfilesSheet.task` and re-fetched when the editor closes (in case a connection was added in another surface during the session). Tolerant on transport failure (stale list preserved) — same posture as `SettingsStore.providerKeys`. Tests: - 4 new `InferenceProfileTests` cases cover wire-shape round-trip, empty-string decode, merging-from-fragment, merging-preserves. - 5 new `InferenceProfileEditorTests` cases cover the per-provider filter, empty-state behaviors, the display-name helper, and the full body construction with a populated `connections` list. - `InferenceProfilesSheetTests` updated to inject the mock client. No daemon changes required. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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: f0375fae54
ℹ️ 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".
| @ViewBuilder | ||
| private var connectionField: some View { | ||
| let available = availableConnectionsForProvider | ||
| if !available.isEmpty, let provider = profile.provider { |
There was a problem hiding this comment.
The Connection field is hidden whenever there are no active matches (if !available.isEmpty), which means a profile that already has providerConnection set to a now-disabled/deleted row cannot be cleared from this editor. In single-provider workspaces the user may have no alternate provider to trigger the provider-change reset path, so the profile can remain stuck with an invalid binding and continue failing connection resolution. Consider rendering this field (with the existing "Not found" warning) whenever a binding exists, even if available is empty.
Useful? React with 👍 / 👎.
| if let label { result["label"] = label } | ||
| if let profileDescription { result["description"] = profileDescription } | ||
| if let provider { result["provider"] = provider } | ||
| if let providerConnection { result["provider_connection"] = providerConnection } |
There was a problem hiding this comment.
When the user chooses “Any active … connection”, providerConnection becomes nil, and toJSON() omits provider_connection. However, the profile-replace route only clears keys listed in INFERENCE_PROFILE_UI_KEYS (in assistant/src/runtime/routes/conversation-query-routes.ts), and that set does not include provider_connection, so the old server-side binding is retained. The UI then appears cleared while runtime routing still uses the stale connection. Add an explicit clear mechanism for this field (or include it in the server-side UI key clear set).
Useful? React with 👍 / 👎.
…_connection in route Codex flagged two P1s on the original PR: 1. **Picker hidden for stale bindings.** When a profile's `providerConnection` referenced a now-disabled or deleted connection AND the workspace had no active connections for that provider, the editor hid the entire Connection field. The user had no way to clear the broken binding from the UI, and the profile remained pinned to an invalid name. Refactor: visibility now gates on (active matches OR non-empty saved binding); when stale, the picker shows a '<name> (not found)' option so the trigger renders the bound name, and the Not Found badge surfaces the state. Selecting 'Any active …' clears the binding back to the daemon's first-active fallback. 2. **`provider_connection` not in `INFERENCE_PROFILE_UI_KEYS`.** The profile-replace route only clears keys listed in that set, so omitting `provider_connection` from the body (which is what the UI does when the user picks 'Any active') silently retained the old server-side binding. Added `provider_connection` so the route now correctly wipes the stored value when the UI clears it. Tests: - 5 new InferenceProfileEditorTests covering staleProviderConnection truth table (binding missing / disabled / matches / nil / empty) + body-builds-with-stale. - 2 new conversation-query-routes tests covering write + clear paths for `provider_connection` on PUT /v1/config/llm/profiles/:name. 13/13 daemon tests pass; assistant tsc + eslint clean. Addresses Codex P1 #1 + P1 #2 from `@codex review` on `f0375fae54`.
|
Round 2 — addressed both P1 findings. Commit: P1: Keep connection picker visible for stale bindings. Refactored visibility to gate on (active matches OR non-empty saved binding). When the binding is stale, the dropdown surfaces the bound name as a P1: Preserve clear semantics for Tests added:
|
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
What
Closes audit finding #5: profile editor cannot bind to a specific provider connection.
The daemon schema already accepts
provider_connectiononProfileEntry(seeassistant/src/config/schemas/llm.tsline ~340). This PR plumbs the field through the macOS model + editor so a user with multipleopenai(oranthropic/gemini/ ...) connections can route a profile to a specific row instead of relying on the dispatcher's "first active wins" fallback.UX
""), preserving the daemon's legacy first-active-wins behavior.label(fallback:name).Wire shape
provider_connection(snake_case — matches the Zod schema)providerConnection(camelCase)preservedJSONexclusion list updated so the snake_case key doesn't double-write.Fetching
Connections are fetched once by
InferenceProfilesSheet.taskand re-fetched when the editor closes (to pick up a connection added in another surface during the same session). Tolerant on transport failure (stale list preserved) — same posture asSettingsStore.providerKeys.Tests
InferenceProfileTests: wire-shape round-trip, empty-string decode-as-nil, merging-from-fragment, merging-preserves.InferenceProfileEditorTests: per-provider + per-status filter, empty-state behaviors (no provider, no connections),connectionDisplayNamehelper (label preferred, fallback to name), full body construction with populatedconnectionslist.InferenceProfilesSheetTestsupdated to inject the mock client.No daemon changes
The wire field already exists in the Zod schema. The daemon's resolver already routes via
provider_connectionwhen set. This is a UI-only change on the macOS side.🤖 Generated with Claude Code