diff --git a/src/main/agent-hooks/manual-compact-hook-stream.test.ts b/src/main/agent-hooks/manual-compact-hook-stream.test.ts index 22b1b0605c2..5db4b606116 100644 --- a/src/main/agent-hooks/manual-compact-hook-stream.test.ts +++ b/src/main/agent-hooks/manual-compact-hook-stream.test.ts @@ -12,25 +12,11 @@ vi.mock('../telemetry/client', () => ({ track: vi.fn() })) vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) })) const PANE_KEY = makePaneKey('manual-compact', '11111111-1111-4111-8111-111111111111') -const PROMPT_ID_1 = '22222222-2222-4222-8222-222222222222' -const PROMPT_ID_2 = '33333333-3333-4333-8333-333333333333' +const COMPACT_PROMPT_ID = '22222222-2222-4222-8222-222222222222' +const TURN_PROMPT_ID = '33333333-3333-4333-8333-333333333333' const SESSION = { key: 'session_id' as const, id: 'session-a' } -type ClaudeHook = { - hook_event_name: string - prompt?: string - prompt_id: string - session_id: string - trigger?: 'manual' | 'auto' - agent_id?: string - agent_type?: string -} - -function claudeHook( - hookEventName: string, - promptId: string, - extra: Partial = {} -): ClaudeHook { +function claudeHook(hookEventName: string, promptId: string, extra: Record = {}) { return { hook_event_name: hookEventName, prompt_id: promptId, @@ -39,30 +25,46 @@ function claudeHook( } } -function manualEnvelope(hookEventName: string, state: 'working' | 'done') { - return { - source: 'claude' as const, - paneKey: PANE_KEY, - hasExplicitPrompt: hookEventName === 'UserPromptSubmit' ? true : undefined, - hookEventName, - providerPromptId: hookEventName === 'UserPromptSubmit' ? PROMPT_ID_2 : PROMPT_ID_1, - compactTrigger: hookEventName === 'UserPromptSubmit' ? undefined : ('manual' as const), - providerSession: SESSION, - payload: { state, prompt: 'work before compact', agentType: 'claude' as const } - } -} - function postHook(port: number, token: string, payload: Record) { return fetch(`http://127.0.0.1:${port}/hook/claude`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Orca-Agent-Hook-Token': token - }, + headers: { 'Content-Type': 'application/json', 'X-Orca-Agent-Hook-Token': token }, body: JSON.stringify({ paneKey: PANE_KEY, payload }) }) } +function turnEnvelope(): AgentHookRelayEnvelope { + return { + source: 'claude', + paneKey: PANE_KEY, + connectionId: null, + hasExplicitPrompt: true, + hookEventName: 'UserPromptSubmit', + providerPromptId: TURN_PROMPT_ID, + providerSession: SESSION, + payload: { state: 'working', prompt: 'work before compact', agentType: 'claude' } + } as AgentHookRelayEnvelope +} + +/** What a relay predating this change forwards: it ran its own shipped normalizer, so a manual + * compact arrives as a PLAIN `done` (no session boundary) and an auto compact as `working`. */ +function legacyRelayCompactEnvelope( + state: 'done' | 'working', + overrides: Partial = {} +): AgentHookRelayEnvelope { + return { + source: 'claude', + paneKey: PANE_KEY, + connectionId: null, + hookEventName: 'PostCompact', + providerPromptId: COMPACT_PROMPT_ID, + compactTrigger: state === 'done' ? 'manual' : 'auto', + providerSession: SESSION, + payload: { state, prompt: 'work before compact', agentType: 'claude' }, + ...overrides + } as AgentHookRelayEnvelope +} + describe('manual Claude compact hook stream', () => { const servers: { stop: () => void }[] = [] const temporaryPaths: string[] = [] @@ -76,75 +78,51 @@ describe('manual Claude compact hook stream', () => { } }) - it('settles an exact local HTTP lifecycle and rejects a duplicate completion', async () => { + it('settles the measured local lifecycle and rejects a duplicate completion', async () => { const server = new AgentHookServer() servers.push(server) await server.start({ env: 'production' }) const env = server.buildPtyEnv() + const port = Number(env.ORCA_AGENT_HOOK_PORT) + const token = env.ORCA_AGENT_HOOK_TOKEN const events: string[] = [] const unsubscribe = server.subscribeEnrichedStatus((event) => { events.push(`${event.hookEventName}:${event.payload.state}`) }) + await postHook(port, token, claudeHook('UserPromptSubmit', TURN_PROMPT_ID, { prompt: 'work' })) + // Measured stream for a successful manual /compact. PreCompact is not registered in production + // and is rejected here too; the summarizer's start-less SubagentStop only republishes state. + await postHook(port, token, claudeHook('PreCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('UserPromptSubmit', PROMPT_ID_2, { prompt: 'work before compact' }) - ) - await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PreCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) - await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('SubagentStart', PROMPT_ID_1, { - agent_id: 'compact-agent', - agent_type: 'general-purpose' + port, + token, + claudeHook('SubagentStop', COMPACT_PROMPT_ID, { + agent_id: 'a75b38b59774e1f31', + agent_type: '', + background_tasks: [], + session_crons: [] }) ) await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('SubagentStop', PROMPT_ID_1, { - agent_id: 'compact-agent', - agent_type: 'general-purpose' - }) - ) - await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PostCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) - await postHook( - Number(env.ORCA_AGENT_HOOK_PORT), - env.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PostCompact', PROMPT_ID_1, { trigger: 'manual' }) + port, + token, + claudeHook('SessionStart', COMPACT_PROMPT_ID, { source: 'compact' }) ) + await postHook(port, token, claudeHook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) + await postHook(port, token, claudeHook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) - expect(events).toEqual([ - 'UserPromptSubmit:working', - 'PreCompact:working', - 'SubagentStart:working', - 'SubagentStop:working', - 'PostCompact:done' - ]) + expect(events).toEqual(['UserPromptSubmit:working', 'SubagentStop:working', 'PostCompact:done']) expect(server.getStatusSnapshot()).toEqual([ - expect.objectContaining({ - state: 'done', - prompt: 'work before compact', - agentType: 'claude' - }) + expect.objectContaining({ state: 'done', prompt: 'work', agentType: 'claude' }) ]) unsubscribe() }) - it('preserves the manual identity over relay and rejects stale transport identities', async () => { + it('forwards the completion over the relay and neutralizes its cached compact identity', async () => { const main = new AgentHookServer() const forwarded: AgentHookRelayEnvelope[] = [] const emitted: string[] = [] - const connectionId = 'conn-a' const endpointDir = mkdtempSync(join(tmpdir(), 'orca-compact-relay-')) temporaryPaths.push(endpointDir) const relay = new RelayAgentHookServer({ @@ -152,7 +130,7 @@ describe('manual Claude compact hook stream', () => { token: 'manual-compact-token', forward: (envelope) => { forwarded.push(envelope) - main.ingestRemote(envelope, connectionId) + main.ingestRemote(envelope, 'conn-a') } }) servers.push(main, relay) @@ -160,208 +138,154 @@ describe('manual Claude compact hook stream', () => { emitted.push(`${event.hookEventName}:${event.payload.state}`) }) await relay.start({ publishEndpoint: false }) - const coordinates = relay.getCoordinates() + const { port, token } = relay.getCoordinates() - await postHook( - coordinates.port, - coordinates.token, - claudeHook('UserPromptSubmit', PROMPT_ID_2, { prompt: 'work before compact' }) - ) - // A restarted relay has no prior listener cache; main still owns the status boundary. - relay.clearPaneState(PANE_KEY) - await postHook( - coordinates.port, - coordinates.token, - claudeHook('PreCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) - const validPost = { - ...forwarded.at(-1)!, - hookEventName: 'PostCompact', - payload: { - state: 'done' as const, - prompt: 'work before compact', - agentType: 'claude' as const - } - } - - main.ingestRemote({ ...validPost, providerPromptId: PROMPT_ID_2 }, 'conn-a') - main.ingestRemote({ ...validPost, providerPromptId: undefined }, 'conn-a') - main.ingestRemote({ ...validPost, providerSession: undefined }, 'conn-a') - main.ingestRemote({ ...validPost, source: 'codex' }, 'conn-a') - main.ingestRemote(validPost, 'conn-b') - - expect(emitted).toEqual(['UserPromptSubmit:working', 'PreCompact:working']) - expect(main.getStatusSnapshot()[0]).toMatchObject({ - state: 'working', - prompt: 'work before compact' - }) - - await postHook( - coordinates.port, - coordinates.token, - claudeHook('SubagentStart', PROMPT_ID_1, { - agent_id: 'compact-agent', - agent_type: 'general-purpose' - }) - ) - await postHook( - coordinates.port, - coordinates.token, - claudeHook('SubagentStop', PROMPT_ID_1, { - agent_id: 'compact-agent', - agent_type: 'general-purpose' - }) - ) - await postHook( - coordinates.port, - coordinates.token, - claudeHook('PostCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) + await postHook(port, token, claudeHook('UserPromptSubmit', TURN_PROMPT_ID, { prompt: 'work' })) + await postHook(port, token, claudeHook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) expect(forwarded.at(-1)).toMatchObject({ source: 'claude', - providerPromptId: PROMPT_ID_1, + hookEventName: 'PostCompact', compactTrigger: 'manual', - providerSession: SESSION, - hookEventName: 'PostCompact' + providerPromptId: COMPACT_PROMPT_ID }) expect(emitted.at(-1)).toBe('PostCompact:done') - expect(emitted.slice(-3)).toEqual([ - 'SubagentStart:working', - 'SubagentStop:working', - 'PostCompact:done' - ]) - expect(main.getStatusSnapshot()[0]).toMatchObject({ - state: 'done', - prompt: 'work before compact' - }) + expect(main.getStatusSnapshot()[0]).toMatchObject({ state: 'done', sessionBoundary: true }) + + // Why: a client that was offline during the compact receives the row only as a reconnect + // replay of this cache. Cached with its compact identity stripped, that replay is an ordinary + // status row rather than a compact event a guard would reject. + const replayed = relay.replayCachedPayloadsForPanes() + expect(replayed).toBeGreaterThan(0) + expect(forwarded.at(-1)).toMatchObject({ isReplay: true, payload: { state: 'done' } }) + expect(forwarded.at(-1)?.hookEventName).toBeUndefined() + expect(forwarded.at(-1)?.compactTrigger).toBeUndefined() unsubscribe() }) - it('invalidates a compact completion when later provider work owns the pane', () => { + it('stamps the silent boundary on a manual completion from a relay that predates it', () => { const server = new AgentHookServer() servers.push(server) - server.ingestRemote(manualEnvelope('UserPromptSubmit', 'working'), 'conn-a') - server.ingestRemote(manualEnvelope('PreCompact', 'working'), 'conn-a') + server.ingestRemote(turnEnvelope(), 'conn-a') + expect(server.getStatusSnapshot()[0]).toMatchObject({ state: 'working' }) + + server.ingestRemote(legacyRelayCompactEnvelope('done'), 'conn-a') + + // Why: the old relay built this payload before the flag existed, so it arrives as a plain + // `done` that every completion-reactive consumer would read as a finished turn. + expect(server.getStatusSnapshot()[0]).toMatchObject({ state: 'done', sessionBoundary: true }) + }) + + it('drops an auto compact from a relay that predates it, instead of minting working', () => { + const server = new AgentHookServer() + servers.push(server) + server.ingestRemote(turnEnvelope(), 'conn-a') server.ingestRemote( { - source: 'codex', - paneKey: PANE_KEY, - hookEventName: 'UserPromptSubmit', - hasExplicitPrompt: true, - payload: { state: 'working', prompt: 'new work', agentType: 'codex' } - }, + ...turnEnvelope(), + payload: { state: 'done', prompt: 'work', agentType: 'claude' }, + hookEventName: 'Stop' + } as AgentHookRelayEnvelope, 'conn-a' ) - server.ingestRemote({ ...manualEnvelope('PreCompact', 'working'), isReplay: true }, 'conn-a') - server.ingestRemote(manualEnvelope('PostCompact', 'done'), 'conn-a') + const before = server.getStatusSnapshot()[0] + expect(before).toMatchObject({ state: 'done' }) - expect(server.getStatusSnapshot()[0]).toMatchObject({ - state: 'working', - prompt: 'new work', - agentType: 'claude' - }) - expect(server._getStateForTests().lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - source: 'codex', - hookEventName: 'UserPromptSubmit' - }) + server.ingestRemote(legacyRelayCompactEnvelope('working'), 'conn-a') + + expect(server.getStatusSnapshot()[0]).toMatchObject({ state: 'done' }) }) - it('keeps automatic compact hooks working without settling the turn', async () => { - const main = new AgentHookServer() - const forwarded: AgentHookRelayEnvelope[] = [] - const endpointDir = mkdtempSync(join(tmpdir(), 'orca-auto-compact-relay-')) - temporaryPaths.push(endpointDir) - const relay = new RelayAgentHookServer({ - endpointDir, - forward: (envelope) => { - forwarded.push(envelope) - main.ingestRemote(envelope, 'conn-auto') - } - }) - servers.push(main, relay) - await relay.start({ publishEndpoint: false }) - const { port, token } = relay.getCoordinates() - - await postHook( - port, - token, - claudeHook('UserPromptSubmit', PROMPT_ID_1, { prompt: 'continue the task' }) - ) - await postHook(port, token, claudeHook('PreCompact', PROMPT_ID_1, { trigger: 'auto' })) - await postHook(port, token, claudeHook('PostCompact', PROMPT_ID_1, { trigger: 'auto' })) - - expect(forwarded.map((event) => event.hookEventName)).toEqual([ - 'UserPromptSubmit', - 'PreCompact', - 'PostCompact' - ]) - expect(main.getStatusSnapshot()[0]).toMatchObject({ state: 'working', agentType: 'claude' }) - expect(main._getStateForTests().lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - hookEventName: 'PostCompact', - compactTrigger: undefined - }) - }) - - it('rejects unproven compact sources at the main relay boundary', () => { + it('suppresses a duplicate completion arriving twice over the relay', () => { const server = new AgentHookServer() servers.push(server) + const emitted: string[] = [] + const unsubscribe = server.subscribeEnrichedStatus((event) => { + emitted.push(`${event.hookEventName}:${event.payload.state}`) + }) + server.ingestRemote(turnEnvelope(), 'conn-a') + server.ingestRemote(legacyRelayCompactEnvelope('done'), 'conn-a') + const applied = server._getStateForTests().lastStatusByPaneKey.get(PANE_KEY) + expect(applied?.payload.state).toBe('done') + + server.ingestRemote(legacyRelayCompactEnvelope('done'), 'conn-a') + + // Why: the deleted ownership cache used to reject a repeat; a same-owner guard alone would + // accept it and keep refreshing the row, so suppression is keyed on the consumed compact id. + expect(emitted).toEqual(['UserPromptSubmit:working', 'PostCompact:done']) + expect(server._getStateForTests().lastStatusByPaneKey.get(PANE_KEY)).toBe(applied) + unsubscribe() + }) + + it('classifies a triggerless replay by payload state and still checks ownership', () => { + const done = new AgentHookServer() + servers.push(done) + done.ingestRemote(turnEnvelope(), 'conn-a') + // An older relay strips compactTrigger from its cached PostCompact before replaying it. + done.ingestRemote( + legacyRelayCompactEnvelope('done', { compactTrigger: undefined, isReplay: true }), + 'conn-a' + ) + expect(done.getStatusSnapshot()[0]).toMatchObject({ state: 'done', sessionBoundary: true }) + + const working = new AgentHookServer() + servers.push(working) + working.ingestRemote(turnEnvelope(), 'conn-a') + working.ingestRemote( + legacyRelayCompactEnvelope('working', { compactTrigger: undefined, isReplay: true }), + 'conn-a' + ) + expect(working.getStatusSnapshot()[0]).toMatchObject({ state: 'working' }) + + const foreign = new AgentHookServer() + servers.push(foreign) + foreign.ingestRemote(turnEnvelope(), 'conn-a') + // Payload state substitutes for the missing trigger only — ownership is still enforced. + foreign.ingestRemote( + legacyRelayCompactEnvelope('done', { + compactTrigger: undefined, + isReplay: true, + providerSession: { key: 'session_id', id: 'a-different-session' } + }), + 'conn-a' + ) + expect(foreign.getStatusSnapshot()[0]).toMatchObject({ state: 'working' }) + }) + + it('rejects a completion with no provider prompt id', () => { + const server = new AgentHookServer() + servers.push(server) + server.ingestRemote(turnEnvelope(), 'conn-a') + + server.ingestRemote( + legacyRelayCompactEnvelope('done', { providerPromptId: undefined }), + 'conn-a' + ) + + expect(server.getStatusSnapshot()[0]).toMatchObject({ state: 'working' }) + }) + + it('never lets a PreCompact envelope drive pane state', () => { + const server = new AgentHookServer() + servers.push(server) + server.ingestRemote(turnEnvelope(), 'conn-a') server.ingestRemote( { - ...manualEnvelope('PreCompact', 'working'), - source: 'kimi', - payload: { state: 'working', prompt: '', agentType: 'kimi' } - }, - 'conn-kimi' + ...turnEnvelope(), + payload: { state: 'done', prompt: 'work', agentType: 'claude' }, + hookEventName: 'Stop' + } as AgentHookRelayEnvelope, + 'conn-a' ) + server.ingestRemote( - { ...manualEnvelope('PreCompact', 'working'), compactTrigger: undefined }, - 'conn-auto' + legacyRelayCompactEnvelope('working', { + hookEventName: 'PreCompact', + compactTrigger: 'manual' + }), + 'conn-a' ) - expect(server.getStatusSnapshot()).toEqual([]) - expect(server._getStateForTests().lastStatusByPaneKey.size).toBe(0) - }) - - it('hydrates a manual PreCompact identity and accepts only its exact local completion', async () => { - const userDataPath = mkdtempSync(join(tmpdir(), 'orca-compact-restore-')) - temporaryPaths.push(userDataPath) - const first = new AgentHookServer() - servers.push(first) - await first.start({ env: 'production', userDataPath }) - const firstEnv = first.buildPtyEnv() - await postHook( - Number(firstEnv.ORCA_AGENT_HOOK_PORT), - firstEnv.ORCA_AGENT_HOOK_TOKEN, - claudeHook('UserPromptSubmit', PROMPT_ID_2, { prompt: 'work before compact' }) - ) - await postHook( - Number(firstEnv.ORCA_AGENT_HOOK_PORT), - firstEnv.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PreCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) - first.stop() - - const restored = new AgentHookServer() - servers.push(restored) - await restored.start({ env: 'production', userDataPath }) - const restoredEnv = restored.buildPtyEnv() - await postHook( - Number(restoredEnv.ORCA_AGENT_HOOK_PORT), - restoredEnv.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PostCompact', PROMPT_ID_2, { trigger: 'manual' }) - ) - expect(restored.getStatusSnapshot()[0]).toMatchObject({ state: 'working' }) - - await postHook( - Number(restoredEnv.ORCA_AGENT_HOOK_PORT), - restoredEnv.ORCA_AGENT_HOOK_TOKEN, - claudeHook('PostCompact', PROMPT_ID_1, { trigger: 'manual' }) - ) - expect(restored.getStatusSnapshot()[0]).toMatchObject({ - state: 'done', - prompt: 'work before compact', - agentType: 'claude' - }) + expect(server.getStatusSnapshot()[0]).toMatchObject({ state: 'done' }) }) }) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 7bb86a34142..a956a4c8b70 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -22,14 +22,12 @@ import { markCodexLeadTurnInterrupted, MAX_PANE_KEY_LEN, movePaneCacheState, - canAcceptClaudeCompactTransition, normalizeClaudePromptId, normalizeHookPayload, parseFormEncodedBody, readRequestBody, reapRestoredClaudeSubagentsForDeadPane, reconcileRemoteCodexState, - resolveCachedClaudeCompactOwnership, resolveHookSource, preparePendingGrokResultDiscovery, readHookBodyProviderSessionId, @@ -41,6 +39,12 @@ import { type AgentHookEventPayload, type HookListenerState } from '../../shared/agent-hook-listener' +import { + canAcceptClaudeCompactCompletion, + isClaudeCompactCompletionConsumed, + markClaudeCompactCompletionConsumed, + resolveLegacyCompactTrigger +} from '../../shared/claude-compact-completion' import { createHookTransportInterferenceTracker, describeHookTransportInterference, @@ -1432,8 +1436,7 @@ export class AgentHookServer { if (!identity.inheritedFromActivePane) { this.maybeTrackAgentPromptSent(effectivePayload, previous) } - const cachedPayload = resolveCachedClaudeCompactOwnership(previous, boundaryAwarePayload) - const enriched = this.attachStatusTiming(cachedPayload, now) + const enriched = this.attachStatusTiming(boundaryAwarePayload, now) if ( typeof enriched.payload.turnCompletedAt === 'number' && Number.isFinite(enriched.payload.turnCompletedAt) @@ -2317,34 +2320,49 @@ export class AgentHookServer { ) const previousStatus = this.state.lastStatusByPaneKey.get(paneKey) if (hookEventName === 'PreCompact' || hookEventName === 'PostCompact') { - if ( - source !== 'claude' || - compactTrigger === undefined || - normalizedPayload.agentType !== source - ) { + // Why: PreCompact is never registered and proves nothing (an aborted compact emits it alone); + // reject it here too so a host on any version cannot drive pane state from it. + if (hookEventName === 'PreCompact' || source !== 'claude') { + return + } + // Why: a relay predating this change strips `compactTrigger` from its cached PostCompact + // before replaying it, so the replay has no manual/auto discriminator. That relay's mapping is + // fixed and known — manual produced `done`, auto produced `working` — so the payload state + // stands in for the missing trigger. Trigger substitution only; ownership is still checked. + const effectiveTrigger = resolveLegacyCompactTrigger(compactTrigger, normalizedPayload.state) + // Why: an auto compact happens inside a turn that resumes and emits its own Stop. An older + // relay maps it to `working`, and this ingest applies the relay's payload verbatim — so + // without this drop, every auto compact on such a host mints exactly the stuck `working` this + // change removes. + if (effectiveTrigger !== 'manual' || normalizedPayload.agentType !== source) { return } if ( - hookEventName === 'PreCompact' && - envelope.isReplay === true && - (previousStatus?.hookEventName !== 'PreCompact' || - previousStatus.compactTrigger !== compactTrigger || - previousStatus.providerPromptId !== providerPromptId) - ) { - return - } - if ( - !canAcceptClaudeCompactTransition(previousStatus, { + isClaudeCompactCompletionConsumed( + this.state.claudeConsumedCompactPromptIdByPaneKey, + paneKey, + providerPromptId + ) || + !canAcceptClaudeCompactCompletion(previousStatus, { source, connectionId: trimmedConnectionId, - hookEventName, providerPromptId, - compactTrigger, providerSession }) ) { return } + markClaudeCompactCompletionConsumed( + this.state.claudeConsumedCompactPromptIdByPaneKey, + paneKey, + providerPromptId + ) + // Why: an older relay built this payload before the boundary flag existed, so it arrives as a + // plain `done` — which every completion-reactive consumer reads as a finished turn. Stamp the + // boundary here so a compact stays silent regardless of which relay normalized it. + if (normalizedPayload.sessionBoundary !== true) { + normalizedPayload = { ...normalizedPayload, sessionBoundary: true } + } } if ( source === 'claude' && diff --git a/src/main/claude/compact-status-registration.test.ts b/src/main/claude/compact-status-registration.test.ts new file mode 100644 index 00000000000..a9a66f1cfd3 --- /dev/null +++ b/src/main/claude/compact-status-registration.test.ts @@ -0,0 +1,273 @@ +import { describe, expect, it } from 'vitest' + +import { + createHookListenerState, + normalizeHookPayload, + type AgentHookEventPayload, + type HookListenerState +} from '../../shared/agent-hook-listener' +import { makePaneKey } from '../../shared/stable-pane-id' +import { applyManagedHooks, CLAUDE_EVENTS } from './hook-settings' + +const PANE_KEY = makePaneKey('compact-registration', '11111111-1111-4111-8111-111111111111') +const TURN_PROMPT_ID = '22222222-2222-4222-8222-222222222222' +const COMPACT_PROMPT_ID = '33333333-3333-4333-8333-333333333333' + +const REGISTERED_EVENT_NAMES = new Set(CLAUDE_EVENTS.map((event) => event.eventName)) + +/** Deliver a hook event the way production does: only if the event is actually REGISTERED with + * Claude. Feeding the normalizer directly is what let a whole compact-correlation feature ship + * green while being unreachable — every behavior assertion here goes through this gate. */ +function deliverIfRegistered( + state: HookListenerState, + payload: Record +): AgentHookEventPayload | null { + if (!REGISTERED_EVENT_NAMES.has(payload.hook_event_name as string)) { + return null + } + const event = normalizeHookPayload(state, 'claude', { paneKey: PANE_KEY, payload }, 'production') + if (event) { + state.lastStatusByPaneKey.set(PANE_KEY, event) + } + return event +} + +function hook(name: string, promptId: string, extra: Record = {}) { + return { hook_event_name: name, prompt_id: promptId, session_id: 'session-a', ...extra } +} + +/** The exact sequence Claude Code emits for a successful manual /compact, measured from a PTY-driven + * interactive session (2.1.227). PreCompact and the summarizer's start-less SubagentStop are part + * of the real stream, so they are delivered here even though only PostCompact is registered. */ +function successfulManualCompact(state: HookListenerState) { + deliverIfRegistered(state, hook('PreCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) + deliverIfRegistered( + state, + hook('SubagentStop', COMPACT_PROMPT_ID, { + agent_id: 'a75b38b59774e1f31', + agent_type: '', + background_tasks: [], + session_crons: [] + }) + ) + deliverIfRegistered(state, hook('SessionStart', COMPACT_PROMPT_ID, { source: 'compact' })) + return deliverIfRegistered(state, hook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) +} + +function startTurn(state: HookListenerState) { + deliverIfRegistered(state, hook('UserPromptSubmit', TURN_PROMPT_ID, { prompt: 'do the thing' })) +} + +describe('Claude compact hook registration', () => { + it('registers PostCompact and deliberately does not register PreCompact', () => { + expect(REGISTERED_EVENT_NAMES.has('PostCompact')).toBe(true) + // Why: PreCompact fires BEFORE the compact is validated and an aborted compact emits it alone, + // so subscribing to it at all is what strands the pane. This assertion is the guard. + expect(REGISTERED_EVENT_NAMES.has('PreCompact')).toBe(false) + }) + + it('writes PostCompact, and no PreCompact, into the settings Claude actually reads', () => { + const written = applyManagedHooks( + { hooks: {} }, + { type: 'command', command: 'orca-claude-hook' }, + 'claude-hook.sh' + ) + const postCompact = written.hooks?.PostCompact ?? [] + expect( + postCompact.some((definition) => + (definition.hooks ?? []).some((entry) => entry.command === 'orca-claude-hook') + ) + ).toBe(true) + expect(written.hooks?.PreCompact).toBeUndefined() + }) +}) + +describe('STA-2915: a manual compact clears the pane', () => { + it('ends a stuck working row as a silent done', () => { + const state = createHookListenerState() + startTurn(state) + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('working') + + const completion = successfulManualCompact(state) + + expect(completion?.payload.state).toBe('done') + // Why: a finished compact is a session-shaped boundary, not a completed turn — consumers that + // react to completion (automation runs, unread counts, notifications) must stay out of it. + expect(completion?.payload.sessionBoundary).toBe(true) + }) + + it('keeps the summarizer SubagentStop from being the thing that resolves the pane', () => { + const state = createHookListenerState() + startTurn(state) + // The start-less SubagentStop re-emits the cached lead state; it can only republish `working`, + // never clear it. Only PostCompact resolves the pane. + const republished = deliverIfRegistered( + state, + hook('SubagentStop', COMPACT_PROMPT_ID, { + agent_id: 'a75b38b59774e1f31', + agent_type: '', + background_tasks: [], + session_crons: [] + }) + ) + expect(republished?.payload.state).toBe('working') + }) +}) + +describe('STA-4613: an aborted compact must not strand the pane', () => { + it('leaves an idle pane idle when PreCompact arrives alone', () => { + const state = createHookListenerState() + startTurn(state) + deliverIfRegistered(state, hook('Stop', TURN_PROMPT_ID)) + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('done') + + // Measured: "Not enough messages to compact" emits PreCompact and nothing else. + deliverIfRegistered(state, hook('PreCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) + + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('done') + }) + + it('leaves a working pane working when PreCompact arrives alone', () => { + const state = createHookListenerState() + startTurn(state) + + deliverIfRegistered(state, hook('PreCompact', COMPACT_PROMPT_ID, { trigger: 'manual' })) + + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('working') + }) + + it('does not map PreCompact even when it is delivered past the registration gate', () => { + const state = createHookListenerState() + startTurn(state) + deliverIfRegistered(state, hook('Stop', TURN_PROMPT_ID)) + + const event = normalizeHookPayload( + state, + 'claude', + { + paneKey: PANE_KEY, + payload: hook('PreCompact', COMPACT_PROMPT_ID, { trigger: 'manual' }) + }, + 'production' + ) + + expect(event).toBeNull() + }) +}) + +describe('an auto compact claims nothing', () => { + it('leaves the in-flight turn alone and lets its own Stop close it', () => { + const state = createHookListenerState() + startTurn(state) + + const autoCompact = deliverIfRegistered( + state, + hook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'auto' }) + ) + + expect(autoCompact).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('working') + + deliverIfRegistered(state, hook('Stop', TURN_PROMPT_ID)) + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('done') + }) +}) + +describe('compact completion guards', () => { + it('does not resurrect a pane that was retired before the completion arrived', () => { + const state = createHookListenerState() + // No cached status: the pane was closed/cleared. A compact clears a row, it never creates one. + expect(successfulManualCompact(state)).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)).toBeUndefined() + }) + + it('clears a stuck row restored from disk after a restart, despite a stale connection id', () => { + const state = createHookListenerState() + startTurn(state) + const stuck = state.lastStatusByPaneKey.get(PANE_KEY)! + // What hydration produces after an Orca update: the row survives, its connectionId is the + // PREVIOUS session's, and it is flagged as restored-but-unconfirmed. + state.lastStatusByPaneKey.set(PANE_KEY, { + ...stuck, + connectionId: 'a-previous-connection', + restoredUnconfirmed: true + }) + + const completion = successfulManualCompact(state) + + expect(completion?.payload.state).toBe('done') + expect(completion?.payload.sessionBoundary).toBe(true) + }) + + it('clears a restored row that predates provider-session persistence', () => { + const state = createHookListenerState() + startTurn(state) + const stuck = state.lastStatusByPaneKey.get(PANE_KEY)! + state.lastStatusByPaneKey.set(PANE_KEY, { + ...stuck, + providerSession: undefined, + connectionId: 'a-previous-connection', + restoredUnconfirmed: true + }) + + expect(successfulManualCompact(state)?.payload.state).toBe('done') + }) + + it('does not clear a live turn owned by a different Claude session', () => { + const state = createHookListenerState() + startTurn(state) + + const foreign = normalizeHookPayload( + state, + 'claude', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PostCompact', + prompt_id: COMPACT_PROMPT_ID, + session_id: 'a-different-session', + trigger: 'manual' + } + }, + 'production' + ) + + expect(foreign).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('working') + }) + + it('rejects a compact completion with no provider prompt id', () => { + const state = createHookListenerState() + startTurn(state) + + const event = normalizeHookPayload( + state, + 'claude', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'PostCompact', session_id: 'session-a', trigger: 'manual' } + }, + 'production' + ) + + expect(event).toBeNull() + }) + + it('applies a compact completion once, so a duplicate cannot refresh the row', () => { + const state = createHookListenerState() + startTurn(state) + expect(successfulManualCompact(state)?.payload.state).toBe('done') + + const duplicate = normalizeHookPayload( + state, + 'claude', + { + paneKey: PANE_KEY, + payload: hook('PostCompact', COMPACT_PROMPT_ID, { trigger: 'manual' }) + }, + 'production' + ) + + expect(duplicate).toBeNull() + }) +}) diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index 3447c62ffa5..c6cf3a9b53c 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -88,6 +88,14 @@ export const CLAUDE_EVENTS = [ { eventName: 'PermissionRequest', definition: { matcher: '*', hooks: [{ type: 'command', command: '' }] } + }, + // Why: a manual /compact ends at an idle prompt without emitting Stop, so PostCompact is the only + // signal that can clear the pane (STA-2915). PreCompact is deliberately NOT registered: it fires + // before the compact is validated, and an aborted compact emits it alone — mapping it to 'working' + // would strand the pane exactly as this registration is meant to prevent (STA-4613). + { + eventName: 'PostCompact', + definition: { hooks: [{ type: 'command', command: '' }] } } ] as const diff --git a/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts b/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts index 2b8fc30c817..7e03d75ce7a 100644 --- a/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts +++ b/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts @@ -330,17 +330,23 @@ describe('SshRelaySession agent hooks over a fake relay transport', () => { }) relay.notifyAgentHook(compactEnvelope('UserPromptSubmit', 'working')) + await waitForStatusCount(events, 1) + + // Why: PreCompact fires before the compact is validated, so it may never move the pane — + // over SSH just as locally. relay.notifyAgentHook(compactEnvelope('PreCompact', 'working')) - await waitForStatusCount(events, 2) + await new Promise((resolve) => setImmediate(resolve)) + expect(events).toHaveLength(1) + relay.notifyAgentHook({ ...compactEnvelope('PostCompact', 'done'), providerPromptId: undefined }) await new Promise((resolve) => setImmediate(resolve)) - expect(events).toHaveLength(2) + expect(events).toHaveLength(1) relay.notifyAgentHook(compactEnvelope('PostCompact', 'done')) - await waitForStatusCount(events, 3) + await waitForStatusCount(events, 2) expect(events.at(-1)).toMatchObject({ connectionId: 'conn-compact', @@ -351,13 +357,15 @@ describe('SshRelaySession agent hooks over a fake relay transport', () => { ).toMatchObject({ source: 'claude', providerPromptId: COMPACT_PROMPT_ID, - compactTrigger: undefined, - connectionId: 'conn-compact' + connectionId: 'conn-compact', + // Why: a compact that finished must not read as a completed turn to notification and + // automation consumers, over SSH just as locally. + payload: { sessionBoundary: true } }) relay.notifyAgentHook(compactEnvelope('PostCompact', 'done')) await new Promise((resolve) => setImmediate(resolve)) - expect(events).toHaveLength(3) + expect(events).toHaveLength(2) }) it('clears stamped status on reconnect loss but not final shutdown', async () => { diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 7dc54d820d5..76d89e61687 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -16,7 +16,6 @@ import { HOOK_REQUEST_SLOWLORIS_MS, normalizeHookPayload, readRequestBody, - resolveCachedClaudeCompactOwnership, resolveHookSource, writeEndpointFile, type AgentHookEventPayload, @@ -248,10 +247,7 @@ export class RelayAgentHookServer { res.end() return } - const event = normalizeHookPayload(this.state, source, body, this.env, { - allowUnanchoredPreCompact: true, - allowUnanchoredPostCompact: true - }) + const event = normalizeHookPayload(this.state, source, body, this.env) if (event) { // TODO: once normalizeHookPayload returns validated env/version, drop bodyEnv/bodyVersion and source them from the listener result. const env = hookBodyEnv(body) @@ -286,8 +282,15 @@ export class RelayAgentHookServer { if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) { this.retryScheduler.clearAssistantMessageRetry(event.paneKey) } - const previous = this.state.lastStatusByPaneKey.get(event.paneKey) - const cachedEvent = resolveCachedClaudeCompactOwnership(previous, event) + // Why: a reconnecting client receives the pane's last event as an isReplay rebuild of this + // cache, and after a compact that entry IS the PostCompact. Cache it stripped of its compact + // identity so the replay arrives as an ordinary status row the client applies normally — + // otherwise the only channel that can deliver the clearing `done` to a client that was offline + // during the compact is the one path a compact-event guard would reject. + const cachedEvent = + event.hookEventName === 'PostCompact' + ? { ...event, hookEventName: undefined, compactTrigger: undefined } + : event // Why: delete-then-set makes Map insertion order = recency, so the cap below evicts the longest-idle pane. this.state.lastStatusByPaneKey.delete(event.paneKey) this.state.lastStatusByPaneKey.set(event.paneKey, cachedEvent) diff --git a/src/shared/agent-hook-listener-claude-turn-state.test.ts b/src/shared/agent-hook-listener-claude-turn-state.test.ts index 050a6815910..15f1973187d 100644 --- a/src/shared/agent-hook-listener-claude-turn-state.test.ts +++ b/src/shared/agent-hook-listener-claude-turn-state.test.ts @@ -223,22 +223,23 @@ describe('shared agent-hook-listener', () => { expect(event).toBeNull() }) - it('maps an identity-matched Claude manual compact lifecycle', () => { + it('maps a Claude manual compact lifecycle, ignoring the pre-validation event', () => { normalizeAndAccept(state, 'claude', { hook_event_name: 'UserPromptSubmit', prompt: 'work before compact', prompt_id: CLAUDE_PREVIOUS_PROMPT_ID, session_id: 'session-a' }) + // Why: PreCompact fires before the compact is validated — an aborted compact emits it alone — + // so it is neither registered nor mapped. Only the completion may move the pane. const pre = normalizeAndAccept(state, 'claude', { hook_event_name: 'PreCompact', trigger: 'manual', prompt_id: CLAUDE_PROMPT_ID, session_id: 'session-a' }) - expect(pre).not.toBeNull() - expect(pre!.payload.state).toBe('working') - expect(pre!.payload.agentType).toBe('claude') + expect(pre).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.state).toBe('working') const post = normalizeAndAccept(state, 'claude', { hook_event_name: 'PostCompact', @@ -249,6 +250,7 @@ describe('shared agent-hook-listener', () => { expect(post).not.toBeNull() expect(post!.payload.state).toBe('done') expect(post!.payload.agentType).toBe('claude') + expect(post!.payload.sessionBoundary).toBe(true) }) it('keeps the preceding user prompt on the completed compact row', () => { @@ -258,12 +260,6 @@ describe('shared agent-hook-listener', () => { prompt_id: CLAUDE_PREVIOUS_PROMPT_ID, session_id: 'session-a' }) - normalizeAndAccept(state, 'claude', { - hook_event_name: 'PreCompact', - trigger: 'manual', - prompt_id: CLAUDE_PROMPT_ID, - session_id: 'session-a' - }) const post = normalizeAndAccept(state, 'claude', { hook_event_name: 'PostCompact', trigger: 'manual', diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 933e3de10d8..ac092103070 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -57,7 +57,6 @@ import { import { ORCA_HOOK_PROTOCOL_VERSION } from './agent-hook-types' import { REMOTE_AGENT_HOOK_ENV, type AgentHookSource } from './agent-hook-relay' import { - agentProviderSessionsEqual, extractAgentProviderSession, type AgentProviderSessionMetadata } from './agent-session-resume' @@ -75,6 +74,11 @@ import { resolveGrokChatHistoryPathSync, resolveGrokSessionsDir } from './grok-session-paths' +import { + canAcceptClaudeCompactCompletion, + isClaudeCompactCompletionConsumed, + markClaudeCompactCompletionConsumed +} from './claude-compact-completion' import { sweepStaleAgentHookEndpointTemps } from './agent-hook-endpoint-temp-cleanup' import { classifyTruncatedHookRequest } from './agent-hook-transport-interference' import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit' @@ -142,6 +146,8 @@ export type HookListenerState = { claudeRunningNonAgentTaskPaneKeys: Set /** Panes whose latest authoritative Claude cron inventory still has a scheduled job. */ claudeActiveSessionCronPaneKeys: Set + /** Compact whose completion each pane already applied, so relay duplicates can't refresh the row. */ + claudeConsumedCompactPromptIdByPaneKey: Map /** Live thread-spawn children per Codex pane. */ codexSubagentRosterByPaneKey: Map /** Incremental parent/child rollout cursors for Codex collaboration v2. */ @@ -182,6 +188,7 @@ export function createHookListenerState(): HookListenerState { claudeUnconfirmedRestoredStatusPaneKeys: new Set(), claudeRunningNonAgentTaskPaneKeys: new Set(), claudeActiveSessionCronPaneKeys: new Set(), + claudeConsumedCompactPromptIdByPaneKey: new Map(), codexSubagentRosterByPaneKey: new Map(), codexSubagentTranscriptByPaneKey: new Map(), codexLeadStateByPaneKey: new Map() @@ -368,92 +375,6 @@ export type AgentHookEventPayload = { payload: ParsedAgentStatusPayload } -type ClaudeCompactIdentity = Pick< - AgentHookEventPayload, - | 'source' - | 'connectionId' - | 'hookEventName' - | 'providerPromptId' - | 'compactTrigger' - | 'providerSession' -> - -export function canAcceptClaudeCompactTransition( - previous: AgentHookEventPayload | undefined, - incoming: ClaudeCompactIdentity, - options: { allowUnanchoredPreCompact?: boolean; allowUnanchoredPostCompact?: boolean } = {} -): boolean { - if ( - incoming.source !== 'claude' || - incoming.compactTrigger === undefined || - incoming.providerPromptId === undefined || - (incoming.hookEventName !== 'PreCompact' && incoming.hookEventName !== 'PostCompact') - ) { - return false - } - if (incoming.hookEventName === 'PreCompact' && options.allowUnanchoredPreCompact) { - return true - } - if (incoming.hookEventName === 'PostCompact' && options.allowUnanchoredPostCompact) { - return true - } - if ( - previous?.source !== 'claude' || - previous.payload.agentType !== 'claude' || - previous.connectionId !== incoming.connectionId || - !agentProviderSessionsEqual('claude', previous.providerSession, incoming.providerSession) - ) { - return false - } - if (incoming.hookEventName === 'PostCompact') { - return ( - previous.compactTrigger === incoming.compactTrigger && - previous.providerPromptId === incoming.providerPromptId - ) - } - return incoming.compactTrigger === 'manual' - ? previous.providerPromptId !== undefined - : previous.providerPromptId === incoming.providerPromptId -} - -export function resolveCachedClaudeCompactOwnership( - previous: AgentHookEventPayload | undefined, - incoming: AgentHookEventPayload -): AgentHookEventPayload { - const sameClaudeOwner = - previous?.source === 'claude' && - previous.payload.agentType === 'claude' && - incoming.source === 'claude' && - incoming.payload.agentType === 'claude' && - incoming.connectionId === previous.connectionId && - agentProviderSessionsEqual('claude', previous.providerSession, incoming.providerSession) - ? previous - : undefined - if (incoming.hookEventName === 'PreCompact' && incoming.compactTrigger) { - return sameClaudeOwner?.payload.prompt && incoming.payload.prompt.length === 0 - ? { ...incoming, payload: { ...incoming.payload, prompt: sameClaudeOwner.payload.prompt } } - : incoming - } - if (incoming.hookEventName === 'PostCompact') { - return incoming.compactTrigger ? { ...incoming, compactTrigger: undefined } : incoming - } - const ownsCompact = - sameClaudeOwner?.compactTrigger !== undefined && - sameClaudeOwner.providerPromptId !== undefined && - incoming.providerPromptId === sameClaudeOwner.providerPromptId - if (ownsCompact) { - return { - ...incoming, - compactTrigger: sameClaudeOwner.compactTrigger, - payload: - incoming.payload.prompt.length === 0 && sameClaudeOwner.payload.prompt - ? { ...incoming.payload, prompt: sameClaudeOwner.payload.prompt } - : incoming.payload - } - } - return incoming.compactTrigger ? { ...incoming, compactTrigger: undefined } : incoming -} - // ─── Body parsing ─────────────────────────────────────────────────── export function parseFormEncodedBody(body: string): Record { @@ -2979,19 +2900,21 @@ function normalizeClaudeEvent( (eventName === 'PreToolUse' || eventName === 'PermissionRequest') && isAskUserQuestionTool(eventToolName) const isAskUserQuestion = eventName === 'PreToolUse' && isAskUserQuestionWait - // Why: /compact can take minutes and does not emit Stop. PreCompact marks the pane busy; - // PostCompact clears it so a finished compact cannot leave a sticky working spinner (#11352). + // Why: a manual /compact swallows the turn boundary — it ends at an idle prompt and emits no + // Stop, so PostCompact is the pane's only clearing signal (STA-2915). An auto compact runs INSIDE + // a turn that resumes and emits its own Stop, so it must claim nothing. PreCompact fires before + // the compact is validated (an aborted compact emits it alone), so it is neither registered nor + // mapped — see claude-compact-completion.ts. + const isManualCompactCompletion = eventName === 'PostCompact' && hookPayload.trigger === 'manual' const reportedStateName = eventName === 'UserPromptSubmit' || eventName === 'PostToolUse' || eventName === 'PostToolUseFailure' || - eventName === 'PreCompact' || - (eventName === 'PostCompact' && hookPayload.trigger === 'auto') || (eventName === 'PreToolUse' && !isAskUserQuestion) ? 'working' : eventName === 'PermissionRequest' || isAskUserQuestion ? 'waiting' - : isTurnBoundary || (eventName === 'PostCompact' && hookPayload.trigger === 'manual') + : isTurnBoundary || isManualCompactCompletion ? 'done' : null @@ -3161,6 +3084,10 @@ function normalizeClaudeEvent( stateName: effectiveState, updateToolSnapshot: true, interrupted, + // Why: a finished compact is a session-shaped boundary, not a completed turn. Without this the + // clearing `done` would fire completion notifications, unread counts and automation-run + // completion evidence for work nobody did. + sessionBoundary: isManualCompactCompletion ? true : undefined, turnCompletedAt }) } @@ -4391,8 +4318,7 @@ export function normalizeHookPayload( state: HookListenerState, source: AgentHookSource, body: unknown, - expectedEnv: string, - options: { allowUnanchoredPreCompact?: boolean; allowUnanchoredPostCompact?: boolean } = {} + expectedEnv: string ): AgentHookEventPayload | null { if (typeof body !== 'object' || body === null) { return null @@ -4457,38 +4383,43 @@ export function normalizeHookPayload( (hookPayloadRecord.trigger === 'manual' || hookPayloadRecord.trigger === 'auto') ? hookPayloadRecord.trigger : undefined - const isCompactEvent = eventName === 'PreCompact' || eventName === 'PostCompact' - if (isCompactEvent && compactTrigger === undefined) { + // Why: fail closed for Claude only. A malformed compact payload must not reach the mapping, but + // the old guard was source-BLIND and pre-empted every other provider's normalizer before it ran. + if ( + source === 'claude' && + (eventName === 'PreCompact' || eventName === 'PostCompact') && + compactTrigger === undefined + ) { return null } const previousStatus = state.lastStatusByPaneKey.get(paneKey) - if ( - compactTrigger !== undefined && - !canAcceptClaudeCompactTransition( - previousStatus, - { + const isCompactCompletion = source === 'claude' && eventName === 'PostCompact' + if (isCompactCompletion) { + if ( + isClaudeCompactCompletionConsumed( + state.claudeConsumedCompactPromptIdByPaneKey, + paneKey, + providerPromptId + ) || + !canAcceptClaudeCompactCompletion(previousStatus, { source, connectionId: null, - hookEventName: typeof eventName === 'string' ? eventName : undefined, providerPromptId, - compactTrigger, providerSession: providerSession ?? undefined - }, - { - allowUnanchoredPreCompact: options.allowUnanchoredPreCompact, - allowUnanchoredPostCompact: options.allowUnanchoredPostCompact - } + }) + ) { + return null + } + markClaudeCompactCompletionConsumed( + state.claudeConsumedCompactPromptIdByPaneKey, + paneKey, + providerPromptId ) - ) { - return null - } - if ( - eventName === 'PostCompact' && - compactTrigger !== undefined && - previousStatus?.payload.prompt && - !state.lastPromptByPaneKey.has(paneKey) - ) { - state.lastPromptByPaneKey.set(paneKey, previousStatus.payload.prompt) + // Why: the compact's own event carries no prompt; keep the pane's label from the turn it + // summarized rather than blanking the row as it clears. + if (previousStatus?.payload.prompt && !state.lastPromptByPaneKey.has(paneKey)) { + state.lastPromptByPaneKey.set(paneKey, previousStatus.payload.prompt) + } } const extractedPrompt = extractPromptText(hookPayload as Record) const promptText = extractedPrompt.text diff --git a/src/shared/claude-compact-completion.ts b/src/shared/claude-compact-completion.ts new file mode 100644 index 00000000000..87ea65aa91b --- /dev/null +++ b/src/shared/claude-compact-completion.ts @@ -0,0 +1,102 @@ +// Claude's compact lifecycle, from Orca's side. +// +// Claude emits PreCompact -> (summarizer SubagentStop) -> SessionStart(source=compact) -> +// PostCompact for a successful /compact, but PreCompact ALONE when the compact aborts +// ("Not enough messages to compact"), and post-compact hooks are only reached on the success +// path. So PreCompact proves nothing and Orca deliberately does not register it: mapping it to +// `working` would strand the pane on every aborted compact, which is the bug this file exists to +// fix (STA-2915/STA-4613). +// +// PostCompact is the only compact event that proves something. `manual` ends at an idle prompt and +// is the pane's missing clearing signal; `auto` fires inside a turn that resumes and emits its own +// Stop, so Orca must not touch the state. + +import { agentProviderSessionsEqual } from './agent-session-resume' +import type { AgentProviderSessionMetadata } from './agent-session-resume' +import type { AgentHookSource } from './agent-hook-relay' + +export type ClaudeCompactCompletionOwner = { + source?: AgentHookSource + connectionId: string | null + providerSession?: AgentProviderSessionMetadata + restoredUnconfirmed?: true + payload: { agentType?: string } +} + +export type ClaudeCompactCompletionIncoming = { + source?: AgentHookSource + connectionId: string | null + providerPromptId?: string + providerSession?: AgentProviderSessionMetadata +} + +/** True when a PostCompact may retire the pane's row. + * + * Fails closed on a missing prompt id: the ownership check this replaces required one, and + * duplicate suppression has nothing to key on without it. */ +export function canAcceptClaudeCompactCompletion( + previous: ClaudeCompactCompletionOwner | undefined, + incoming: ClaudeCompactCompletionIncoming +): boolean { + if (incoming.source !== 'claude' || incoming.providerPromptId === undefined) { + return false + } + // Why: a compact CLEARS a pane, it never creates one. An empty cache means the pane was retired + // (closed tab, deleted worktree, explicit clear) and a late completion must not resurrect it. + // The restart case is not this case: hydration restores the stuck row and marks it + // `restoredUnconfirmed`, so it is handled by the branch below. + if (previous === undefined) { + return false + } + if (previous.source !== 'claude' || previous.payload.agentType !== 'claude') { + return false + } + if (previous.restoredUnconfirmed) { + // Why: a hydrated row keeps the PREVIOUS session's connectionId, and rows predating provider + // session persistence have none at all; neither can contradict the live event, so neither may + // veto it. Match on provider session only, and only when the row actually carries one. + return ( + previous.providerSession === undefined || + agentProviderSessionsEqual('claude', previous.providerSession, incoming.providerSession) + ) + } + return ( + previous.connectionId === incoming.connectionId && + agentProviderSessionsEqual('claude', previous.providerSession, incoming.providerSession) + ) +} + +/** The compact whose completion a pane has already applied, so relay duplicates cannot keep + * refreshing the row. Replaces the suppression the deleted ownership cache provided. */ +export function isClaudeCompactCompletionConsumed( + consumedByPaneKey: Map, + paneKey: string, + providerPromptId: string | undefined +): boolean { + return providerPromptId !== undefined && consumedByPaneKey.get(paneKey) === providerPromptId +} + +export function markClaudeCompactCompletionConsumed( + consumedByPaneKey: Map, + paneKey: string, + providerPromptId: string | undefined +): void { + if (providerPromptId !== undefined) { + consumedByPaneKey.set(paneKey, providerPromptId) + } +} + +/** An old relay strips `compactTrigger` from its cached PostCompact before replaying it, so a + * replay arrives tagged PostCompact with no manual/auto discriminator. The baseline mapping it was + * built with is fixed and known — manual produced `done`, auto produced `working` — so the payload + * state stands in for the missing trigger. This substitutes for the trigger only: the caller still + * runs the ownership guard above. */ +export function resolveLegacyCompactTrigger( + compactTrigger: 'manual' | 'auto' | undefined, + payloadState: string +): 'manual' | 'auto' | undefined { + if (compactTrigger !== undefined) { + return compactTrigger + } + return payloadState === 'done' ? 'manual' : undefined +} diff --git a/src/shared/manual-compact-prompt-identity.test.ts b/src/shared/manual-compact-prompt-identity.test.ts deleted file mode 100644 index 9205e8b7a80..00000000000 --- a/src/shared/manual-compact-prompt-identity.test.ts +++ /dev/null @@ -1,235 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { - createHookListenerState, - normalizeHookPayload, - resolveCachedClaudeCompactOwnership, - type AgentHookEventPayload, - type HookListenerState -} from './agent-hook-listener' -import type { AgentHookSource } from './agent-hook-relay' -import { makePaneKey } from './stable-pane-id' - -const LEAF_ID = '11111111-1111-4111-8111-111111111111' -const PANE_KEY = makePaneKey('manual-compact-prompt', LEAF_ID) -const PROMPT_ID_1 = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1' -const PROMPT_ID_2 = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa2' - -function accept( - state: HookListenerState, - source: AgentHookSource, - payload: Record, - paneKey = PANE_KEY -): AgentHookEventPayload | null { - const event = normalizeHookPayload(state, source, { paneKey, payload }, 'production') - if (event) { - state.lastStatusByPaneKey.set( - paneKey, - resolveCachedClaudeCompactOwnership(state.lastStatusByPaneKey.get(paneKey), event) - ) - } - return event -} - -function beginManualCompact( - state: HookListenerState, - promptId: string, - sessionId: string | undefined = 'session-a' -): AgentHookEventPayload | null { - const identity = { - prompt_id: promptId, - ...(sessionId ? { session_id: sessionId } : {}) - } - accept(state, 'claude', { - hook_event_name: 'UserPromptSubmit', - prompt: '/compact', - ...identity - }) - return accept(state, 'claude', { - hook_event_name: 'PreCompact', - trigger: 'manual', - ...identity - }) -} - -function finishManualCompact( - state: HookListenerState, - promptId: string | undefined, - sessionId: string | null = 'session-a' -): AgentHookEventPayload | null { - return accept(state, 'claude', { - hook_event_name: 'PostCompact', - trigger: 'manual', - ...(promptId ? { prompt_id: promptId } : {}), - ...(sessionId ? { session_id: sessionId } : {}) - }) -} - -describe('manual compact prompt identity', () => { - it('settles only the matching Claude manual compact', () => { - const state = createHookListenerState() - - const pre = beginManualCompact(state, PROMPT_ID_1) - const post = finishManualCompact(state, PROMPT_ID_1) - - expect(pre).toMatchObject({ - source: 'claude', - providerPromptId: PROMPT_ID_1, - compactTrigger: 'manual', - hookEventName: 'PreCompact', - payload: { state: 'working', prompt: '/compact', agentType: 'claude' } - }) - expect(post).toMatchObject({ - source: 'claude', - providerPromptId: PROMPT_ID_1, - compactTrigger: 'manual', - hookEventName: 'PostCompact', - payload: { state: 'done', prompt: '/compact', agentType: 'claude' } - }) - }) - - it('rejects stale, duplicate, and session-mismatched completion', () => { - const state = createHookListenerState() - beginManualCompact(state, PROMPT_ID_1) - expect(finishManualCompact(state, PROMPT_ID_1, 'session-b')).toBeNull() - expect(finishManualCompact(state, PROMPT_ID_1)).not.toBeNull() - expect(finishManualCompact(state, PROMPT_ID_1)).toBeNull() - - beginManualCompact(state, PROMPT_ID_2) - expect(finishManualCompact(state, PROMPT_ID_1)).toBeNull() - expect(state.lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - providerPromptId: PROMPT_ID_2, - hookEventName: 'PreCompact', - payload: { state: 'working' } - }) - }) - - it('does not let stale compact events replace intervening provider work', () => { - const state = createHookListenerState() - beginManualCompact(state, PROMPT_ID_1) - expect(finishManualCompact(state, PROMPT_ID_1)).not.toBeNull() - accept(state, 'codex', { - hook_event_name: 'UserPromptSubmit', - prompt: 'new Codex work', - session_id: 'codex-session' - }) - - expect( - accept(state, 'claude', { - hook_event_name: 'PreCompact', - trigger: 'manual', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - ).toBeNull() - expect(finishManualCompact(state, PROMPT_ID_1)).toBeNull() - expect(state.lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - source: 'codex', - payload: { state: 'working', prompt: 'new Codex work', agentType: 'codex' } - }) - }) - - it('fails closed without exact prompt identity and session presence', () => { - const state = createHookListenerState() - expect(beginManualCompact(state, 'not-a-uuid')).toBeNull() - expect(beginManualCompact(state, PROMPT_ID_1)).not.toBeNull() - expect(finishManualCompact(state, undefined)).toBeNull() - expect(finishManualCompact(state, PROMPT_ID_1, null)).toBeNull() - expect(state.lastStatusByPaneKey.get(PANE_KEY)?.hookEventName).toBe('PreCompact') - }) - - it('keeps automatic compact working and ignores unproven Kimi events', () => { - const state = createHookListenerState() - const current = accept(state, 'claude', { - hook_event_name: 'UserPromptSubmit', - prompt: 'continue the task', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - const automaticPre = accept(state, 'claude', { - hook_event_name: 'PreCompact', - trigger: 'auto', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - const automaticPost = accept(state, 'claude', { - hook_event_name: 'PostCompact', - trigger: 'auto', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - const kimiPre = accept(state, 'kimi', { - hook_event_name: 'PreCompact', - trigger: 'manual', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - const kimiPost = accept(state, 'kimi', { - hook_event_name: 'PostCompact', - trigger: 'manual', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - - expect(automaticPre).toMatchObject({ - compactTrigger: 'auto', - hookEventName: 'PreCompact', - payload: { state: 'working' } - }) - expect(automaticPost).toMatchObject({ - compactTrigger: 'auto', - hookEventName: 'PostCompact', - payload: { state: 'working' } - }) - expect(kimiPre).toBeNull() - expect(kimiPost).toBeNull() - expect(state.lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - hookEventName: 'PostCompact', - compactTrigger: undefined, - payload: { state: 'working', prompt: current?.payload.prompt } - }) - }) - - it('does not let stale manual completion retire a newer automatic compact', () => { - const state = createHookListenerState() - beginManualCompact(state, PROMPT_ID_1) - expect( - accept(state, 'claude', { - hook_event_name: 'PreCompact', - trigger: 'auto', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - }) - ).toMatchObject({ compactTrigger: 'auto', payload: { state: 'working' } }) - - expect(finishManualCompact(state, PROMPT_ID_1)).toBeNull() - expect(state.lastStatusByPaneKey.get(PANE_KEY)).toMatchObject({ - compactTrigger: 'auto', - hookEventName: 'PreCompact', - payload: { state: 'working' } - }) - }) - - it('does not allocate ownership for unaccepted untrusted pane keys', () => { - const state = createHookListenerState() - for (let index = 0; index < 4_097; index += 1) { - normalizeHookPayload( - state, - 'claude', - { - paneKey: makePaneKey(`untrusted-${index}`, LEAF_ID), - payload: { - hook_event_name: 'PreCompact', - trigger: 'manual', - prompt_id: PROMPT_ID_1, - session_id: 'session-a' - } - }, - 'production' - ) - } - - expect(state.lastStatusByPaneKey.size).toBe(0) - expect(state.lastPromptByPaneKey.size).toBe(0) - }) -})