refactor(desktop): unify API config IPC + drop separate fast model slot

.changeset/unify-api-config.md

@@ -0,0 +1,28 @@
+---
+'@open-codesign/desktop': minor
+'@open-codesign/shared': minor
+---
+
+refactor(desktop): unify API config IPC + drop the separate "fast" model slot
+
+Two threads of cleanup, shipped together because they both reshape the API config surface area.
+
+## 1. Canonical IPC for adding/updating a provider
+
+The 3 surfaces that touch API config (top-bar pill, Settings → API 服务, onboarding) used to call 3 different IPC handlers with 3 different return shapes. The Settings `add-provider` path returned `ProviderRow[]` and never updated the Zustand store, so the top-bar pill could lag behind reality until a manual reload.
+
+- New canonical handler `config:v1:set-provider-and-models` accepts `{ provider, apiKey, modelPrimary, baseUrl?, setAsActive }` and atomically writes config + returns full `OnboardingState`.
+- Existing `onboarding:save-key`, `settings:v1:add-provider`, `settings:v1:set-active-provider` are now thin delegates of the new handler — back-compat preserved.
+- Settings → AddProviderModal and the post-add Zustand sync now both go through the new handler. `handleAddSave` re-pulls `OnboardingState` so the top-bar pill reflects the new active provider immediately.
+- Preload bridge: new `window.codesign.config.setProviderAndModels()`.
+
+## 2. Drop the separate `modelFast` model slot
+
+The `modelFast` field was used in exactly one place (`applyComment` fallback), and its value was indistinguishable from `modelPrimary` for users — which is why every Settings UI showed two near-identical dropdowns and confused everyone.
+
+- `Config.modelFast` is dropped from the v2 schema. v1 configs are accepted (Zod treats `modelFast` as optional and the new handler never writes it back).
+- `OnboardingState.modelFast` removed; `ProviderShortlist.fast` / `defaultFast` removed.
+- `applyComment` now falls back to `modelPrimary` instead of `modelFast`.
+- All AddProviderModal / ChooseModel / ModelsTab UI drops the second dropdown.
+
+Schema bump: `Config.version: 1 → 2`. Migration is read-only (drop the dead field on next write); no data loss.

apps/desktop/src/main/index.ts

@@ -376,7 +376,7 @@ function registerIpcHandlers(): void {
     // Inline-comment edits don't need to be tied to whatever provider was
     // pinned in the original generate; resolve fresh against the canonical
     // active provider so a switch in Settings takes effect immediately.
-    const hint = payload.model ?? { provider: cfg.provider, modelId: cfg.modelFast };
+    const hint = payload.model ?? { provider: cfg.provider, modelId: cfg.modelPrimary };
     const active = resolveActiveModel(cfg, hint);
     const apiKey = getApiKeyForProvider(active.model.provider);
     const baseUrl = active.baseUrl ?? undefined;

apps/desktop/src/main/onboarding-ipc.test.ts

@@ -112,6 +112,45 @@ describe('registerOnboardingIpc — channel versioning', () => {
       expect(registeredChannels).toContain(ch);
     }
   });
+
+  it('registers the canonical config:v1:set-provider-and-models handler', async () => {
+    const { registerOnboardingIpc } = await import('./onboarding-ipc');
+    registerOnboardingIpc();
+    expect(registeredChannels).toContain('config:v1:set-provider-and-models');
+  });
+});
+
+describe('config:v1:set-provider-and-models — payload validation', () => {
+  it('rejects payloads without a setAsActive boolean', async () => {
+    const { registerOnboardingIpc } = await import('./onboarding-ipc');
+    registerOnboardingIpc();
+    const handler = handlers.get('config:v1:set-provider-and-models');
+    expect(handler).toBeDefined();
+    if (!handler) return;
+    await expect(
+      handler({} as never, {
+        provider: 'openrouter',
+        apiKey: 'sk-test',
+        modelPrimary: 'a',
+      }),
+    ).rejects.toThrow(/setAsActive/);
+  });
+
+  it('rejects payloads with an unsupported schemaVersion', async () => {
+    const { registerOnboardingIpc } = await import('./onboarding-ipc');
+    registerOnboardingIpc();
+    const handler = handlers.get('config:v1:set-provider-and-models');
+    if (!handler) throw new Error('handler missing');
+    await expect(
+      handler({} as never, {
+        schemaVersion: 99,
+        provider: 'openrouter',
+        apiKey: 'sk-test',
+        modelPrimary: 'a',
+        setAsActive: true,
+      }),
+    ).rejects.toThrow(/schemaVersion/);
+  });
 });
 describe('registerOnboardingIpc — validate-key passes baseUrl to pingProvider', () => {
   it('forwards baseUrl to pingProvider when provided', async () => {
@@ -151,10 +190,9 @@ describe('getApiKeyForProvider — API key retrieval', () => {
     // Override readConfig to return a config with an anthropic secret.
     const { readConfig } = await import('./config');
     vi.mocked(readConfig).mockResolvedValueOnce({
-      version: 1,
+      version: 2,
       provider: 'anthropic',
       modelPrimary: 'claude-sonnet-4-6',
-      modelFast: 'claude-haiku-3',
       secrets: { anthropic: { ciphertext: 'enc:sk-ant-test' } },
       baseUrls: {},
     });

apps/desktop/src/main/onboarding-ipc.ts

@@ -27,7 +27,6 @@ interface SaveKeyInput {
   provider: SupportedOnboardingProvider;
   apiKey: string;
   modelPrimary: string;
-  modelFast: string;
   baseUrl?: string;
 }
 
@@ -82,7 +81,6 @@ function toState(cfg: Config | null): OnboardingState {
       hasKey: false,
       provider: null,
       modelPrimary: null,
-      modelFast: null,
       baseUrl: null,
       designSystem: null,
     };
@@ -92,7 +90,6 @@ function toState(cfg: Config | null): OnboardingState {
       hasKey: false,
       provider: null,
       modelPrimary: null,
-      modelFast: null,
       baseUrl: null,
       designSystem: cfg.designSystem ?? null,
     };
@@ -103,7 +100,6 @@ function toState(cfg: Config | null): OnboardingState {
       hasKey: false,
       provider: cfg.provider,
       modelPrimary: null,
-      modelFast: null,
       baseUrl: null,
       designSystem: cfg.designSystem ?? null,
     };
@@ -112,7 +108,6 @@ function toState(cfg: Config | null): OnboardingState {
     hasKey: true,
     provider: cfg.provider,
     modelPrimary: cfg.modelPrimary,
-    modelFast: cfg.modelFast,
     baseUrl: cfg.baseUrls?.[cfg.provider]?.baseUrl ?? null,
     designSystem: cfg.designSystem ?? null,
   };
@@ -153,7 +148,6 @@ function parseSaveKey(raw: unknown): SaveKeyInput {
   const provider = r['provider'];
   const apiKey = r['apiKey'];
   const modelPrimary = r['modelPrimary'];
-  const modelFast = r['modelFast'];
   const baseUrl = r['baseUrl'];
   if (typeof provider !== 'string' || !isSupportedOnboardingProvider(provider)) {
     throw new CodesignError(
@@ -167,10 +161,7 @@ function parseSaveKey(raw: unknown): SaveKeyInput {
   if (typeof modelPrimary !== 'string' || modelPrimary.trim().length === 0) {
     throw new CodesignError('modelPrimary must be a non-empty string', 'IPC_BAD_INPUT');
   }
-  if (typeof modelFast !== 'string' || modelFast.trim().length === 0) {
-    throw new CodesignError('modelFast must be a non-empty string', 'IPC_BAD_INPUT');
-  }
-  const out: SaveKeyInput = { provider, apiKey, modelPrimary, modelFast };
+  const out: SaveKeyInput = { provider, apiKey, modelPrimary };
   if (typeof baseUrl === 'string' && baseUrl.trim().length > 0) {
     try {
       new URL(baseUrl);
@@ -213,29 +204,79 @@ function runListProviders(): ProviderRow[] {
   return toProviderRows(getCachedConfig(), decryptSecret);
 }
 
-async function runAddProvider(raw: unknown): Promise<ProviderRow[]> {
-  const input = parseSaveKey(raw);
+interface SetProviderAndModelsInput extends SaveKeyInput {
+  setAsActive: boolean;
+}
+
+function parseSetProviderAndModels(raw: unknown): SetProviderAndModelsInput {
+  if (typeof raw !== 'object' || raw === null) {
+    throw new CodesignError('set-provider-and-models expects an object payload', 'IPC_BAD_INPUT');
+  }
+  const r = raw as Record<string, unknown>;
+  const sv = r['schemaVersion'];
+  if (sv !== undefined && sv !== 1) {
+    throw new CodesignError(
+      `Unsupported schemaVersion ${String(sv)} (expected 1)`,
+      'IPC_BAD_INPUT',
+    );
+  }
+  const setAsActive = r['setAsActive'];
+  if (typeof setAsActive !== 'boolean') {
+    throw new CodesignError('setAsActive must be a boolean', 'IPC_BAD_INPUT');
+  }
+  return { ...parseSaveKey(raw), setAsActive };
+}
+
+/**
+ * Canonical "add or update a provider" mutation. Atomic: writes secret +
+ * baseUrl + (optionally) flips active provider in a single writeConfig.
+ *
+ * Returns the full OnboardingState so renderer can hydrate Zustand without a
+ * follow-up read — that store-sync gap is what made TopBar drift out of date
+ * after Settings mutations.
+ */
+async function runSetProviderAndModels(input: SetProviderAndModelsInput): Promise<OnboardingState> {
   const ciphertext = encryptSecret(input.apiKey);
   const nextBaseUrls = { ...(cachedConfig?.baseUrls ?? {}) };
   if (input.baseUrl !== undefined) {
     nextBaseUrls[input.provider] = { baseUrl: input.baseUrl };
   } else {
     delete nextBaseUrls[input.provider];
   }
-  const nextDefaults = getAddProviderDefaults(cachedConfig, input);
-  const next: Config = {
-    version: 1,
-    provider: nextDefaults.activeProvider,
-    modelPrimary: nextDefaults.modelPrimary,
-    modelFast: nextDefaults.modelFast,
-    secrets: {
-      ...(cachedConfig?.secrets ?? {}),
-      [input.provider]: { ciphertext },
-    },
-    baseUrls: nextBaseUrls,
+  const nextSecrets = {
+    ...(cachedConfig?.secrets ?? {}),
+    [input.provider]: { ciphertext },
   };
+  const activate = input.setAsActive || cachedConfig === null;
+  const next: Config = activate
+    ? {
+        version: 2,
+        provider: input.provider,
+        modelPrimary: input.modelPrimary,
+        secrets: nextSecrets,
+        baseUrls: nextBaseUrls,
+      }
+    : {
+        version: 2,
+        provider: cachedConfig?.provider ?? input.provider,
+        modelPrimary: cachedConfig?.modelPrimary ?? input.modelPrimary,
+        secrets: nextSecrets,
+        baseUrls: nextBaseUrls,
+      };
   await writeConfig(next);
   cachedConfig = next;
+  configLoaded = true;
+  return toState(cachedConfig);
+}
+
+async function runAddProvider(raw: unknown): Promise<ProviderRow[]> {
+  const input = parseSaveKey(raw);
+  const defaults = getAddProviderDefaults(cachedConfig, input);
+  await runSetProviderAndModels({
+    ...input,
+    setAsActive: defaults.activeProvider === input.provider,
+    modelPrimary: defaults.modelPrimary,
+  });
   return toProviderRows(cachedConfig, decryptSecret);
 }
 
@@ -250,15 +291,13 @@ async function runDeleteProvider(raw: unknown): Promise<ProviderRow[]> {
   const nextBaseUrls = { ...(cfg.baseUrls ?? {}) };
   delete nextBaseUrls[raw];
 
-  const { nextActive, modelPrimary, modelFast } = computeDeleteProviderResult(cfg, raw);
+  const { nextActive, modelPrimary } = computeDeleteProviderResult(cfg, raw);
 
   if (nextActive === null) {
-    // No providers left — write a tombstone config so onboarding triggers again.
     const emptyNext: Config = {
-      version: 1,
+      version: 2,
       provider: cfg.provider,
       modelPrimary: '',
-      modelFast: '',
       secrets: {},
       baseUrls: {},
     };
@@ -268,10 +307,9 @@ async function runDeleteProvider(raw: unknown): Promise<ProviderRow[]> {
   }
 
   const next: Config = {
-    version: 1,
+    version: 2,
     provider: nextActive,
     modelPrimary,
-    modelFast,
     secrets: nextSecrets,
     baseUrls: nextBaseUrls,
   };
@@ -287,27 +325,24 @@ async function runSetActiveProvider(raw: unknown): Promise<OnboardingState> {
   const r = raw as Record<string, unknown>;
   const provider = r['provider'];
   const modelPrimary = r['modelPrimary'];
-  const modelFast = r['modelFast'];
   if (typeof provider !== 'string' || !isSupportedOnboardingProvider(provider)) {
     throw new CodesignError('provider must be a supported provider string', 'IPC_BAD_INPUT');
   }
   if (typeof modelPrimary !== 'string' || modelPrimary.trim().length === 0) {
     throw new CodesignError('modelPrimary must be a non-empty string', 'IPC_BAD_INPUT');
   }
-  if (typeof modelFast !== 'string' || modelFast.trim().length === 0) {
-    throw new CodesignError('modelFast must be a non-empty string', 'IPC_BAD_INPUT');
-  }
   const cfg = getCachedConfig();
   if (cfg === null) {
     throw new CodesignError('No configuration found', 'CONFIG_MISSING');
   }
   assertProviderHasStoredSecret(cfg, provider);
   const next: Config = {
     ...cfg,
+    version: 2,
     provider,
     modelPrimary,
-    modelFast,
   };
+  next.modelFast = undefined;
   await writeConfig(next);
   cachedConfig = next;
   return toState(cachedConfig);
@@ -348,35 +383,25 @@ export function registerOnboardingIpc(): void {
   });
 
   ipcMain.handle('onboarding:save-key', async (_e, raw: unknown): Promise<OnboardingState> => {
-    const input = parseSaveKey(raw);
-    const ciphertext = encryptSecret(input.apiKey);
-    const nextBaseUrls = { ...(cachedConfig?.baseUrls ?? {}) };
-    if (input.baseUrl !== undefined) {
-      nextBaseUrls[input.provider] = { baseUrl: input.baseUrl };
-    } else {
-      delete nextBaseUrls[input.provider];
-    }
-    const next: Config = {
-      version: 1,
-      provider: input.provider,
-      modelPrimary: input.modelPrimary,
-      modelFast: input.modelFast,
-      secrets: {
-        ...(cachedConfig?.secrets ?? {}),
-        [input.provider]: { ciphertext },
-      },
-      baseUrls: nextBaseUrls,
-    };
-    await writeConfig(next);
-    cachedConfig = next;
-    configLoaded = true;
-    return toState(cachedConfig);
+    // Onboarding always activates the provider it just saved — that's the
+    // whole point of the first-time flow. Delegated to the canonical handler
+    // so behavior matches Settings exactly.
+    return runSetProviderAndModels({ ...parseSaveKey(raw), setAsActive: true });
   });
 
   ipcMain.handle('onboarding:skip', async (): Promise<OnboardingState> => {
     return toState(cachedConfig);
   });
 
+  // ── Canonical config mutation (preferred entry point) ─────────────────────
+
+  ipcMain.handle(
+    'config:v1:set-provider-and-models',
+    async (_e, raw: unknown): Promise<OnboardingState> => {
+      return runSetProviderAndModels(parseSetProviderAndModels(raw));
+    },
+  );
+
   // ── Settings v1 channels ────────────────────────────────────────────────────
 
   ipcMain.handle('settings:v1:list-providers', (): ProviderRow[] => runListProviders());