From 784118e38306c38ad67445087ac75c7c7c0be974 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: Sun, 3 May 2026 15:25:50 +0800 Subject: [PATCH] fix: stabilize desktop rewind resume Rewinding a desktop session can race with a still-exiting CLI process, leaving late transcript entries after the trim point. Desktop placeholder sessions also need to preserve canonical launch metadata so a zero-message rewind can restart from the real workspace instead of reconstructing it from a lossy sanitized project directory name. Constraint: Rewind must work after trimming back to an empty transcript. Constraint: Session project directory names are sanitized and not reversible. Rejected: Add retries around CLI startup | would leave corrupted transcript state in place. Rejected: Delete desktop placeholder files before first CLI launch | drops the only reliable workDir metadata. Confidence: high Scope-risk: moderate Directive: Do not change rewind process stopping without rechecking late CLI output and transcript metadata preservation. Tested: Real DeepSeek deepseek-v4-pro rewind smoke, bun test src/server/__tests__/sessions.test.ts --timeout 30000, bun test src/server/__tests__/conversations.test.ts --timeout 30000, bun run check:server, bun run check:native, bun run quality:pr, git diff --check Not-tested: Native Windows desktop binary smoke for the original reporter environment. --- src/server/__tests__/conversations.test.ts | 3 +- src/server/__tests__/sessions.test.ts | 58 +++++++++++++++++++-- src/server/services/conversationService.ts | 22 ++++++-- src/server/services/sessionRewindService.ts | 4 +- src/server/services/sessionService.ts | 34 +++++++++--- 5 files changed, 101 insertions(+), 20 deletions(-) diff --git a/src/server/__tests__/conversations.test.ts b/src/server/__tests__/conversations.test.ts index 1a07b691..6830f5d2 100644 --- a/src/server/__tests__/conversations.test.ts +++ b/src/server/__tests__/conversations.test.ts @@ -1024,6 +1024,7 @@ describe('WebSocket Chat Integration', () => { 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 canonicalWorkDir = await fs.realpath(workDir) const createRes = await fetch(`${baseUrl}/api/sessions`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -1042,7 +1043,7 @@ describe('WebSocket Chat Integration', () => { }) expect(error?.message).toContain('Desktop service diagnostics:') expect(error?.message).toContain(`sessionId: ${sessionId}`) - expect(error?.message).toContain(`workDir: ${workDir}`) + expect(error?.message).toContain(`workDir: ${canonicalWorkDir}`) expect(error?.message).toContain('runtimeOverride: (none)') expect(error?.message).toContain('activeProviderId:') expect(error?.message).toContain('configuredProviders:') diff --git a/src/server/__tests__/sessions.test.ts b/src/server/__tests__/sessions.test.ts index 040009ba..af2fe26d 100644 --- a/src/server/__tests__/sessions.test.ts +++ b/src/server/__tests__/sessions.test.ts @@ -847,7 +847,8 @@ describe('SessionService', () => { ) // Verify the file was created - const sanitized = sanitizePath(workDir) + const canonicalWorkDir = await fs.realpath(workDir) + const sanitized = sanitizePath(canonicalWorkDir) const filePath = path.join(tmpDir, 'projects', sanitized, `${sessionId}.jsonl`) const stat = await fs.stat(filePath) expect(stat.isFile()).toBe(true) @@ -979,11 +980,12 @@ describe('SessionService', () => { }) it('should detect placeholder launch info for desktop-created sessions', async () => { - const { sessionId } = await service.createSession(os.tmpdir()) + const workDir = await fs.realpath(os.tmpdir()) + const { sessionId } = await service.createSession(workDir) const launchInfo = await service.getSessionLaunchInfo(sessionId) expect(launchInfo).not.toBeNull() - expect(launchInfo!.workDir).toBe(os.tmpdir()) + expect(launchInfo!.workDir).toBe(workDir) expect(launchInfo!.transcriptMessageCount).toBe(0) expect(launchInfo!.customTitle).toBeNull() }) @@ -1225,7 +1227,7 @@ describe('Sessions API', () => { isGitRepo: boolean } expect(statusBody.state).toBe('ok') - expect(statusBody.workDir).toBe(workDir) + expect(statusBody.workDir).toBe(await fs.realpath(workDir)) expect(statusBody.isGitRepo).toBe(true) expect(statusBody.changedFiles).toEqual( expect.arrayContaining([ @@ -1706,6 +1708,54 @@ describe('Sessions API', () => { ]) }) + it('trimSessionMessagesFrom should remove orphan transcript entries beyond the rewind point', async () => { + const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + const firstUserId = crypto.randomUUID() + const firstAssistantId = crypto.randomUUID() + const secondUserId = crypto.randomUUID() + const secondAssistantId = crypto.randomUUID() + + const filePath = await writeSessionFile('-tmp-api-rewind-orphans', sessionId, [ + makeSnapshotEntry(), + makeSessionMetaEntry('/tmp/project-with-hyphen'), + { + ...makeUserEntry('first prompt', firstUserId), + sessionId, + }, + { + ...makeAssistantEntry('first reply', firstUserId), + uuid: firstAssistantId, + }, + { + ...makeUserEntry('second prompt', secondUserId), + parentUuid: firstAssistantId, + sessionId, + }, + { + ...makeAssistantEntry('second reply', secondUserId), + uuid: secondAssistantId, + }, + { + ...makeAssistantEntry('late stale reply', secondUserId), + uuid: crypto.randomUUID(), + }, + ]) + + const result = await service.trimSessionMessagesFrom(sessionId, firstUserId) + expect(result.removedMessageIds).toContain(firstUserId) + expect(result.removedMessageIds).toContain(secondUserId) + + const raw = await fs.readFile(filePath, 'utf-8') + expect(raw).toContain('"type":"session-meta"') + expect(raw).not.toContain('late stale reply') + expect(await service.getSessionMessages(sessionId)).toEqual([]) + + const launchInfo = await service.getSessionLaunchInfo(sessionId) + expect(launchInfo).not.toBeNull() + expect(launchInfo!.workDir).toBe('/tmp/project-with-hyphen') + expect(launchInfo!.transcriptMessageCount).toBe(0) + }) + it('POST /api/sessions/:id/rewind should target the selected message id instead of a shifted visible index', async () => { const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-ffffffffffff' const firstUserId = crypto.randomUUID() diff --git a/src/server/services/conversationService.ts b/src/server/services/conversationService.ts index f7a20640..e8db23e3 100644 --- a/src/server/services/conversationService.ts +++ b/src/server/services/conversationService.ts @@ -133,10 +133,6 @@ export class ConversationService { ) } - if (shouldReplacePlaceholder) { - await sessionService.deleteSessionFile(sessionId) - } - if (!fs.existsSync(workDir) || !fs.statSync(workDir).isDirectory()) { throw new ConversationStartupError( `Working directory does not exist or is not a directory: ${workDir}`, @@ -144,6 +140,10 @@ export class ConversationService { ) } + if (shouldReplacePlaceholder) { + await sessionService.clearSessionTranscript(sessionId, workDir) + } + const args = this.buildSessionCliArgs( sessionId, sdkUrl, @@ -540,6 +540,20 @@ export class ConversationService { } } + async stopSessionAndWait(sessionId: string, timeoutMs = 2_000): Promise { + const session = this.sessions.get(sessionId) + if (!session) return + + this.sessions.delete(sessionId) + session.proc.kill() + + await Promise.race([ + session.proc.exited.catch(() => undefined), + new Promise((resolve) => setTimeout(resolve, timeoutMs)), + ]) + await this.waitForProcessOutputDrain(session, timeoutMs) + } + markSessionDeleted(sessionId: string): void { this.deletedSessions.add(sessionId) this.stopSession(sessionId) diff --git a/src/server/services/sessionRewindService.ts b/src/server/services/sessionRewindService.ts index 91cfbc6b..323ef386 100644 --- a/src/server/services/sessionRewindService.ts +++ b/src/server/services/sessionRewindService.ts @@ -1096,9 +1096,7 @@ export async function executeSessionRewind( target.targetUserMessageId, ) - if (conversationService.hasSession(sessionId)) { - conversationService.stopSession(sessionId) - } + await conversationService.stopSessionAndWait(sessionId) if (preview.available && snapshots) { const targetSnapshot = findTargetSnapshot(snapshots, target.targetUserMessageId) diff --git a/src/server/services/sessionService.ts b/src/server/services/sessionService.ts index 292bbf93..9e9f916d 100644 --- a/src/server/services/sessionService.ts +++ b/src/server/services/sessionService.ts @@ -1197,18 +1197,20 @@ export class SessionService { // expand relative paths — in bundled sidecar mode the server's cwd is // typically '/'. Callers (IM adapters) already send absolute realPath, // but we log here so cwd regressions are caught early. - const absWorkDir = path.resolve(resolvedWorkDir) + const resolvedPath = path.resolve(resolvedWorkDir) + let absWorkDir: string + try { + absWorkDir = await fs.realpath(resolvedPath) + } catch { + throw ApiError.badRequest(`Working directory does not exist: ${resolvedPath}`) + } console.log( `[SessionService] createSession: requested workDir=${JSON.stringify( workDir, )}, resolved=${absWorkDir} (process.cwd()=${process.cwd()})`, ) let stat - try { - stat = await fs.stat(absWorkDir) - } catch { - throw ApiError.badRequest(`Working directory does not exist: ${absWorkDir}`) - } + stat = await fs.stat(absWorkDir) if (!stat.isDirectory()) { throw ApiError.badRequest(`Working directory is not a directory: ${absWorkDir}`) } @@ -1378,7 +1380,8 @@ export class SessionService { async clearSessionTranscript(sessionId: string, fallbackWorkDir?: string): Promise { let found = await this.findSessionFile(sessionId) if (!found && fallbackWorkDir) { - const absWorkDir = path.resolve(fallbackWorkDir) + const resolvedPath = path.resolve(fallbackWorkDir) + const absWorkDir = await fs.realpath(resolvedPath).catch(() => resolvedPath) const dirPath = path.join(this.getProjectsDir(), this.sanitizePath(absWorkDir)) await fs.mkdir(dirPath, { recursive: true }) found = { @@ -1462,6 +1465,11 @@ export class SessionService { const removedMessageIds = activeMessages .slice(startIndex) .map((message) => message.id) + const remainingMessageIds = new Set( + activeMessages + .slice(0, startIndex) + .map((message) => message.id), + ) if (removedMessageIds.length === 0) { return { removedCount: 0, removedMessageIds: [] } @@ -1469,7 +1477,17 @@ export class SessionService { const removedIds = new Set(removedMessageIds) const filteredEntries = entries.filter( - (entry) => !(typeof entry.uuid === 'string' && removedIds.has(entry.uuid)), + (entry) => { + if (typeof entry.uuid !== 'string') return true + if (removedIds.has(entry.uuid)) return false + if ( + entry.message?.role && + (entry.type === 'user' || entry.type === 'assistant' || entry.type === 'system') + ) { + return remainingMessageIds.has(entry.uuid) + } + return true + }, ) const content =