mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-31 16:33:34 +08:00
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
This commit is contained in:
parent
8ef169d693
commit
dcf844b7e7
@ -859,6 +859,32 @@ describe('WebSocket Chat Integration', () => {
|
|||||||
expect(nextTurn.some((m) => m.type === 'message_complete')).toBe(true)
|
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 () => {
|
it('should prewarm the CLI before the first user turn and reuse that process', async () => {
|
||||||
const createRes = await fetch(`${baseUrl}/api/sessions`, {
|
const createRes = await fetch(`${baseUrl}/api/sessions`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
@ -1143,6 +1169,108 @@ describe('WebSocket Chat Integration', () => {
|
|||||||
}
|
}
|
||||||
}, 20_000)
|
}, 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<void>((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 () => {
|
it('should resume streaming to a reconnected client during an active turn', async () => {
|
||||||
await withMockStreamDelay(150, async () => {
|
await withMockStreamDelay(150, async () => {
|
||||||
const sessionId = `chat-reconnect-${crypto.randomUUID()}`
|
const sessionId = `chat-reconnect-${crypto.randomUUID()}`
|
||||||
|
|||||||
@ -61,6 +61,7 @@ const runtimeOverrides = new Map<string, {
|
|||||||
|
|
||||||
const runtimeTransitionPromises = new Map<string, Promise<void>>()
|
const runtimeTransitionPromises = new Map<string, Promise<void>>()
|
||||||
const sessionStartupPromises = new Map<string, Promise<void>>()
|
const sessionStartupPromises = new Map<string, Promise<void>>()
|
||||||
|
const lastResolvedStartupWorkDirs = new Map<string, string>()
|
||||||
const prewarmPendingSessions = new Set<string>()
|
const prewarmPendingSessions = new Set<string>()
|
||||||
const prewarmedSessions = new Set<string>()
|
const prewarmedSessions = new Set<string>()
|
||||||
const prewarmIdleTimers = new Map<string, ReturnType<typeof setTimeout>>()
|
const prewarmIdleTimers = new Map<string, ReturnType<typeof setTimeout>>()
|
||||||
@ -265,7 +266,7 @@ async function handleUserMessage(
|
|||||||
console.error(`[WS] CLI start failed for ${sessionId}: ${errMsg}`)
|
console.error(`[WS] CLI start failed for ${sessionId}: ${errMsg}`)
|
||||||
sendMessage(ws, {
|
sendMessage(ws, {
|
||||||
type: 'error',
|
type: 'error',
|
||||||
message: errMsg,
|
message: await buildSessionStartupDiagnosticMessage(sessionId, errMsg),
|
||||||
code,
|
code,
|
||||||
retryable:
|
retryable:
|
||||||
err instanceof ConversationStartupError ? err.retryable : false,
|
err instanceof ConversationStartupError ? err.retryable : false,
|
||||||
@ -511,7 +512,10 @@ async function restartSessionWithPermissionMode(
|
|||||||
console.error(`[WS] Failed to restart CLI for ${sessionId}: ${errMsg}`)
|
console.error(`[WS] Failed to restart CLI for ${sessionId}: ${errMsg}`)
|
||||||
sendMessage(ws, {
|
sendMessage(ws, {
|
||||||
type: 'error',
|
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',
|
code: 'CLI_RESTART_FAILED',
|
||||||
})
|
})
|
||||||
sendMessage(ws, { type: 'status', state: 'idle' })
|
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}`)
|
console.error(`[WS] Failed to restart CLI for ${sessionId} after runtime override: ${errMsg}`)
|
||||||
sendMessage(ws, {
|
sendMessage(ws, {
|
||||||
type: 'error',
|
type: 'error',
|
||||||
message: `Failed to switch provider/model: ${errMsg}`,
|
message: await buildSessionStartupDiagnosticMessage(
|
||||||
|
sessionId,
|
||||||
|
`Failed to switch provider/model: ${errMsg}`,
|
||||||
|
),
|
||||||
code: 'CLI_RESTART_FAILED',
|
code: 'CLI_RESTART_FAILED',
|
||||||
})
|
})
|
||||||
sendMessage(ws, { type: 'status', state: 'idle' })
|
sendMessage(ws, { type: 'status', state: 'idle' })
|
||||||
@ -661,6 +668,7 @@ function cleanupSessionRuntimeState(sessionId: string) {
|
|||||||
runtimeOverrides.delete(sessionId)
|
runtimeOverrides.delete(sessionId)
|
||||||
runtimeTransitionPromises.delete(sessionId)
|
runtimeTransitionPromises.delete(sessionId)
|
||||||
sessionStartupPromises.delete(sessionId)
|
sessionStartupPromises.delete(sessionId)
|
||||||
|
lastResolvedStartupWorkDirs.delete(sessionId)
|
||||||
clearPrewarmState(sessionId)
|
clearPrewarmState(sessionId)
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -758,6 +766,7 @@ async function ensureCliSessionStarted(
|
|||||||
|
|
||||||
const startup = (async () => {
|
const startup = (async () => {
|
||||||
const workDir = await resolveSessionWorkDir(sessionId)
|
const workDir = await resolveSessionWorkDir(sessionId)
|
||||||
|
lastResolvedStartupWorkDirs.set(sessionId, workDir)
|
||||||
const runtimeSettings = await getRuntimeSettings(sessionId)
|
const runtimeSettings = await getRuntimeSettings(sessionId)
|
||||||
const sdkUrl =
|
const sdkUrl =
|
||||||
`ws://${ws.data.serverHost}:${ws.data.serverPort}/sdk/${sessionId}` +
|
`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
|
permissionMode?: string
|
||||||
model?: string
|
model?: string
|
||||||
effort?: string
|
effort?: string
|
||||||
providerId?: string | null
|
providerId?: string | null
|
||||||
}> {
|
}
|
||||||
|
|
||||||
|
async function getRuntimeSettings(sessionId?: string): Promise<RuntimeSettings> {
|
||||||
const runtimeOverride = sessionId ? runtimeOverrides.get(sessionId) : undefined
|
const runtimeOverride = sessionId ? runtimeOverrides.get(sessionId) : undefined
|
||||||
if (runtimeOverride) {
|
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 userSettings = await settingsService.getUserSettings()
|
||||||
const effort =
|
const effort =
|
||||||
typeof userSettings.effort === 'string' && userSettings.effort.trim()
|
typeof userSettings.effort === 'string' && userSettings.effort.trim()
|
||||||
@ -1245,10 +1268,21 @@ async function getRuntimeSettings(sessionId?: string): Promise<{
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return getDefaultRuntimeSettings()
|
||||||
|
}
|
||||||
|
|
||||||
|
async function getDefaultRuntimeSettings(): Promise<RuntimeSettings> {
|
||||||
// Check if a custom provider is active
|
// 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 userSettings = await settingsService.getUserSettings()
|
||||||
const providerSettings = activeId
|
const providerSettings = resolvedActiveId
|
||||||
? await providerService.getManagedSettings()
|
? await providerService.getManagedSettings()
|
||||||
: undefined
|
: undefined
|
||||||
const modelSettings = providerSettings ?? userSettings
|
const modelSettings = providerSettings ?? userSettings
|
||||||
@ -1262,7 +1296,7 @@ async function getRuntimeSettings(sessionId?: string): Promise<{
|
|||||||
: undefined
|
: undefined
|
||||||
|
|
||||||
let model: string | undefined
|
let model: string | undefined
|
||||||
if (activeId) {
|
if (resolvedActiveId) {
|
||||||
// Provider is active — only consult provider-managed cc-haha settings.
|
// Provider is active — only consult provider-managed cc-haha settings.
|
||||||
// Global ~/.claude/settings.json model values must not bleed into provider mode.
|
// Global ~/.claude/settings.json model values must not bleed into provider mode.
|
||||||
const baseModel =
|
const baseModel =
|
||||||
@ -1286,10 +1320,58 @@ async function getRuntimeSettings(sessionId?: string): Promise<{
|
|||||||
permissionMode: await settingsService.getPermissionMode().catch(() => undefined),
|
permissionMode: await settingsService.getPermissionMode().catch(() => undefined),
|
||||||
model,
|
model,
|
||||||
effort,
|
effort,
|
||||||
providerId: activeId,
|
providerId: resolvedActiveId,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function buildSessionStartupDiagnosticMessage(
|
||||||
|
sessionId: string,
|
||||||
|
cause: string,
|
||||||
|
): Promise<string> {
|
||||||
|
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(
|
function enqueueRuntimeTransition(
|
||||||
sessionId: string,
|
sessionId: string,
|
||||||
transition: () => Promise<void>,
|
transition: () => Promise<void>,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user