💭 I'm checking the web files involved—ProviderInstanceCard.tsx, ProviderSettingsPanel.tsx, and providerInstances.ts—since these handle the enabled-state resolution logic rather than styling. I want to verify whether other parts of the web app still fall back to instance.enabled ?? true for consistency. This is a UI consistency check since the web changes are logic-only with no styling changes, though there could be implications for cards tied to disabled-by-default providers. I should check other web files for places that compute enabled state for provider instances or toggles, like ProviderSettingsPanel, which might derive enabled status elsewhere. I don't see any UI styling changes in this diff for web, so I'm double-checking the ProviderInstanceCard region to confirm nothing about rendering changed, particularly around how built-in providers used to write their enabled flag versus now. I want to check whether the toggle handling still writes config.enabled directly, which could conflict with the new envelope structure — that's more of a behavioral concern than pure UI consistency, but it's still within scope since it touches the changed lines. Let me look at the relevant sections directly. I'm worried that updateEnabled only writes the envelope's enabled flag, so if a legacy config still has its own enabled: false before the server folds it, toggling on locally could still resolve to false since explicit false wins. I need to check whether the server strips that legacy field on update and whether the web UI applies the change optimistically before getting the server's response. This looks like a real potential regression: the toggle could snap back to off until the round-trip completes, caused by the changed line resolving enabled state differently. I should trace how ProviderSettingsPanel's onUpdate flows through the web settings update path to confirm whether it's optimistic or server-driven. I'm trying to figure out where these rows originate — whether they come from server settings already folded in, or from a local legacy migration path. I recall there's logic around lines 530-560 where legacyConfig.enabled synthesizes instance rows for built-in defaults, so I want to look closer at that section. For the default slot without an explicit instance, the effective config falls back to the legacy provider blob, so enabled starts false for providers like grok/cursor/opencode, matching the envelope state. I'm now tracing what happens when the user flips the toggle on—the update call passes the legacy config through with enabled set to true. If buildProviderInstanceUpdatePatch doesn't strip the stale enabled: false from the nested config, resolveProviderInstanceEnabled would see the explicit false inside config win out, making the switch appear to snap back off locally until the server round-trip corrects it. That looks like a genuine UI bug for the default slot of legacy-mirrored providers, so I want to check how buildProviderInstanceUpdatePatch actually constructs the patch. I'm realizing the default slot toggle patch resets the legacy provider config to enabled:false while the envelope says enabled:true, and the server folds these by favoring the config's false value—meaning turning on Grok, Cursor, or OpenCode from Settings would silently fail to actually enable them. This looks like a genuine regression introduced by the PR, so I want to double-check whether the legacy config value is really what I think it is. If settings update optimistically, the UI would immediately flip back to off either way, confirming the bug. I'm also checking whether ProviderInstanceCard strips the enabled field from config before calling onUpdate — previously the envelope value won over the legacy config flag, but now explicit false wins, which breaks the toggle. I want to trace whether the config object passed through is really the raw legacy blob or something sanitized, and I'm looking at the client-side settings application logic to confirm. This looks like a genuine interaction regression on changed lines within scope, since toggling the instance on doesn't actually re-enable it in pickers — I'd flag this as a concrete bug rather than just a style nit. I still want to verify whether the settings panel strips the legacy enabled flag when building the effective instance, and confirm the server-side normalization folds the enabled flags consistently on updates too. I should also check whether there's a sanitize step somewhere, like in ProviderInstanceCard's built-in config handling, or in the updateConfigField function used by the card. I'm confirming that the toggle reads from resolveProviderInstanceEnabled while the update only writes the envelope's enabled flag, so any instance whose config still carries an explicit enabled:false—like the legacy-blob default slot or one created by an older client—will silently ignore the toggle since that stale false takes precedence. I'm now checking whether this breaks the toggle-off-then-on flow even for a normally stored instance once the server has folded the config properly. For fresh installs of Grok, Cursor, or OpenCode, the legacy blob's enabled:false gets carried into the synthesized config, so flipping the toggle on still writes a config with enabled:false, the server folds it back down, and the UI bounces right back off—this is the core bug the PR is meant to fix, and I'm fairly confident in this finding. The cleanest fix is to strip the stale enabled key out of config when writing the update, so I'm adjusting updateEnabled to destructure out the legacy flag before merging the rest of the config back into the update payload. I'm drafting a review comment for line 396 in ProviderInstanceCard.tsx, pointing out that the toggle can never be enabled for the default slot since its config uses the legacy blob format. I'll suggest stripping the legacy in-config flag when the switch writes its value. Since the actual fix location (the updateEnabled function around line 464) falls outside the diff, I need to present it as a standalone diff block rather than an inline suggestion. I'm also weighing whether the real root cause is in ProviderSettingsPanel's config synthesis around line 536-540, where stripping the enabled field from the legacy config there might be the more fundamental fix, though older saved instances would still need the server-side fold to handle stray config.enabled values on load. I'm deciding where to anchor my comment — the card's checked-state resolution at line 396 is self-contained and in the diff, while the panel's line 538 is the true root cause but sits outside the diff. I'll go with a self-contained fix at the card location rather than splitting this across two comments or using a cross-file diff. But I need to verify this fix actually holds: stripping config.enabled when toggling works for the default-slot flow, but I should check whether other config edits (like editing binaryPath) also write back a stale config.enabled that could silently override the envelope's enabled state. The cleanest fix is to strip the legacy enabled flag wherever the config blob gets written, likely at the panel synthesis step around where it builds from legacyConfig. I'm deciding where to anchor my review comment—probably on ProviderInstanceCard describing the regression, with a cross-file diff pointing to the panel's synthesis logic. I'm verifying the exact line numbers match between the head file and the diff hunk, confirming the Switch's checked state resolves through resolveProviderInstanceEnabled, and double-checking that the web client does send config for the default slot so the fix actually addresses the real code path. Now I'm drafting the review comment itself, explaining that enabling an off-by-default provider from its default card is a no-op because the synthesized default-slot instance still carries the legacy enabled: false in its config blob, so the most-restrictive resolution rule snaps the toggle back off even after the user flips it. I'm suggesting the fix should drop the stale legacy enabled flag when synthesizing the instance. Trying out a diff that strips the enabled key out of the legacy config before spreading it into the new instance, but I'm second-guessing whether setting enabled: undefined will pass the stricter optional-property typecheck, so I'm reworking the approach to avoid that issue.

1. modify_pr — One interaction regression found in the settings UI: with the new most-restrictive resolveProviderInstanceEnabled rule, the enable Switch on a default provider card can no longer turn an off-by-default provider on, because the card's instance still carries the legacy config.enabled: false from the legacy provider blob. Details inline.