From 3667dc8296a8a714dc963c6025700f6c2e93a30f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 17 Aug 2026 18:04:26 -0700 Subject: [PATCH] fix(agent-status): clear the pane when a Claude compact finishes (STA-2915, STA-4613) A manual /compact ends at an idle prompt without emitting Stop, so nothing in the compact window could ever clear the pane. A worktree that entered the compact `working` stayed `working` until the 30-minute stale sweep -- and the summarizer's start-less SubagentStop kept republishing the row, resetting that clock each time. The correlation added by #12332 was supposed to own this, but it could never run: PreCompact and PostCompact were never added to CLAUDE_EVENTS, so they were never registered with Claude. compactTrigger was always undefined, and the transition guard, the ownership cache, the relay wire field and the ingest branch were all unreachable. Five test files exercised the logic by injecting events past the registration boundary, so the suite stayed green over code that could not execute. Register PostCompact -- and deliberately NOT PreCompact. Measured on Claude Code 2.1.227, a successful manual compact emits PreCompact, a start-less SubagentStop, SessionStart(source=compact), then PostCompact; an ABORTED compact ("Not enough messages to compact") emits PreCompact ALONE. Mapping PreCompact to `working` would strand the pane on every aborted compact, which is the bug being fixed, so the abort guard is structural: Orca never subscribes to the pre-validation event. PostCompact carries its own trigger, so no anchor is needed to tell manual from auto and the correlation machinery is deleted rather than repaired. Manual becomes a `done` with sessionBoundary set -- a finished compact is a session-shaped boundary, not a completed turn, so completion notifications, unread counts and automation-run evidence stay out of it. Auto claims nothing: it runs inside a turn that resumes and emits its own Stop. The source-blind early return that dropped compact events for EVERY provider before its normalizer ran is narrowed to Claude, so it keeps failing closed on a malformed payload without pre-empting other providers. Ownership is kept where the deleted guard had it: a valid provider prompt id is required, a completion clears a row but never creates one (a retired pane must not be resurrected), and a hydrated row is matched on provider session only -- it carries the previous session's connectionId, and older rows carry no session at all, so a strict check would reject the restart case this fixes. A consumed prompt id keeps relay duplicates from refreshing the row. Mixed versions: no new wire field and no new opcode. An older relay normalizes with its own shipped mapping and forwards the event, so ingest drops `auto` envelopes and stamps the boundary on `manual` ones; its replay strips the trigger entirely, so payload state stands in for it while ownership is still enforced. The relay now caches a completion with its compact identity removed, so a client that was offline during the compact still receives the clearing row on reconnect. Tests go red before this change and green after: 6 of 12 in the new registration-gated suite and 5 of 8 in the relay/ingest suite. The harness delivers only events present in CLAUDE_EVENTS, so a fix that is never registered cannot pass -- the failure mode that let the original correlation ship unreachable. --- .../manual-compact-hook-stream.test.ts | 438 ++++++++---------- src/main/agent-hooks/server.ts | 60 ++- .../compact-status-registration.test.ts | 273 +++++++++++ src/main/claude/hook-settings.ts | 8 + ...ay-session-agent-hooks.integration.test.ts | 20 +- src/relay/agent-hook-server.ts | 17 +- ...nt-hook-listener-claude-turn-state.test.ts | 16 +- src/shared/agent-hook-listener.ts | 167 ++----- src/shared/claude-compact-completion.ts | 102 ++++ .../manual-compact-prompt-identity.test.ts | 235 ---------- 10 files changed, 682 insertions(+), 654 deletions(-) create mode 100644 src/main/claude/compact-status-registration.test.ts create mode 100644 src/shared/claude-compact-completion.ts delete mode 100644 src/shared/manual-compact-prompt-identity.test.ts 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) - }) -})