Skip to content

Commit c60d378

Browse files
NanmiCoderNanmiCoder
committed
fix(server): close release QA regressions (#1019, #1047, #1051)
1 parent 0cb612c commit c60d378

9 files changed

Lines changed: 509 additions & 30 deletions

‎src/server/__tests__/sessions.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5523,6 +5523,59 @@ describe('Sessions API', () => {
55235523
expect(body.checkpoints[0]!.code.deletions).toBe(0)
55245524
})
55255525

5526+
it('GET /api/sessions/:id/turn-checkpoints should ignore rejected transcript tool changes', async () => {
5527+
const sessionId = '99999999-bbbb-cccc-dddd-000000000004'
5528+
const workDir = path.join(tmpDir, 'transcript-rejected-session')
5529+
const userId = crypto.randomUUID()
5530+
const toolUseId = 'Write:rejected'
5531+
await fs.mkdir(workDir, { recursive: true })
5532+
5533+
await writeSessionFile('-tmp-transcript-rejected-session', sessionId, [
5534+
makeSessionMetaEntry(workDir),
5535+
{
5536+
...makeUserEntry('write a denied file', userId),
5537+
cwd: workDir,
5538+
sessionId,
5539+
},
5540+
makeAssistantToolUseEntry([{
5541+
id: toolUseId,
5542+
name: 'Write',
5543+
input: {
5544+
file_path: path.join(workDir, 'permission-denial-test.txt'),
5545+
content: 'must not be written\n',
5546+
},
5547+
}], userId),
5548+
{
5549+
...makeToolResultUserEntry(
5550+
toolUseId,
5551+
'The user rejected this tool use.',
5552+
undefined,
5553+
undefined,
5554+
sessionId,
5555+
),
5556+
message: {
5557+
role: 'user',
5558+
content: [{
5559+
type: 'tool_result',
5560+
tool_use_id: toolUseId,
5561+
content: 'The user rejected this tool use.',
5562+
is_error: true,
5563+
}],
5564+
},
5565+
cwd: workDir,
5566+
},
5567+
makeAssistantEntry('The requested write was not completed.', userId),
5568+
])
5569+
5570+
const res = await fetch(`${baseUrl}/api/sessions/${sessionId}/turn-checkpoints`)
5571+
expect(res.status).toBe(200)
5572+
const body = await res.json() as { checkpoints: unknown[] }
5573+
5574+
expect(body.checkpoints).toEqual([])
5575+
await expect(fs.stat(path.join(workDir, 'permission-denial-test.txt')))
5576+
.rejects.toMatchObject({ code: 'ENOENT' })
5577+
})
5578+
55265579
it('GET /api/sessions/:id/turn-checkpoints/diff should return transcript tool diffs when file snapshots are missing', async () => {
55275580
const sessionId = '99999999-bbbb-cccc-dddd-000000000002'
55285581
const workDir = path.join(tmpDir, 'transcript-only-diff-session')

‎src/server/__tests__/task-notification-persistence.test.ts‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test'
2+
import * as fs from 'node:fs/promises'
3+
import * as os from 'node:os'
4+
import * as path from 'node:path'
5+
import {
6+
__persistCliTaskNotificationForTests,
7+
__resetWebSocketHandlerStateForTests,
8+
} from '../ws/handler.js'
9+
import { SessionService, sessionService } from '../services/sessionService.js'
10+
11+
describe('background task notification persistence', () => {
12+
let configDir = ''
13+
let previousConfigDir: string | undefined
14+
15+
beforeEach(async () => {
16+
previousConfigDir = process.env.CLAUDE_CONFIG_DIR
17+
configDir = await fs.mkdtemp(path.join(os.tmpdir(), 'cc-haha-task-notification-'))
18+
process.env.CLAUDE_CONFIG_DIR = configDir
19+
await fs.mkdir(path.join(configDir, 'projects'), { recursive: true })
20+
})
21+
22+
afterEach(async () => {
23+
__resetWebSocketHandlerStateForTests()
24+
mock.restore()
25+
if (previousConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR
26+
else process.env.CLAUDE_CONFIG_DIR = previousConfigDir
27+
await fs.rm(configDir, { recursive: true, force: true })
28+
})
29+
30+
it('restores a terminal Agent notification that was not forwarded into transcript history', async () => {
31+
const sessionId = crypto.randomUUID()
32+
const projectDir = path.join(configDir, 'projects', '-tmp-background-agent')
33+
const transcriptPath = path.join(projectDir, `${sessionId}.jsonl`)
34+
await fs.mkdir(projectDir, { recursive: true })
35+
await fs.writeFile(transcriptPath, `${JSON.stringify({
36+
type: 'assistant',
37+
uuid: crypto.randomUUID(),
38+
timestamp: '2026-07-18T00:00:00.000Z',
39+
message: {
40+
role: 'assistant',
41+
content: [{
42+
type: 'tool_use',
43+
id: 'agent-tool-1',
44+
name: 'Agent',
45+
input: { description: 'Verify background restore', run_in_background: true },
46+
}],
47+
},
48+
})}\n`, 'utf8')
49+
50+
const service = new SessionService()
51+
await service.appendSessionTaskNotification(sessionId, {
52+
taskId: 'agent-task-1',
53+
toolUseId: 'agent-tool-1',
54+
status: 'completed',
55+
summary: 'Agent completed after the foreground Skill output',
56+
result: 'Background verification passed',
57+
timestamp: '2026-07-18T00:01:00.000Z',
58+
})
59+
60+
expect(await service.getSessionTaskNotifications(sessionId)).toEqual([{
61+
taskId: 'agent-task-1',
62+
toolUseId: 'agent-tool-1',
63+
status: 'completed',
64+
summary: 'Agent completed after the foreground Skill output',
65+
result: 'Background verification passed',
66+
timestamp: '2026-07-18T00:01:00.000Z',
67+
}])
68+
expect(await fs.readFile(transcriptPath, 'utf8')).toContain('"type":"cc-haha-task-notification"')
69+
})
70+
71+
it('keeps restoring legacy task-notification transcript turns', async () => {
72+
const sessionId = crypto.randomUUID()
73+
const projectDir = path.join(configDir, 'projects', '-tmp-legacy-notification')
74+
const transcriptPath = path.join(projectDir, `${sessionId}.jsonl`)
75+
await fs.mkdir(projectDir, { recursive: true })
76+
await fs.writeFile(transcriptPath, `${JSON.stringify({
77+
type: 'user',
78+
uuid: crypto.randomUUID(),
79+
timestamp: '2026-07-17T00:00:00.000Z',
80+
message: {
81+
role: 'user',
82+
content: '<task-notification>\n<task-id>legacy-task</task-id>\n<tool-use-id>legacy-tool</tool-use-id>\n<status>completed</status>\n<summary>Legacy task completed</summary>\n</task-notification>',
83+
},
84+
})}\n`, 'utf8')
85+
86+
const service = new SessionService()
87+
expect(await service.getSessionTaskNotifications(sessionId)).toEqual([{
88+
taskId: 'legacy-task',
89+
toolUseId: 'legacy-tool',
90+
status: 'completed',
91+
summary: 'Legacy task completed',
92+
timestamp: '2026-07-17T00:00:00.000Z',
93+
}])
94+
})
95+
96+
it('normalizes and persists one terminal SDK event for multiple observers', async () => {
97+
const append = spyOn(sessionService, 'appendSessionTaskNotification').mockResolvedValue()
98+
const sdkEvent = {
99+
type: 'system',
100+
subtype: 'task_notification',
101+
uuid: 'terminal-event-1',
102+
task_id: 'agent-task-1',
103+
tool_use_id: 'agent-tool-1',
104+
status: 'completed',
105+
summary: 'Agent completed',
106+
result: 'All checks passed',
107+
output_file: '/tmp/agent-task-1.output',
108+
timestamp: '2026-07-18T00:01:00.000Z',
109+
}
110+
111+
const first = __persistCliTaskNotificationForTests('session-1', sdkEvent)
112+
const second = __persistCliTaskNotificationForTests('session-1', sdkEvent)
113+
expect(first).not.toBeNull()
114+
expect(second).toBe(first)
115+
await Promise.all([first, second])
116+
117+
expect(append).toHaveBeenCalledTimes(1)
118+
expect(append).toHaveBeenCalledWith('session-1', {
119+
taskId: 'agent-task-1',
120+
toolUseId: 'agent-tool-1',
121+
status: 'completed',
122+
summary: 'Agent completed',
123+
result: 'All checks passed',
124+
outputFile: '/tmp/agent-task-1.output',
125+
timestamp: '2026-07-18T00:01:00.000Z',
126+
})
127+
})
128+
})

‎src/server/__tests__/websocket-handler.test.ts‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { ServerWebSocket } from 'bun'
33
import {
44
__markPrewarmPendingForTests,
55
__markActiveTurnForTests,
6+
__refreshDisconnectedTurnCleanupWatcherForTests,
67
__registerPendingUserTurnForTests,
78
__markPrewarmedForTests,
89
__resetWebSocketHandlerStateForTests,
@@ -322,6 +323,57 @@ describe('WebSocket handler session isolation', () => {
322323
})
323324
})
324325

326+
it('persists terminal task notifications before forwarding them to the client', async () => {
327+
const sessionId = `task-notification-${crypto.randomUUID()}`
328+
const ws = makeClientSocket(sessionId)
329+
let outputCallback: ((cliMsg: any) => void) | null = null
330+
spyOn(conversationService, 'hasSession').mockReturnValue(true)
331+
spyOn(conversationService, 'onOutput').mockImplementation((_sid, callback) => {
332+
outputCallback = callback
333+
})
334+
const append = spyOn(sessionService, 'appendSessionTaskNotification').mockResolvedValue()
335+
336+
handleWebSocket.open(ws)
337+
ws.sent.length = 0
338+
339+
const completed = {
340+
type: 'system',
341+
subtype: 'task_notification',
342+
uuid: 'terminal-task-event-1',
343+
task_id: 'agent-task-1',
344+
tool_use_id: 'agent-tool-1',
345+
status: 'completed',
346+
summary: 'Background task completed',
347+
timestamp: '2026-07-18T00:01:00.000Z',
348+
}
349+
outputCallback?.(completed)
350+
await Promise.resolve()
351+
await Promise.resolve()
352+
353+
expect(append).toHaveBeenCalledTimes(1)
354+
expect(ws.sent.map((payload) => JSON.parse(payload))).toContainEqual({
355+
type: 'system_notification',
356+
subtype: 'task_notification',
357+
data: completed,
358+
})
359+
360+
// A running notification is UI activity, not a terminal state that should
361+
// be restored after restart. It must forward without another persistence.
362+
const running = {
363+
...completed,
364+
uuid: 'running-task-event-1',
365+
status: 'running',
366+
}
367+
outputCallback?.(running)
368+
369+
expect(append).toHaveBeenCalledTimes(1)
370+
expect(ws.sent.map((payload) => JSON.parse(payload))).toContainEqual({
371+
type: 'system_notification',
372+
subtype: 'task_notification',
373+
data: running,
374+
})
375+
})
376+
325377
it('broadcasts tool and Computer Use permission resolutions to every client', () => {
326378
const sessionId = `permission-resolution-${crypto.randomUUID()}`
327379
const first = makeClientSocket(sessionId)
@@ -430,24 +482,37 @@ describe('WebSocket handler session isolation', () => {
430482
expect(stopSession).toHaveBeenCalledWith(sessionId)
431483
})
432484

433-
it('starts the permission cleanup bound when the prompt arrives after disconnect', () => {
485+
it('starts the permission cleanup bound when disconnect happens before the turn is sent', () => {
434486
const sessionId = `late-permission-disconnect-${crypto.randomUUID()}`
435487
const ws = makeClientSocket(sessionId)
436488
const setTimeoutSpy = spyOn(globalThis, 'setTimeout').mockImplementation(() => 0 as any)
437489
const pendingRequests = spyOn(conversationService, 'getPendingPermissionRequests')
438490
.mockReturnValue([])
439491
let turnOutputCallback: ((cliMsg: any) => void) | null = null
492+
let cliSessionReady = false
440493
spyOn(conversationService, 'onOutput').mockImplementation((_sid, callback) => {
441-
turnOutputCallback = callback
494+
// ConversationService.onOutput is a no-op until startSession has inserted
495+
// the session. This was the gap hidden by the previous regression test.
496+
if (cliSessionReady) turnOutputCallback = callback
442497
})
443498
spyOn(conversationService, 'removeOutputCallback').mockImplementation(() => {})
444499

445500
handleWebSocket.open(ws)
446-
__markActiveTurnForTests(sessionId)
501+
// Mirrors the real H5 race: user_message has synchronously claimed the
502+
// turn, but CLI startup has not completed and messageSent is still false.
503+
__registerPendingUserTurnForTests(sessionId)
447504
setTimeoutSpy.mockClear()
448505
handleWebSocket.close(ws, 1006, 'renderer closed before permission prompt')
449506

450507
expect(setTimeoutSpy).not.toHaveBeenCalled()
508+
expect(turnOutputCallback).toBeNull()
509+
510+
// CLI startup finishes while the H5 tab remains closed. handleUserMessage
511+
// refreshes the watcher immediately before sending the queued turn.
512+
cliSessionReady = true
513+
__refreshDisconnectedTurnCleanupWatcherForTests(sessionId)
514+
expect(turnOutputCallback).not.toBeNull()
515+
451516
pendingRequests.mockReturnValue([{
452517
requestId: 'late-request-1',
453518
toolName: 'Bash',

‎src/server/__tests__/workspace-service.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,50 @@ describe('WorkspaceService', () => {
286286
expect(diff.diff).toContain('+export default function App() { return <main>New</main> }')
287287
})
288288

289+
it('does not report a rejected session tool edit as a changed file', async () => {
290+
const nonGitDir = await makeTempDir('workspace-service-rejected-change-')
291+
const toolUseId = 'Write:rejected'
292+
const service = new WorkspaceService(
293+
async () => nonGitDir,
294+
async () => [
295+
{
296+
id: 'assistant-1',
297+
type: 'tool_use',
298+
timestamp: new Date().toISOString(),
299+
content: [{
300+
type: 'tool_use',
301+
id: toolUseId,
302+
name: 'Write',
303+
input: {
304+
file_path: 'permission-denial-test.txt',
305+
content: 'must not be written\n',
306+
},
307+
}],
308+
},
309+
{
310+
id: 'tool-result-1',
311+
type: 'tool_result',
312+
timestamp: new Date().toISOString(),
313+
content: [{
314+
type: 'tool_result',
315+
tool_use_id: toolUseId,
316+
content: 'The user rejected this tool use.',
317+
is_error: true,
318+
}],
319+
},
320+
],
321+
)
322+
323+
const status = await service.getStatus('session-1')
324+
325+
expect(status).toMatchObject({
326+
state: 'ok',
327+
workDir: nonGitDir,
328+
isGitRepo: false,
329+
changedFiles: [],
330+
})
331+
})
332+
289333
it('reports file-history changes without requiring a git repository', async () => {
290334
const nonGitDir = await makeTempDir('workspace-service-file-history-')
291335
const generatedFile = path.join(nonGitDir, 'aacc', 'src', 'App.tsx')

‎src/server/services/sessionRewindService.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { getClaudeConfigHomeDir } from '../../utils/envUtils.js'
1010
import { conversationService } from './conversationService.js'
1111
import { sessionService, type MessageEntry } from './sessionService.js'
12+
import { collectErroredToolUseIds } from './transcriptToolResults.js'
1213

1314
type RewindTarget = {
1415
targetUserMessageId: string
@@ -618,13 +619,15 @@ function collectTranscriptTurnFileChanges(
618619
if (turnMessages.length === 0) return []
619620

620621
const changes = new Map<string, TranscriptFileChange>()
622+
const erroredToolUseIds = collectErroredToolUseIds(turnMessages)
621623
for (const message of turnMessages) {
622624
if (message.type !== 'tool_use' || !Array.isArray(message.content)) continue
623625

624626
for (const block of message.content) {
625627
if (!block || typeof block !== 'object') continue
626628
const record = block as Record<string, unknown>
627629
if (record.type !== 'tool_use' || typeof record.name !== 'string') continue
630+
if (typeof record.id === 'string' && erroredToolUseIds.has(record.id)) continue
628631
const input = record.input
629632
if (!input || typeof input !== 'object') continue
630633

0 commit comments

Comments
 (0)