Skip to content

feat(settings): add Remove configuration for non-primary providers#280

Open
caezium wants to merge 3 commits into
JerryZLiu:mainfrom
caezium:feat/remove-provider-config
Open

feat(settings): add Remove configuration for non-primary providers#280
caezium wants to merge 3 commits into
JerryZLiu:mainfrom
caezium:feat/remove-provider-config

Conversation

@caezium

@caezium caezium commented Jun 3, 2026

Copy link
Copy Markdown

Summary

There's currently no way to remove a configured provider from Settings → Providers — you can only switch which one is active. If you've experimented with multiple providers (Local, Gemini, ChatGPT/Claude) the abandoned configs, Keychain entries, and UserDefaults keys stay around indefinitely, and the only escape hatch is hand-editing.

This PR adds a Remove configuration affordance for non-primary providers, plus two safety fixes for edge cases the removal path uncovered during implementation.

1. Add Remove configuration UI (feat)

A "Remove configuration" button on each non-primary provider row in SettingsProvidersTabView, wired through a new ProvidersSettingsViewModel.removeProviderConfig(_:) which:

  • Deletes Keychain entries for the provider
  • Removes \(provider)SetupComplete and provider-specific UserDefaults (model id, base URL, CLI tool preference)
  • Clears the backup-provider slot if this provider occupied it
  • Resets in-memory @Published state so the UI reflects the cleared config immediately

canRemoveProviderConfig(_:) gates the affordance to prevent removing:

  • Dayflow Pro (server-side entitlement, not local config)
  • A provider that isn't configured in the first place
  • The currently-active primary

2. Compare canonical provider IDs when gating removal (fix)

ChatGPT and Claude share one persisted config under the canonical id chatgpt_claude, but each surfaces as its own provider id in the routing layer. The initial guard compared raw provider IDs, which meant the chatgpt_claude config could be removed while claude (or chatgpt) was still active — bricking the provider until the user re-onboarded.

- guard primaryRoutingProviderId != providerId else { return false }
+ guard canonicalProviderId(for: primaryRoutingProviderId) != canonical else { return false }

The backup-clear branch in removeProviderConfig gets the same canonical comparison.

3. Stop UserDefaults write-back race on Local provider removal (fix)

The Local-provider branch of removeProviderConfig called UserDefaults.removeObject(forKey:) for llmLocalBaseURL, llmLocalModelId, llmLocalEngine, llmLocalAPIKey, then set the corresponding @Published properties to "". Each @Published has a didSet that writes the new value back into UserDefaults — so the removes were immediately overwritten with empty strings, leaving stale empty-string keys instead of cleared state.

Fix:

  • Add a suppressLocalSettingsPersistence flag guarded inside every Local-prop didSet
  • Extract resetLocalProviderFieldsAfterRemoval() which flips the flag, restores proper defaults (Ollama base URL, default model id) rather than blanks, and clears the flag in defer
  • Also call LocalModelPreferences.clearPreset() so the preset selection doesn't survive removal

Test plan

  • Builds (Debug, macOS arm64)
  • Configure two providers, switch primary, verify "Remove configuration" appears on the non-primary row only
  • Remove Gemini → Keychain entry deleted, geminiSetupComplete cleared, UI reverts to "Not configured"
  • Remove Local → UserDefaults keys actually removed (no empty-string ghosts), in-memory fields reset to Ollama defaults
  • Try to remove the active primary → button absent / disabled
  • With Claude as primary, "Remove configuration" hidden on chatgpt_claude row until primary switches away
  • With Local as backup, removing Local clears the backup slot, primary intact

caezium added 3 commits June 3, 2026 03:51
Today the Settings -> Providers tab lets you Set / Unset a provider in
the secondary slot, but leaves the underlying credentials (Keychain
entries, setup-complete flags, endpoint URLs, CLI tool preference)
behind. You can't fully revert after trying a provider out.

Adds:

- ProvidersSettingsViewModel.removeProviderConfig(_:) — wipes the
  per-provider persisted state for Gemini (Keychain + model preference
  + setup flag), Ollama (endpoint, model, engine, local API key,
  setup flag), and ChatGPT/Claude (CLI tool preference, setup flag).
  Also clears the backup-slot assignment if the removed provider was
  occupying it.
- canRemoveProviderConfig(_:) — guards against removing the currently
  active primary (would leave the app with no working provider) and
  against Dayflow Pro (server-side entitlement, not local config).
- GeminiModelPreference.clear() — small static helper so the view model
  doesn't need to know the private storage key.
- "Remove configuration" button per provider row + destructive
  confirmation alert with a per-provider message explaining what gets
  cleared (e.g. "deletes Gemini API key from Keychain").

`canonicalProviderId(for:)` promoted from private to internal so the
alert can render a provider-specific message.
ChatGPT and Claude share one persisted configuration under the
canonical id chatgpt_claude, but each surfaces as its own provider
id in the routing layer. The old guards compared the raw provider id
against primaryRoutingProviderId / isBackupProvider, which meant the
user could remove the chatgpt_claude config while "claude" (or
vice-versa) was still the active routing primary — bricking the
provider until they re-onboarded.

- canRemoveProviderConfig now compares canonical ids on both sides
  (canonicalProviderId(for: primaryRoutingProviderId) != canonical).
- removeProviderConfig's backup-clear branch likewise checks the
  stored backupProvider against canonical instead of routing the
  comparison through isBackupProvider(providerId) which uses raw ids.
…provider

removeProviderConfig's local branch did UserDefaults.removeObject for
llmLocalBaseURL, llmLocalModelId, llmLocalEngine, llmLocalAPIKey, then
set the @published fields to empty strings. Each @published has a
didSet that writes the new value back into UserDefaults — so the
removes were immediately overwritten with empty strings, leaving stale
keys instead of cleared state.

- Add a suppressLocalSettingsPersistence flag guarded inside every
  Local-prop didSet (localEngine, localBaseURL, localModelId,
  localAPIKey).
- Extract a resetLocalProviderFieldsAfterRemoval() helper that flips
  the flag, restores proper defaults (Ollama base URL, default model
  id) instead of blanks, and clears in defer.
- Also call LocalModelPreferences.clearPreset() so the preset
  selection doesn't survive removal.
@caezium

caezium commented Jun 7, 2026

Copy link
Copy Markdown
Author

@JerryZLiu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant