fix(server): force CLI restart when provider config edits land mid-session (#56)

Symptom: after editing a provider's baseUrl / apiKey / apiFormat / model
mapping in the desktop UI, an already-running chat session keeps using
its spawn-time env snapshot. Most visible failure mode: rename or remove
a model id and the next user turn upstreams the stale literal, getting

  错误: There's an issue with the selected model (mimo-v2.5-pro). It
  may not exist or you may not have access to it. Run --model to pick
  a different model.

verbatim from the provider's 404 wrapped in getAssistantMessageFromError
(src/services/api/errors.ts).

Cause: handleSetRuntimeConfig short-circuits when the
(providerId, modelId, effort, thinkingEnabled) tuple is unchanged.
The desktop fan-out re-emits set_runtime_config on every provider
update, but for non-modelId edits the tuple is identical so nothing
restarts. The CLI never re-reads providers.json — its
ANTHROPIC_BASE_URL / ANTHROPIC_AUTH_TOKEN / ANTHROPIC_MODEL env is
captured at spawn and immutable until the next respawn.

Fix:

1) Add a monotonic `revision` counter to SavedProvider, bumped in
   ProviderService.updateProvider on every save (src/server/types/
   provider.ts, src/server/services/providerService.ts).

2) Capture the revision into RuntimeOverride.providerRevision in
   handleSetRuntimeConfig, and extend the short-circuit to compare it
   (src/server/ws/handler.ts). When the user updates a provider, the
   bumped revision flows into the next set_runtime_config, the
   short-circuit fails, and scheduleRestartSessionWithRuntimeConfig
   respawns the CLI with fresh env from buildProviderManagedEnv.

3) Add a stale-modelId guard in getRuntimeSettings: when the
   persisted runtime modelId is no longer present in any of the
   active provider's four model slots, fall back to
   provider.models.main rather than letting --model pass an unknown
   id to the upstream. Mirrors the existing stale-providerId guard.

4) Extract the override equality into runtimeOverridesMatch (pure)
   and unit-lock it.

Tested:
  - bun test src/server/__tests__/runtime-override-match.test.ts → 10/10
    (covers undefined prev, all-fields-match, providerRevision
    differs forces restart, absent-revision treated as 0, modelId /
    providerId / effort / thinkingEnabled diffs)
  - bun test src/server/__tests__/providers.test.ts → 32/32, includes
    new "bumps the revision counter monotonically" case
  - bun test src/server/__tests__/conversations.test.ts → 160/160
    pass when run individually; the 3 deferred-restart e2e cases are
    flake-prone in batch (pre-existing, not introduced here)
  - cd desktop && bun run lint → tsc --noEmit clean

Confidence: high
Scope-risk: narrow

Co-authored-by: 你的姓名 <you@example.com>

src/server/__tests__/providers.test.ts

@@ -478,6 +478,24 @@ describe('ProviderService', () => {
       expect(updated.apiKey).toBe('sk-test-key-123')
     })
 
