diff --git a/cli/src/claude/claudeRemote.test.ts b/cli/src/claude/claudeRemote.test.ts index 60723281..04afb0c2 100644 --- a/cli/src/claude/claudeRemote.test.ts +++ b/cli/src/claude/claudeRemote.test.ts @@ -285,3 +285,101 @@ describe('claudeRemote async message handling', () => { } }); }); + +describe('claudeRemote /compact result reporting', () => { + const resultMessage = { + type: 'result', + subtype: 'success', + num_turns: 1, + total_cost_usd: 0, + duration_ms: 1, + duration_api_ms: 1, + is_error: false, + session_id: 's-1' + } as unknown as SDKMessage; + + async function runCompact(sdkMessages: SDKMessage[]): Promise { + const querySpy = vi.spyOn(claudeSdk, 'query').mockImplementation(queryMock as typeof claudeSdk.query); + const { claudeRemote } = await import('./claudeRemote'); + const completionEvents: string[] = []; + + queryMock.mockReturnValueOnce(createAsyncStream(sdkMessages)); + + let nextCallCount = 0; + try { + await claudeRemote({ + sessionId: 'session-1', + path: process.cwd(), + mcpServers: {}, + claudeEnvVars: {}, + claudeArgs: [], + allowedTools: [], + hookSettingsPath: '/tmp/hook.json', + canCallTool: async () => ({ behavior: 'allow', updatedInput: {} }), + nextMessage: async () => { + nextCallCount += 1; + if (nextCallCount === 1) { + return { message: '/compact', mode: { permissionMode: 'default' } }; + } + return null; + }, + onReady: () => {}, + isAborted: () => false, + onSessionFound: () => {}, + onMessage: () => {}, + onCompletionEvent: (message) => { + completionEvents.push(message); + }, + onSessionReset: () => {} + }); + } finally { + queryMock.mockReset(); + querySpy.mockRestore(); + } + + return completionEvents; + } + + it('reports the failure reason when the SDK says the compaction failed', async () => { + // Shape taken from a real session: the SDK emits a 'compacting' status + // first, then a second status carrying the outcome. + const completionEvents = await runCompact([ + { + type: 'system', + subtype: 'status', + status: 'compacting', + session_id: 's-1', + uuid: 'u-1' + } as unknown as SDKMessage, + { + type: 'system', + subtype: 'status', + status: null, + compact_result: 'failed', + compact_error: 'Not enough messages to compact.', + session_id: 's-1', + uuid: 'u-2' + } as unknown as SDKMessage, + resultMessage + ]); + + expect(completionEvents).toContain('Compaction started'); + expect(completionEvents.some((event) => event.includes('Not enough messages to compact.'))).toBe(true); + expect(completionEvents).not.toContain('Compaction completed'); + }, 15_000); + + it('still reports success when no failure status arrives', async () => { + const completionEvents = await runCompact([ + { + type: 'system', + subtype: 'status', + status: 'compacting', + session_id: 's-1', + uuid: 'u-1' + } as unknown as SDKMessage, + resultMessage + ]); + + expect(completionEvents).toEqual(['Compaction started', 'Compaction completed']); + }, 15_000); +}); diff --git a/cli/src/claude/claudeRemote.ts b/cli/src/claude/claudeRemote.ts index 038b037f..4b97294f 100644 --- a/cli/src/claude/claudeRemote.ts +++ b/cli/src/claude/claudeRemote.ts @@ -112,6 +112,11 @@ export async function claudeRemote(opts: { // Handle /compact command let isCompactCommand = false; + // Claude reports the /compact outcome on a `system`/`status` message that + // arrives before the `result` message. Hold it here so the completion event + // can report what actually happened. Stays null unless a failure is + // reported, so an unseen or successful status keeps the success path. + let compactFailure: string | null = null; if (specialCommand.type === 'compact') { logger.debug('[claudeRemote] /compact command detected - will process as normal but with compaction behavior'); isCompactCommand = true; @@ -254,6 +259,20 @@ export async function claudeRemote(opts: { } } + // Capture the /compact outcome. Only a reported failure is recorded: + // anything else leaves the success path untouched, so a status shape + // we do not recognise cannot invent a failure. + if (message.type === 'system' && message.subtype === 'status' && isCompactCommand) { + const systemStatus = message as SDKSystemMessage; + if (systemStatus.compact_result === 'failed') { + const reason = typeof systemStatus.compact_error === 'string' + ? systemStatus.compact_error.trim() + : ''; + compactFailure = reason.length > 0 ? reason : 'Compaction failed'; + logger.debug(`[claudeRemote] Compaction reported as failed: ${compactFailure}`); + } + } + // Handle result messages if (message.type === 'result') { resultSeq += 1; @@ -265,11 +284,15 @@ export async function claudeRemote(opts: { // Send completion messages if (isCompactCommand) { - logger.debug('[claudeRemote] Compaction completed'); + const completion = compactFailure + ? `Compaction failed: ${compactFailure}` + : 'Compaction completed'; + logger.debug(`[claudeRemote] ${completion}`); if (opts.onCompletionEvent) { - opts.onCompletionEvent('Compaction completed'); + opts.onCompletionEvent(completion); } isCompactCommand = false; + compactFailure = null; } // Send ready event diff --git a/cli/src/claude/claudeRemoteLauncher.test.ts b/cli/src/claude/claudeRemoteLauncher.test.ts new file mode 100644 index 00000000..561fd8ec --- /dev/null +++ b/cli/src/claude/claudeRemoteLauncher.test.ts @@ -0,0 +1,252 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { RPC_METHODS } from '@hapi/protocol/rpcMethods' +import { MessageQueue2 } from '@/utils/MessageQueue2' +import type { EnhancedMode } from './loop' + +// These tests cover the one-time --resume flag's lifetime across the relaunch +// loop. The flag lives in session.claudeArgs and is the only resume anchor a +// remote Claude session has until Claude reports a session id back, so it must +// survive launch attempts that never reached Claude, and must be dropped both +// when it has been used and when the user explicitly discards the context. +const harness = vi.hoisted(() => ({ + callCount: 0, + claudeArgsPerCall: [] as (string[] | undefined)[], + triggerSwitch: null as (() => void) | null, + switchAfterCall: 2 +})) + +vi.mock('./claudeRemote', () => ({ + claudeRemote: async (opts: any) => { + const { parseSpecialCommand } = await import('@/parsers/specialCommands') + + harness.callCount += 1 + harness.claudeArgsPerCall.push(opts.claudeArgs ? [...opts.claudeArgs] : opts.claudeArgs) + + const initial = await opts.nextMessage() + if (!initial) { + // Mirrors claudeRemote()'s early return when the initial + // nextMessage() resolves null (an isolate message got parked as + // `pending`): it returns before spawning Claude, so onSessionFound + // never fires and the --resume flag is never actually used. + return + } + + // Mirrors claudeRemote()'s /clear contract: it reports the context as + // discarded and returns before spawning Claude, so onSessionFound + // never fires here either. + if (parseSpecialCommand(initial.message).type === 'clear') { + opts.onSessionReset() + return + } + + // Mirrors a launch that actually spawns Claude and observes the + // session id via the SDK's system/init message. + opts.onSessionFound('captured-session-id') + + if (harness.callCount === harness.switchAfterCall && harness.triggerSwitch) { + // Stop the runMainLoop() while-loop so the test doesn't hang + // waiting on a further claudeRemote() call. Mirrors the real + // 'switch' RPC exit path already wired by setupAbortHandlers(). + harness.triggerSwitch() + } + } +})) + +vi.mock('./utils/permissionHandler', () => ({ + PermissionHandler: class { + setOnPermissionRequest(): void {} + getResponses(): Map { return new Map() } + onMessage(): void {} + handleToolCall = async () => ({ behavior: 'allow', updatedInput: {} }) + reset(): void {} + isAborted(): boolean { return false } + handleModeChange(): void {} + } +})) + +vi.mock('./utils/sdkToLogConverter', () => ({ + SDKToLogConverter: class { + updateSessionId(): void {} + resetParentChain(): void {} + convert(): null { return null } + convertSidechainUserMessage(): null { return null } + updateSelectedModel(): void {} + generateInterruptedToolResult(): null { return null } + } +})) + +vi.mock('./utils/OutgoingMessageQueue', () => ({ + OutgoingMessageQueue: class { + releaseToolCall(): void {} + enqueue(): void {} + async flush(): Promise {} + destroy(): void {} + } +})) + +import { claudeRemoteLauncher } from './claudeRemoteLauncher' +import { Session } from './session' + +function createClientStub() { + const rpcHandlers = new Map void | Promise>() + return { + rpcHandlerManager: { + registerHandler: (method: string, handler: () => void | Promise) => { + rpcHandlers.set(method, handler) + } + }, + rpcHandlers, + keepAlive: () => {}, + updateMetadata: (mutator: (metadata: any) => any) => { mutator({}) }, + emitMessagesConsumed: () => {}, + sendClaudeSessionMessage: () => {}, + sendSessionEvent: () => {} + } +} + +const RESUME_ARGS = ['--resume', 'original-session-id'] + +// claudeArgs is required rather than defaulted: passing `undefined` to a +// defaulted parameter would silently fall back to the default and hand the +// no-resume test a --resume flag it is supposed to be running without. +function createSession( + client: ReturnType, + claudeArgs: string[] | undefined +) { + const queue = new MessageQueue2((mode) => JSON.stringify(mode)) + const session = new Session({ + api: {} as any, + client: client as any, + path: '/tmp/test', + logPath: '/tmp/test.log', + sessionId: null, + claudeEnvVars: {}, + claudeArgs, + mcpServers: {}, + messageQueue: queue, + onModeChange: () => {}, + allowedTools: [], + mode: 'remote', + startedBy: 'runner', + startingMode: 'remote', + hookSettingsPath: '/tmp/hook.json', + permissionMode: 'default' + }) + return { session, queue } +} + +describe('claudeRemoteLauncher resume anchor', () => { + afterEach(() => { + harness.callCount = 0 + harness.claudeArgsPerCall = [] + harness.triggerSwitch = null + harness.switchAfterCall = 2 + vi.clearAllMocks() + }) + + it('keeps --resume available for the launch that actually captures the session id', async () => { + const client = createClientStub() + const { session, queue } = createSession(client, [...RESUME_ARGS]) + + try { + // Simulate the reopen-then-idle-then-/compact repro: an + // isolate-triggering message is already queued before the very + // first claudeRemote() attempt ever runs. + queue.pushIsolateAndClear('/compact', { permissionMode: 'default' }, 'local-1') + harness.triggerSwitch = () => { + client.rpcHandlers.get(RPC_METHODS.Switch)?.() + } + + await claudeRemoteLauncher(session as any) + + expect(harness.callCount).toBe(2) + + // The first attempt bailed out before Claude ever spawned - it + // must not have consumed the one-time --resume flag. + // The second attempt is the one that actually captures the + // session, and it must still see --resume in claudeArgs. + expect(harness.claudeArgsPerCall[1]).toEqual(['--resume', 'original-session-id']) + expect(session.sessionId).toBe('captured-session-id') + } finally { + session.stopKeepAlive() + } + }) + + it('drops --resume when /clear discards the context before any launch reached Claude', async () => { + const client = createClientStub() + const { session, queue } = createSession(client, [...RESUME_ARGS]) + + try { + // /clear is pushed as an isolate message exactly like /compact, so + // it takes the same reopen-then-idle relaunch path. Unlike /compact + // it must NOT keep the resume anchor alive: the user asked for the + // context to be discarded, so the follow-up message has to start a + // genuinely fresh Claude session rather than resume the cleared one. + queue.pushIsolateAndClear('/clear', { permissionMode: 'default' }, 'local-1') + queue.push('hello', { permissionMode: 'default' }, 'local-2') + harness.switchAfterCall = 3 + harness.triggerSwitch = () => { + client.rpcHandlers.get(RPC_METHODS.Switch)?.() + } + + await claudeRemoteLauncher(session as any) + + // 1st attempt: bails out (isolate message parked as pending). + // 2nd attempt: runs /clear -> onSessionReset, still no spawn. + // 3rd attempt: the follow-up message, which must start fresh. + expect(harness.callCount).toBe(3) + expect(harness.claudeArgsPerCall[2]).toBeUndefined() + expect(session.claudeArgs).toBeUndefined() + } finally { + session.stopKeepAlive() + } + }) + + it('regression: still consumes --resume once the very first attempt captures the session (no relaunch needed)', async () => { + const client = createClientStub() + const { session, queue } = createSession(client, [...RESUME_ARGS]) + + try { + // Ordinary happy path: a normal (non-isolate) message is already + // queued, so the very first claudeRemote() attempt captures the + // session immediately - no relaunch/idle gap involved. + queue.push('hello', { permissionMode: 'default' }, 'local-1') + harness.switchAfterCall = 1 + harness.triggerSwitch = () => { + client.rpcHandlers.get(RPC_METHODS.Switch)?.() + } + + await claudeRemoteLauncher(session as any) + + expect(harness.callCount).toBe(1) + expect(harness.claudeArgsPerCall[0]).toEqual(['--resume', 'original-session-id']) + expect(session.sessionId).toBe('captured-session-id') + // The one-time flag must not linger once it has actually been + // consumed by a launch that captured the session id. + expect(session.claudeArgs).toBeUndefined() + } finally { + session.stopKeepAlive() + } + }) + + it('regression: fresh spawn with no --resume flag is unaffected', async () => { + const client = createClientStub() + const { session, queue } = createSession(client, undefined) + + try { + queue.push('hello', { permissionMode: 'default' }, 'local-1') + harness.switchAfterCall = 1 + harness.triggerSwitch = () => { + client.rpcHandlers.get(RPC_METHODS.Switch)?.() + } + + await claudeRemoteLauncher(session as any) + + expect(harness.callCount).toBe(1) + expect(harness.claudeArgsPerCall[0]).toBeUndefined() + expect(session.sessionId).toBe('captured-session-id') + } finally { + session.stopKeepAlive() + } + }) +}) diff --git a/cli/src/claude/claudeRemoteLauncher.ts b/cli/src/claude/claudeRemoteLauncher.ts index fb009dfc..1df84bdf 100644 --- a/cli/src/claude/claudeRemoteLauncher.ts +++ b/cli/src/claude/claudeRemoteLauncher.ts @@ -350,6 +350,13 @@ class ClaudeRemoteLauncher extends RemoteLauncherBase { }, onSessionFound: (sessionId) => { session.onSessionFound(sessionId); + // The one-time --resume flag must only be dropped once we know + // Claude actually captured a session id from it. If claudeRemote() + // bails out before this fires (e.g. the initial nextMessage() came + // back null because a relaunch trigger arrived before any turn was + // ever processed), the flag stays in session.claudeArgs so the next + // launch attempt can still resume the original session. + session.consumeOneTimeFlags(); }, onThinkingChange: session.onThinkingChange, claudeEnvVars: session.claudeEnvVars, @@ -362,6 +369,13 @@ class ClaudeRemoteLauncher extends RemoteLauncherBase { onSessionReset: () => { logger.debug('[remote]: Session reset'); session.clearSessionId(); + // /clear discards the resume anchor along with the + // context. Without this, the flag would outlive the + // reset (claudeRemote() returns before spawning + // Claude, so onSessionFound never fires) and the + // next launch would resume the session the user + // just asked to clear. + session.consumeOneTimeFlags(); }, onReady: () => { logger.debug( @@ -378,8 +392,6 @@ class ClaudeRemoteLauncher extends RemoteLauncherBase { signal: controller.signal, }); - session.consumeOneTimeFlags(); - if (!this.exitReason && controller.signal.aborted) { session.client.sendSessionEvent({ type: 'message', message: 'Aborted by user' }); } diff --git a/cli/src/claude/sdk/types.ts b/cli/src/claude/sdk/types.ts index 5af54833..927cf130 100644 --- a/cli/src/claude/sdk/types.ts +++ b/cli/src/claude/sdk/types.ts @@ -53,6 +53,13 @@ export interface SDKSystemMessage extends SDKMessage { cwd?: string tools?: string[] slash_commands?: string[] + /** + * Present on `subtype: 'status'` messages that report a /compact outcome. + * Claude emits a `status: 'compacting'` message first, then a second one + * carrying the result. + */ + compact_result?: string + compact_error?: string } export interface SDKResultMessage extends SDKMessage {