From 4a9965e22709ce929beef32b72201e6490892d0c Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:49:10 -0700 Subject: [PATCH] fix(review): cover Claude terminal stop_reasons and RPC lifecycle frames Treat max_tokens/stop_sequence/refusal as completed markers so capable hosts do not stay working after a dropped Stop, and assert lifecycle payloads on runtime subscribe/read frames plus mid-turn non-terminal stop_reason cases. --- .../transcript-turn-lifecycle.test.ts | 72 +++++++++--- .../native-chat/transcript-turn-lifecycle.ts | 13 ++- .../runtime/rpc/methods/native-chat.test.ts | 106 +++++++++++++++++- 3 files changed, 169 insertions(+), 22 deletions(-) diff --git a/src/main/native-chat/transcript-turn-lifecycle.test.ts b/src/main/native-chat/transcript-turn-lifecycle.test.ts index 3dfcb5b64cd..0748cec285c 100644 --- a/src/main/native-chat/transcript-turn-lifecycle.test.ts +++ b/src/main/native-chat/transcript-turn-lifecycle.test.ts @@ -73,22 +73,24 @@ describe('native chat transcript turn lifecycle', () => { ).toBeNull() }) - it('uses Claude end_turn and excludes tool-result user rows', () => { - expect( - decodeClaudeTurnLifecycle( - JSON.stringify({ - type: 'assistant', - uuid: 'assistant-1', - timestamp: '2026-07-16T23:45:37.608Z', - message: { role: 'assistant', stop_reason: 'end_turn', content: [] } - }), - 'fallback' - ) - ).toEqual({ - state: 'completed', - turnId: 'assistant-1', - timestamp: Date.parse('2026-07-16T23:45:37.608Z') - }) + it('uses Claude terminal stop_reasons and excludes tool-result user rows', () => { + for (const stopReason of ['end_turn', 'max_tokens', 'stop_sequence', 'refusal'] as const) { + expect( + decodeClaudeTurnLifecycle( + JSON.stringify({ + type: 'assistant', + uuid: `assistant-${stopReason}`, + timestamp: '2026-07-16T23:45:37.608Z', + message: { role: 'assistant', stop_reason: stopReason, content: [] } + }), + 'fallback' + ) + ).toEqual({ + state: 'completed', + turnId: `assistant-${stopReason}`, + timestamp: Date.parse('2026-07-16T23:45:37.608Z') + }) + } expect( decodeClaudeTurnLifecycle( @@ -106,6 +108,44 @@ describe('native chat transcript turn lifecycle', () => { ).toBeNull() }) + it('does not treat Claude mid-turn assistant rows as completed', () => { + expect( + decodeClaudeTurnLifecycle( + JSON.stringify({ + type: 'assistant', + uuid: 'assistant-tool', + timestamp: '2026-07-16T23:45:37.608Z', + message: { role: 'assistant', stop_reason: 'tool_use', content: [] } + }), + 'fallback' + ) + ).toBeNull() + + expect( + decodeClaudeTurnLifecycle( + JSON.stringify({ + type: 'assistant', + uuid: 'assistant-null-stop', + timestamp: '2026-07-16T23:45:37.608Z', + message: { role: 'assistant', stop_reason: null, content: [] } + }), + 'fallback' + ) + ).toBeNull() + + expect( + decodeClaudeTurnLifecycle( + JSON.stringify({ + type: 'assistant', + uuid: 'assistant-missing-stop', + timestamp: '2026-07-16T23:45:37.608Z', + message: { role: 'assistant', content: [] } + }), + 'fallback' + ) + ).toBeNull() + }) + it('excludes Claude tool-result rows that also carry text sidecars', () => { expect( decodeClaudeTurnLifecycle( diff --git a/src/main/native-chat/transcript-turn-lifecycle.ts b/src/main/native-chat/transcript-turn-lifecycle.ts index 4159d7469b2..609e018820b 100644 --- a/src/main/native-chat/transcript-turn-lifecycle.ts +++ b/src/main/native-chat/transcript-turn-lifecycle.ts @@ -59,6 +59,13 @@ export function decodeCodexTurnLifecycle( } } +/** Claude stop reasons that end the lead generation (not mid-turn tool_use). */ +const CLAUDE_TERMINAL_STOP_REASONS = new Set(['end_turn', 'max_tokens', 'stop_sequence', 'refusal']) + +function isClaudeTerminalStopReason(value: unknown): boolean { + return typeof value === 'string' && CLAUDE_TERMINAL_STOP_REASONS.has(value) +} + export function decodeClaudeTurnLifecycle( line: string, fallbackId: string @@ -75,10 +82,12 @@ export function decodeClaudeTurnLifecycle( // the active generation and must not be mistaken for the next user prompt. return { state: 'interrupted', turnId: interruptedMessageId, timestamp } } - if (record.type === 'assistant' && message?.stop_reason === 'end_turn') { + // Why: capable hosts disable the prose-fallback settlement path, so every + // real terminal stop_reason must emit completed — not only end_turn. + if (record.type === 'assistant' && isClaudeTerminalStopReason(message?.stop_reason)) { return { state: 'completed', - turnId: extractString(record.uuid) ?? extractString(message.id) ?? fallbackId, + turnId: extractString(record.uuid) ?? extractString(message?.id) ?? fallbackId, timestamp } } diff --git a/src/main/runtime/rpc/methods/native-chat.test.ts b/src/main/runtime/rpc/methods/native-chat.test.ts index 50202529e62..1086a473b15 100644 --- a/src/main/runtime/rpc/methods/native-chat.test.ts +++ b/src/main/runtime/rpc/methods/native-chat.test.ts @@ -5,16 +5,45 @@ import type { RpcContext } from '../core' // Stub the bounded tail reader so the handler returns a deterministic transcript with // one oversized tool-result block; the test then asserts clip behavior per client. const OVERSIZED = 'x'.repeat(5000) -const cachedResult = vi.hoisted(() => ({ value: { messages: [] as NativeChatMessage[] } })) +const cachedResult = vi.hoisted(() => ({ + value: { + messages: [] as NativeChatMessage[], + lifecycle: undefined as + | { state: 'working' | 'completed' | 'interrupted'; turnId: string; timestamp: number | null } + | undefined + } +})) const watcher = vi.hoisted(() => ({ args: null as null | { onInitialSnapshot?: ( messages: NativeChatMessage[], hasMore: boolean, beforeOffset: number, - error?: string + error?: string, + lifecycle?: { + state: 'working' | 'completed' | 'interrupted' + turnId: string + timestamp: number | null + } + ) => void + onReplace?: ( + messages: NativeChatMessage[], + hasMore: boolean, + beforeOffset: number, + lifecycle?: { + state: 'working' | 'completed' | 'interrupted' + turnId: string + timestamp: number | null + } + ) => void + onAppend: ( + messages: NativeChatMessage[], + lifecycle?: { + state: 'working' | 'completed' | 'interrupted' + turnId: string + timestamp: number | null + } ) => void - onAppend: (messages: NativeChatMessage[]) => void }, watching: true })) @@ -24,7 +53,8 @@ vi.mock('../../../native-chat/transcript-watch', () => ({ return Promise.resolve({ messages: messages.slice(-limit), hasMore: messages.length > limit, - beforeOffset: 123 + beforeOffset: 123, + ...(cachedResult.value.lifecycle ? { lifecycle: cachedResult.value.lifecycle } : {}) }) }, subscribeNativeChatTranscript: (args: NonNullable) => { @@ -387,4 +417,72 @@ describe('nativeChat.subscribe initial snapshot', () => { } ]) }) + + it('forwards lifecycle on snapshot, append, and replacement frames', async () => { + watcher.watching = true + watcher.args = null + const emitted: unknown[] = [] + await subscribeHandler()( + { agent: 'claude', sessionId: 's' }, + streamingContext('runtime'), + (value) => emitted.push(value) + ) + + const completed = { + state: 'completed' as const, + turnId: 'turn-rpc-1', + timestamp: 1_720_000_000_000 + } + const callbacks = activeWatcherArgs() + callbacks.onInitialSnapshot?.([makeMessage('snap')], false, 3, undefined, completed) + callbacks.onAppend([], completed) + callbacks.onReplace?.([makeMessage('repl')], false, 9, completed) + + expect(emitted).toEqual([ + { + type: 'snapshot', + messages: [expect.objectContaining({ id: 'a-1' })], + hasMore: false, + beforeOffset: 3, + turnLifecycleCapable: true, + lifecycle: completed + }, + { + type: 'appended', + messages: [], + turnLifecycleCapable: true, + lifecycle: completed + }, + { + type: 'replacement', + messages: [expect.objectContaining({ id: 'a-1' })], + hasMore: false, + beforeOffset: 9, + turnLifecycleCapable: true, + lifecycle: completed + } + ]) + }) +}) + +describe('nativeChat.readSession lifecycle payload', () => { + it('forwards lifecycle from the tail reader on success', async () => { + const lifecycle = { + state: 'interrupted' as const, + turnId: 'turn-read-1', + timestamp: 1_720_000_000_100 + } + cachedResult.value = { messages: [makeMessage('done')], lifecycle } + const result = await readSessionHandler()( + { agent: 'claude', sessionId: 's' }, + ctxWith('runtime') + ) + expect(result).toMatchObject({ + hasMore: false, + beforeOffset: 123, + turnLifecycleCapable: true, + lifecycle + }) + expect((result as { messages: NativeChatMessage[] }).messages).toHaveLength(1) + }) })