From dcf844b7e77fda35aa934fd142b1296e4d959b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Fri, 1 May 2026 00:10:00 +0800 Subject: [PATCH] fix: keep old desktop sessions usable after provider changes Desktop sessions can carry persisted runtime provider selections long after the provider index changes. The server now validates those session-level and active provider ids before launching the CLI, falls back to a valid default path, and includes non-secret launch diagnostics in startup failure messages so future issue reports expose the server-side context. Constraint: Desktop users often report only the visible chat error, not sidecar console logs Rejected: Frontend-only cleanup of stale localStorage selections | does not protect API or reconnect paths and loses server-side evidence Confidence: high Scope-risk: narrow Directive: Do not pass session-scoped provider ids into CLI startup without validating they still exist in providers.json Tested: bun test src/server/__tests__/conversations.test.ts Tested: bun test src/server/__tests__/providers.test.ts Tested: bun test src/server/__tests__/conversation-service.test.ts --- src/server/__tests__/conversations.test.ts | 128 +++++++++++++++++++++ src/server/ws/handler.ts | 100 ++++++++++++++-- 2 files changed, 219 insertions(+), 9 deletions(-) diff --git a/src/server/__tests__/conversations.test.ts b/src/server/__tests__/conversations.test.ts index 48505ba6..af22e877 100644 --- a/src/server/__tests__/conversations.test.ts +++ b/src/server/__tests__/conversations.test.ts @@ -859,6 +859,32 @@ describe('WebSocket Chat Integration', () => { expect(nextTurn.some((m) => m.type === 'message_complete')).toBe(true) }) + it('should include desktop service diagnostics when CLI startup fails', async () => { + const workDir = await fs.mkdtemp(path.join(os.tmpdir(), 'claude-startup-missing-workdir-')) + const createRes = await fetch(`${baseUrl}/api/sessions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workDir }), + }) + expect(createRes.status).toBe(201) + const { sessionId } = await createRes.json() as { sessionId: string } + + await fs.rm(workDir, { recursive: true, force: true }) + + const messages = await runTurn(sessionId, 'trigger startup diagnostics', true) + const error = messages.find((msg) => msg.type === 'error') + + expect(error).toMatchObject({ + code: 'WORKDIR_INVALID', + }) + expect(error?.message).toContain('Desktop service diagnostics:') + expect(error?.message).toContain(`sessionId: ${sessionId}`) + expect(error?.message).toContain(`workDir: ${workDir}`) + expect(error?.message).toContain('runtimeOverride: (none)') + expect(error?.message).toContain('activeProviderId:') + expect(error?.message).toContain('configuredProviders:') + }) + it('should prewarm the CLI before the first user turn and reuse that process', async () => { const createRes = await fetch(`${baseUrl}/api/sessions`, { method: 'POST', @@ -1143,6 +1169,108 @@ describe('WebSocket Chat Integration', () => { } }, 20_000) + it('should ignore stale persisted runtime provider ids when resuming old sessions', async () => { + const providerService = new ProviderService() + const activeProvider = await providerService.addProvider({ + presetId: 'custom', + name: 'Current Valid Provider', + apiKey: 'key-current-valid', + baseUrl: 'http://127.0.0.1:1/anthropic', + apiFormat: 'anthropic', + models: { + main: 'current-main', + haiku: 'current-haiku', + sonnet: 'current-sonnet', + opus: 'current-opus', + }, + }) + await providerService.activateProvider(activeProvider.id) + + const createRes = await fetch(`${baseUrl}/api/sessions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workDir: process.cwd() }), + }) + expect(createRes.status).toBe(201) + const { sessionId } = await createRes.json() as { sessionId: string } + + const staleProviderId = crypto.randomUUID() + const originalStartSession = conversationService.startSession.bind(conversationService) + const startCalls: Array<{ + sessionId: string + options: { permissionMode?: string; model?: string; effort?: string; providerId?: string | null } | undefined + }> = [] + + conversationService.startSession = (async function patchedStartSession( + sid: string, + workDir: string, + sdkUrl: string, + options?: { permissionMode?: string; model?: string; effort?: string; providerId?: string | null }, + ) { + startCalls.push({ sessionId: sid, options }) + return originalStartSession(sid, workDir, sdkUrl, options) + }) as typeof conversationService.startSession + + const ws = new WebSocket(`${wsUrl}/ws/${sessionId}`) + const messages: any[] = [] + try { + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + ws.close() + reject(new Error(`Timed out waiting for stale runtime resume for session ${sessionId}`)) + }, 10_000) + + ws.onmessage = (event) => { + const msg = JSON.parse(event.data as string) + messages.push(msg) + + if (msg.type === 'connected') { + ws.send(JSON.stringify({ + type: 'set_runtime_config', + providerId: staleProviderId, + modelId: 'stale-model', + })) + ws.send(JSON.stringify({ type: 'user_message', content: 'resume old session' })) + return + } + + if (msg.type === 'error') { + clearTimeout(timeout) + ws.close() + reject(new Error(msg.message)) + return + } + + if (msg.type === 'message_complete') { + clearTimeout(timeout) + ws.close() + resolve() + } + } + + ws.onerror = () => { + clearTimeout(timeout) + reject(new Error(`WebSocket error for stale runtime resume session ${sessionId}`)) + } + }) + + expect(startCalls).toHaveLength(1) + expect(startCalls[0]).toMatchObject({ + sessionId, + options: { + providerId: activeProvider.id, + }, + }) + expect(startCalls[0]?.options?.model).not.toBe('stale-model') + expect(messages.some((msg) => msg.type === 'message_complete')).toBe(true) + } finally { + ws.close() + conversationService.startSession = originalStartSession + conversationService.stopSession(sessionId) + await providerService.activateOfficial() + } + }, 20_000) + it('should resume streaming to a reconnected client during an active turn', async () => { await withMockStreamDelay(150, async () => { const sessionId = `chat-reconnect-${crypto.randomUUID()}` diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 5d35c744..1c5ce276 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -61,6 +61,7 @@ const runtimeOverrides = new Map>() const sessionStartupPromises = new Map>() +const lastResolvedStartupWorkDirs = new Map() const prewarmPendingSessions = new Set() const prewarmedSessions = new Set() const prewarmIdleTimers = new Map>() @@ -265,7 +266,7 @@ async function handleUserMessage( console.error(`[WS] CLI start failed for ${sessionId}: ${errMsg}`) sendMessage(ws, { type: 'error', - message: errMsg, + message: await buildSessionStartupDiagnosticMessage(sessionId, errMsg), code, retryable: err instanceof ConversationStartupError ? err.retryable : false, @@ -511,7 +512,10 @@ async function restartSessionWithPermissionMode( console.error(`[WS] Failed to restart CLI for ${sessionId}: ${errMsg}`) sendMessage(ws, { type: 'error', - message: `Failed to restart session with new permission mode: ${errMsg}`, + message: await buildSessionStartupDiagnosticMessage( + sessionId, + `Failed to restart session with new permission mode: ${errMsg}`, + ), code: 'CLI_RESTART_FAILED', }) sendMessage(ws, { type: 'status', state: 'idle' }) @@ -545,7 +549,10 @@ async function restartSessionWithRuntimeConfig( console.error(`[WS] Failed to restart CLI for ${sessionId} after runtime override: ${errMsg}`) sendMessage(ws, { type: 'error', - message: `Failed to switch provider/model: ${errMsg}`, + message: await buildSessionStartupDiagnosticMessage( + sessionId, + `Failed to switch provider/model: ${errMsg}`, + ), code: 'CLI_RESTART_FAILED', }) sendMessage(ws, { type: 'status', state: 'idle' }) @@ -661,6 +668,7 @@ function cleanupSessionRuntimeState(sessionId: string) { runtimeOverrides.delete(sessionId) runtimeTransitionPromises.delete(sessionId) sessionStartupPromises.delete(sessionId) + lastResolvedStartupWorkDirs.delete(sessionId) clearPrewarmState(sessionId) } @@ -758,6 +766,7 @@ async function ensureCliSessionStarted( const startup = (async () => { const workDir = await resolveSessionWorkDir(sessionId) + lastResolvedStartupWorkDirs.set(sessionId, workDir) const runtimeSettings = await getRuntimeSettings(sessionId) const sdkUrl = `ws://${ws.data.serverHost}:${ws.data.serverPort}/sdk/${sessionId}` + @@ -1223,14 +1232,28 @@ function rebindSessionOutput( }) } -async function getRuntimeSettings(sessionId?: string): Promise<{ +type RuntimeSettings = { permissionMode?: string model?: string effort?: string providerId?: string | null -}> { +} + +async function getRuntimeSettings(sessionId?: string): Promise { const runtimeOverride = sessionId ? runtimeOverrides.get(sessionId) : undefined if (runtimeOverride) { + if (typeof runtimeOverride.providerId === 'string') { + const { providers } = await providerService.listProviders() + const providerExists = providers.some((provider) => provider.id === runtimeOverride.providerId) + if (!providerExists) { + console.warn( + `[WS] Ignoring stale runtime provider id for ${sessionId}: ${runtimeOverride.providerId}`, + ) + runtimeOverrides.delete(sessionId!) + return getDefaultRuntimeSettings() + } + } + const userSettings = await settingsService.getUserSettings() const effort = typeof userSettings.effort === 'string' && userSettings.effort.trim() @@ -1245,10 +1268,21 @@ async function getRuntimeSettings(sessionId?: string): Promise<{ } } + return getDefaultRuntimeSettings() +} + +async function getDefaultRuntimeSettings(): Promise { // Check if a custom provider is active - const { activeId } = await providerService.listProviders() + const { providers, activeId } = await providerService.listProviders() + let resolvedActiveId = activeId + if (activeId && !providers.some((provider) => provider.id === activeId)) { + console.warn(`[WS] Active provider id is stale, falling back to official provider: ${activeId}`) + resolvedActiveId = null + await providerService.activateOfficial() + } + const userSettings = await settingsService.getUserSettings() - const providerSettings = activeId + const providerSettings = resolvedActiveId ? await providerService.getManagedSettings() : undefined const modelSettings = providerSettings ?? userSettings @@ -1262,7 +1296,7 @@ async function getRuntimeSettings(sessionId?: string): Promise<{ : undefined let model: string | undefined - if (activeId) { + if (resolvedActiveId) { // Provider is active — only consult provider-managed cc-haha settings. // Global ~/.claude/settings.json model values must not bleed into provider mode. const baseModel = @@ -1286,10 +1320,58 @@ async function getRuntimeSettings(sessionId?: string): Promise<{ permissionMode: await settingsService.getPermissionMode().catch(() => undefined), model, effort, - providerId: activeId, + providerId: resolvedActiveId, } } +async function buildSessionStartupDiagnosticMessage( + sessionId: string, + cause: string, +): Promise { + const lines = [ + cause, + '', + 'Desktop service diagnostics:', + `- sessionId: ${sessionId}`, + ] + + try { + const recentWorkDir = lastResolvedStartupWorkDirs.get(sessionId) + const workDir = + recentWorkDir || + conversationService.getSessionWorkDir(sessionId) || + await sessionService.getSessionWorkDir(sessionId) + lines.push(`- workDir: ${workDir ?? '(unknown)'}`) + } catch (err) { + lines.push(`- workDir: failed to resolve (${err instanceof Error ? err.message : String(err)})`) + } + + const runtimeOverride = runtimeOverrides.get(sessionId) + if (runtimeOverride) { + lines.push(`- runtimeOverride.providerId: ${runtimeOverride.providerId ?? '(official)'}`) + lines.push(`- runtimeOverride.modelId: ${runtimeOverride.modelId}`) + } else { + lines.push('- runtimeOverride: (none)') + } + + try { + const { providers, activeId } = await providerService.listProviders() + lines.push(`- activeProviderId: ${activeId ?? '(official)'}`) + lines.push(`- configuredProviders: ${providers.length}`) + if (providers.length > 0) { + lines.push( + `- providerIndex: ${providers + .map((provider) => `${provider.name} (${provider.id})`) + .join(', ')}`, + ) + } + } catch (err) { + lines.push(`- providers: failed to read (${err instanceof Error ? err.message : String(err)})`) + } + + return lines.join('\n') +} + function enqueueRuntimeTransition( sessionId: string, transition: () => Promise,