+    test('bumps the revision counter monotonically on every update so runtime overrides can detect stale snapshots', async () => {
+      const svc = new ProviderService()
+      const added = await svc.addProvider(sampleInput())
+      // New providers don't pre-populate revision (treated as 0 by readers).
+      expect(added.revision).toBeUndefined()
+
+      const r1 = await svc.updateProvider(added.id, { apiKey: 'rotated-once' })
+      expect(r1.revision).toBe(1)
+
+      const r2 = await svc.updateProvider(added.id, { baseUrl: 'https://new.example.com' })
+      expect(r2.revision).toBe(2)
+
+      // Edits that touch nothing useful still bump — the contract is
+      // "user clicked save, force any in-flight session to re-snapshot".
+      const r3 = await svc.updateProvider(added.id, { notes: 'note' })
+      expect(r3.revision).toBe(3)
+    })
+
     test('should throw 404 for non-existent provider', async () => {
       const svc = new ProviderService()
 

src/server/__tests__/runtime-override-match.test.ts

@@ -0,0 +1,84 @@
+/**
+ * Unit-locks the short-circuit equality used by `handleSetRuntimeConfig`
+ * to decide whether a `set_runtime_config` message should respawn the CLI.
+ *
+ * The producer-side bug we're guarding against: a user edits provider
+ * baseUrl / apiKey / apiFormat / model mapping. The desktop fan-out
+ * re-emits `set_runtime_config` for every idle session on that provider,
+ * but the (providerId, modelId, effort, thinkingEnabled) tuple is
+ * identical to the running CLI's. Without `providerRevision` in the
+ * comparison, the handler short-circuits and the running CLI keeps its
+ * spawn-time `ANTHROPIC_BASE_URL` / `ANTHROPIC_AUTH_TOKEN` / model env
+ * forever — resulting in "There's an issue with the selected model (...)"
+ * after a model rename.
+ */
+import { describe, expect, it } from 'bun:test'
+import { runtimeOverridesMatch, type RuntimeOverride } from '../ws/handler.js'
+
+const base: RuntimeOverride = {
+  providerId: 'p1',
+  modelId: 'mimo-v2.5-pro',
+  effort: 'medium',
+  thinkingEnabled: true,
+  providerRevision: 1,
+}
+
+describe('runtimeOverridesMatch', () => {
+  it('returns false when prev is undefined (first set_runtime_config)', () => {
+    expect(runtimeOverridesMatch(undefined, base)).toBe(false)
+  })
+
+  it('returns true when every field matches', () => {
+    expect(runtimeOverridesMatch({ ...base }, { ...base })).toBe(true)
+  })
+
+  it('forces a restart when providerRevision differs (PR-A regression)', () => {
+    // The whole point: same tuple, bumped revision after updateProvider.
+    expect(
+      runtimeOverridesMatch(
+        { ...base, providerRevision: 1 },
+        { ...base, providerRevision: 2 },
+      ),
+    ).toBe(false)
+  })
+
+  it('treats absent providerRevision as 0 on both sides', () => {
+    const { providerRevision: _a, ...prev } = base
+    const { providerRevision: _b, ...next } = base
+    expect(runtimeOverridesMatch(prev, next)).toBe(true)
+  })
+
+  it('treats absent providerRevision (legacy override loaded from disk) and revision=0 as equal', () => {
+    const { providerRevision: _a, ...prev } = base
+    expect(runtimeOverridesMatch(prev, { ...base, providerRevision: 0 })).toBe(true)
+  })
+
+  it('forces a restart when modelId differs', () => {
+    expect(
+      runtimeOverridesMatch(base, { ...base, modelId: 'sonnet-4.5' }),
+    ).toBe(false)
+  })
+
+  it('forces a restart when providerId differs', () => {
+    expect(
+      runtimeOverridesMatch(base, { ...base, providerId: 'p2' }),
+    ).toBe(false)
+  })
+
+  it('forces a restart when effort differs', () => {
+    expect(
+      runtimeOverridesMatch(base, { ...base, effort: 'high' }),
+    ).toBe(false)
+  })
+
+  it('forces a restart when thinkingEnabled flips', () => {
+    expect(
+      runtimeOverridesMatch(base, { ...base, thinkingEnabled: false }),
+    ).toBe(false)
+  })
+
+  it('treats null providerId on both sides as matching (no provider configured)', () => {
+    const noProvider: RuntimeOverride = { providerId: null, modelId: 'x' }
+    expect(runtimeOverridesMatch(noProvider, { ...noProvider })).toBe(true)
+  })
+})

src/server/services/providerService.ts

@@ -198,6 +198,12 @@ export class ProviderService {
     delete updated.thinkingIncompatible
     delete updated.thinkingIncompatibleReason
 
+    // Bump the revision counter so any in-flight `runtimeOverride` snapshot
+    // for this provider is recognised as stale by handleSetRuntimeConfig
+    // and forces a CLI restart even when the (providerId, modelId, effort)
+    // tuple is unchanged. See SavedProviderSchema.revision docstring.
+    updated.revision = (existing.revision ?? 0) + 1
+
     index.providers[idx] = updated
     await this.writeIndex(index)
 

src/server/types/provider.ts

@@ -75,6 +75,23 @@ export const SavedProviderSchema = z.object({
    * providers.json file.
    */
   thinkingIncompatibleReason: z.string().max(500).optional(),
+  /**
+   * Monotonically-increasing counter bumped on every {@link updateProvider}.
+   * The runtime layer captures this value alongside the per-session
+   * `runtimeOverride` at session start, and the next `set_runtime_config`
+   * compares revisions to decide whether to force a CLI restart even when
+   * the (providerId, modelId, effort, thinkingEnabled) tuple is unchanged.
+   *
+   * This catches "user changed baseUrl / apiKey / apiFormat / model mapping
+   * but the override tuple still matches" — without it, the CLI keeps its
+   * spawn-time `ANTHROPIC_BASE_URL` / `ANTHROPIC_AUTH_TOKEN` env until the
+   * subprocess is killed for some other reason, which makes provider
+   * config edits feel "stuck".
+   *
+   * Optional for backwards compatibility with providers.json files written
+   * before this field existed; absent reads as 0.
+   */
+  revision: z.number().int().nonnegative().optional(),
 })
 
 export const ProvidersIndexSchema = z.object({

src/server/ws/handler.ts

@@ -81,11 +81,23 @@ const sessionTitleState = new Map<string, {
   generationSeq: number
 }>()
 
-type RuntimeOverride = {
+export type RuntimeOverride = {
   providerId: string | null
   modelId: string
   effort?: string
   thinkingEnabled?: boolean
+  /**
+   * Snapshot of the provider's `revision` at the moment this override was
+   * captured. Compared on the next `set_runtime_config` to detect that the
+   * underlying provider config (baseUrl / apiKey / apiFormat / model
+   * mapping) has changed even when the override tuple is the same — which
+   * would otherwise silently keep the running CLI on a stale env snapshot.
+   *
+   * Absent for runtime overrides loaded from older session JSONL metadata
+   * that pre-date this field; treated as 0 so any subsequent
+   * provider.update bumps it past the captured value.
+   */
+  providerRevision?: number
 }
 
 type ActiveUserTurnState = {
@@ -894,20 +906,18 @@ async function handleSetRuntimeConfig(
   const thinkingEnabled =
     typeof message.thinkingEnabled === 'boolean' ? message.thinkingEnabled : undefined
 
-  const nextOverride = {
-    providerId: message.providerId ?? null,
+  const providerId = message.providerId ?? null
+  const providerRevision = await resolveProviderRevision(providerId)
+
+  const nextOverride: RuntimeOverride = {
+    providerId,
     modelId,
     ...(effortLevel ? { effort: effortLevel } : {}),
     ...(thinkingEnabled !== undefined ? { thinkingEnabled } : {}),
+    ...(providerRevision > 0 ? { providerRevision } : {}),
   }
   const prevOverride = runtimeOverrides.get(sessionId)
-  if (
-    prevOverride &&
-    prevOverride.providerId === nextOverride.providerId &&
-    prevOverride.modelId === nextOverride.modelId &&
-    prevOverride.effort === nextOverride.effort &&
-    prevOverride.thinkingEnabled === nextOverride.thinkingEnabled
-  ) {
+  if (runtimeOverridesMatch(prevOverride, nextOverride)) {
     return
   }
 
@@ -945,10 +955,7 @@ async function handleSetRuntimeConfig(
       await pendingStartup.catch(() => undefined)
       const currentOverride = runtimeOverrides.get(sessionId)
       if (
-        currentOverride?.providerId !== nextOverride.providerId ||
-        currentOverride.modelId !== nextOverride.modelId ||
-        currentOverride.effort !== nextOverride.effort ||
-        currentOverride.thinkingEnabled !== nextOverride.thinkingEnabled ||
+        !runtimeOverridesMatch(currentOverride, nextOverride) ||
         !conversationService.hasSession(sessionId)
       ) {
         return
@@ -2576,6 +2583,52 @@ function isKnownRuntimeProviderId(
   )
 }
 
+/**
+ * Look up the current revision of a saved provider for use in
+ * {@link RuntimeOverride.providerRevision}. Returns 0 for the OpenAI Official
+ * built-in (it has no provider config to mutate), 0 for unknown / null ids,
+ * and never throws — a stale providerId is handled by the existing
+ * stale-providerId guard in {@link getRuntimeSettings}.
+ */
+async function resolveProviderRevision(
+  providerId: string | null,
+): Promise<number> {
+  if (!providerId) return 0
+  if (isOpenAIOfficialProviderId(providerId)) return 0
+  try {
+    const provider = await providerService.getProvider(providerId)
+    return provider.revision ?? 0
+  } catch {
+    return 0
+  }
+}
+
+/**
+ * Pure equality check used by `handleSetRuntimeConfig`'s short-circuit.
+ * Exported for unit testing — kept here (not in a separate module) because
+ * it depends on the locally-defined RuntimeOverride shape.
+ *
+ * Returns true when the two overrides describe the same effective CLI
+ * runtime — i.e. respawning the CLI would be a no-op. Critically this
+ * includes `providerRevision`: when the user edits provider config without
+ * touching modelId/effort, the tuple appears unchanged but the spawn-time
+ * env (baseUrl / apiKey / model mapping) is stale, so we must consider
+ * that a difference and force a restart.
+ */
+export function runtimeOverridesMatch(
+  prev: RuntimeOverride | undefined,
+  next: RuntimeOverride,
+): boolean {
+  if (!prev) return false
+  return (
+    prev.providerId === next.providerId &&
+    prev.modelId === next.modelId &&
+    prev.effort === next.effort &&
+    prev.thinkingEnabled === next.thinkingEnabled &&
+    (prev.providerRevision ?? 0) === (next.providerRevision ?? 0)
+  )
+}
+
 async function getRuntimeSettings(sessionId?: string): Promise<RuntimeSettings> {
   const coordinatorMode = sessionId ? coordinatorModeSessions.has(sessionId) : false
   const soloPipelineMode = sessionId ? soloPipelineModeSessions.has(sessionId) : false
@@ -2606,6 +2659,7 @@ async function getRuntimeSettings(sessionId?: string): Promise<RuntimeSettings>
     ? runtimeOverrides.get(sessionId) ?? persistedRuntimeOverride
     : undefined
   if (runtimeOverride) {
+    let resolvedModelId = runtimeOverride.modelId
     if (typeof runtimeOverride.providerId === 'string') {
       const { providers } = await providerService.listProviders()
       const providerExists = isKnownRuntimeProviderId(runtimeOverride.providerId, providers)
@@ -2623,6 +2677,36 @@ async function getRuntimeSettings(sessionId?: string): Promise<RuntimeSettings>
           ...(handoffSystemPrompt ? { handoffSystemPrompt } : {}),
         }
       }
+
+      // Stale-modelId guard: when the persisted runtime modelId is no longer
+      // present in any of the active provider's four model slots
+      // (main / haiku / sonnet / opus), the upstream will return 404 and we
+      // surface "There's an issue with the selected model (...)" — which is
+      // exactly the cycle a user hits when they rename a model in Settings
+      // and resume an old session. Fall back to the provider's main model
+      // instead of letting `--model <unknown>` reach the wire.
+      // Skipped for the OpenAI Official built-in (no editable mapping).
+      if (!isOpenAIOfficialProviderId(runtimeOverride.providerId)) {
+        const provider = providers.find((p) => p.id === runtimeOverride.providerId)
+        if (provider) {
+          const knownModels = new Set(
+            [
+              provider.models.main,
+              provider.models.haiku,
+              provider.models.sonnet,
+              provider.models.opus,
+            ]
+              .map((value) => (typeof value === 'string' ? value.trim() : ''))
+              .filter(Boolean),
+          )
+          if (knownModels.size > 0 && !knownModels.has(runtimeOverride.modelId)) {
+            console.warn(
+              `[WS] Persisted runtime modelId '${runtimeOverride.modelId}' is no longer in provider ${provider.id}'s model map; falling back to ${provider.models.main}`,
+            )
+            resolvedModelId = provider.models.main
+          }
+        }
+      }
     }
 
     const userSettings = await settingsService.getUserSettings()
@@ -2630,7 +2714,7 @@ async function getRuntimeSettings(sessionId?: string): Promise<RuntimeSettings>
 
     return {
       permissionMode: sessionPermissionMode ?? await settingsService.getPermissionMode().catch(() => undefined),
-      model: runtimeOverride.modelId,
+      model: resolvedModelId,
       effort: runtimeOverride.effort,
       thinking,
       providerId: runtimeOverride.providerId,