mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(claude): preserve resume anchor and report real /compact outcome (#1056)
* fix(claude): consume the one-time --resume flag only once it is used or discarded
Reopening a remote Claude session and hitting a relaunch trigger (e.g.
/compact, or a mode/model/effort change) before the first turn is ever
processed loses the original --resume session id and starts a brand new
Claude session with no prior context. With /compact this also shows up as
the compaction ending immediately with "Not enough messages to compact.",
because the session it lands in is empty.
claudeRemoteLauncher called session.consumeOneTimeFlags() right after every
claudeRemote() call, including calls that returned before spawning Claude
(nextMessage() resolves null when the relaunch trigger arrives before any
turn was handled, so the message is parked as pending). That retired the
one-time --resume flag before the SDK ever had a chance to use it, so the
next launch started fresh instead of resuming.
Tie the flag's lifetime to the invariant it needs: retire it only once it
has been used, or once the context it points at has been explicitly
discarded. It is now consumed from onSessionFound (Claude reported a
session id back) and from onSessionReset (/clear dropped the context, and
/clear likewise returns before spawning Claude, so without this the flag
would outlive the reset and the next launch would resume the very session
the user just cleared). Attempts that reach neither outcome never touched
the anchor, so the flag survives for the next launch.
* fix(claude): report the actual /compact outcome instead of always success
When Claude cannot compact a session it says so on a system/status message
carrying compact_result: "failed" and a compact_error reason (for example
"Not enough messages to compact."), which arrives shortly before the result
message. The completion event was emitted from the result message alone, so
a compaction that did not happen was still surfaced to the user as
"Compaction completed".
Record the reported outcome when the status message arrives and use it when
the result message is handled, mirroring the wording the Codex launcher
already uses for the same situation ("Compaction failed: <reason>").
Only an explicitly reported failure is recorded: a status shape without
compact_result, or one reporting anything other than "failed", leaves the
existing success path untouched, so an unrecognised or unseen status can
never invent a failure.
This commit is contained in:
@@ -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<string[]> {
|
||||
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);
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, unknown> { 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<void> {}
|
||||
destroy(): void {}
|
||||
}
|
||||
}))
|
||||
|
||||
import { claudeRemoteLauncher } from './claudeRemoteLauncher'
|
||||
import { Session } from './session'
|
||||
|
||||
function createClientStub() {
|
||||
const rpcHandlers = new Map<string, () => void | Promise<void>>()
|
||||
return {
|
||||
rpcHandlerManager: {
|
||||
registerHandler: (method: string, handler: () => void | Promise<void>) => {
|
||||
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<typeof createClientStub>,
|
||||
claudeArgs: string[] | undefined
|
||||
) {
|
||||
const queue = new MessageQueue2<EnhancedMode>((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()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -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' });
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user