diff --git a/src/main/agent-hooks/server-codex-review-turn-end.test.ts b/src/main/agent-hooks/server-codex-review-turn-end.test.ts index b1a0dc190f7e..ff8950e72755 100644 --- a/src/main/agent-hooks/server-codex-review-turn-end.test.ts +++ b/src/main/agent-hooks/server-codex-review-turn-end.test.ts @@ -76,52 +76,59 @@ describe('Codex turn completion without Stop', () => { rmSync(directory, { recursive: true, force: true }) }) - it('settles a /review from its child rollout and leaves later turns working', async () => { - await startHost(false) - const childPath = join(directory, 'review-child.jsonl') - writeFileSync(childPath, `${reviewRecords[0]}\n`) - // Review hooks use the root session with child turn/path and emit no agent_id or Stop. - const review = { transcript_path: childPath, turn_id: 'child-turn' } - await hook({ ...review, hook_event_name: 'UserPromptSubmit', prompt: 'Review changes' }) - for (const hookEventName of ['PreToolUse', 'PostToolUse', 'PreToolUse', 'PostToolUse']) { - await hook({ ...review, hook_event_name: hookEventName, tool_name: 'Bash' }) - } - expect(desktop.getStatusSnapshot()[0]).toMatchObject({ - state: 'working', - providerSession: { id: 'root-session', transcriptPath: childPath } - }) + it.each([false, true])( + 'settles a /review and preserves completion across compaction (relayed=%s)', + async (relayed) => { + await startHost(relayed) + const childPath = join(directory, 'review-child.jsonl') + writeFileSync(childPath, `${reviewRecords[0]}\n`) + // Real review hooks carry the root session ID and child turn/path, with no agent_id or Stop. + const review = { transcript_path: childPath, turn_id: 'child-turn' } + await hook({ ...review, hook_event_name: 'UserPromptSubmit', prompt: 'Review changes' }) + for (const hookEventName of ['PreToolUse', 'PostToolUse', 'PreToolUse', 'PostToolUse']) { + await hook({ ...review, hook_event_name: hookEventName, tool_name: 'Bash' }) + } + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ + state: 'working', + providerSession: { id: 'root-session', transcriptPath: childPath } + }) - appendFileSync(childPath, `${reviewRecords[1]}\n`) - await vi.advanceTimersByTimeAsync(5_000) - expect(desktop.getStatusSnapshot()[0]).toMatchObject({ - state: 'done', - mainAgent: { state: 'done' } - }) - // Desktops that predate host-owned turn verdicts still recognize a labelled Stop. - expect(published.at(-1)).toBe('Stop') + appendFileSync(childPath, `${reviewRecords[1]}\n`) + await vi.advanceTimersByTimeAsync(5_000) + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ + state: 'done', + mainAgent: { state: 'done' } + }) + // Desktops that predate host-owned turn verdicts still recognize a labelled Stop. + expect(published.at(-1)).toBe('Stop') - await hook({ hook_event_name: 'SessionStart', source: 'compact', transcript_path: childPath }) - await vi.advanceTimersByTimeAsync(5_000) - expect(published.at(-1)).toBe('SessionStart') - appendFileSync(childPath, event({ type: 'task_started', turn_id: 'next-turn' })) - await hook({ - ...review, - turn_id: 'next-turn', - hook_event_name: 'UserPromptSubmit', - prompt: 'Next' - }) - await vi.advanceTimersByTimeAsync(5_000) - expect(desktop.getStatusSnapshot()[0]).toMatchObject({ mainAgent: { state: 'working' } }) - await hook({ ...review, hook_event_name: 'PostToolUse', tool_name: 'Bash' }) - expect(desktop.getStatusSnapshot()[0]).toMatchObject({ mainAgent: { state: 'working' } }) + // Neither a compaction queued before the next prompt nor a newer turn replays the review's end. + await hook({ hook_event_name: 'SessionStart', source: 'compact', transcript_path: childPath }) + await vi.advanceTimersByTimeAsync(5_000) + expect(published.at(-1)).toBe('Stop') + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ state: 'done' }) + appendFileSync(childPath, event({ type: 'task_started', turn_id: 'next-turn' })) + await hook({ + ...review, + turn_id: 'next-turn', + hook_event_name: 'UserPromptSubmit', + prompt: 'Next' + }) + await vi.advanceTimersByTimeAsync(5_000) + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ mainAgent: { state: 'working' } }) + // A delayed hook from the review's ended turn does not end the newer turn. + await hook({ ...review, hook_event_name: 'PostToolUse', tool_name: 'Bash' }) + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ mainAgent: { state: 'working' } }) - // A Stop consumed before rollout completion must not be rearmed by compaction. - await hook({ ...review, turn_id: 'next-turn', hook_event_name: 'Stop' }) - await hook({ hook_event_name: 'SessionStart', source: 'compact', transcript_path: childPath }) - appendFileSync(childPath, event({ type: 'task_complete', turn_id: 'next-turn' })) - await vi.advanceTimersByTimeAsync(5_000) - expect(published.slice(-2)).toEqual(['Stop', 'SessionStart']) - }) + // A real Stop that lands before its rollout end, then a compaction, never rearms that turn. + await hook({ ...review, turn_id: 'next-turn', hook_event_name: 'Stop' }) + await hook({ hook_event_name: 'SessionStart', source: 'compact', transcript_path: childPath }) + appendFileSync(childPath, event({ type: 'task_complete', turn_id: 'next-turn' })) + await vi.advanceTimersByTimeAsync(5_000) + expect(published.at(-1)).toBe('Stop') + expect(desktop.getStatusSnapshot()[0]).toMatchObject({ state: 'done' }) + } + ) it.each([ ['locally', 'in an earlier poll', false, true], diff --git a/src/main/runtime/agent-transcript-pane-test-harness.ts b/src/main/runtime/agent-transcript-pane-test-harness.ts index fbde1c071cc4..1bccdc43da91 100644 --- a/src/main/runtime/agent-transcript-pane-test-harness.ts +++ b/src/main/runtime/agent-transcript-pane-test-harness.ts @@ -54,9 +54,10 @@ function transcriptLayoutStore(): RuntimeStore { export async function createTranscriptPane( options: TranscriptPaneOptions, - runtimeDeps?: ConstructorParameters[2] + runtimeDeps?: ConstructorParameters[2], + Runtime: typeof OrcaRuntimeService = OrcaRuntimeService ): Promise<{ runtime: OrcaRuntimeService; handle: string }> { - const runtime = new OrcaRuntimeService( + const runtime = new Runtime( ORCA_LAYOUT_OWNER_IS_MAIN ? transcriptLayoutStore() : null, undefined, runtimeDeps diff --git a/src/shared/agent-hook-listener-test-harness.ts b/src/shared/agent-hook-listener-test-harness.ts index 9cd1c2bbd09e..15930fdc5d2e 100644 --- a/src/shared/agent-hook-listener-test-harness.ts +++ b/src/shared/agent-hook-listener-test-harness.ts @@ -21,3 +21,18 @@ export function normalizeAndAccept( } return event } + +export function codexCompactionSequence(state: 'done' | 'waiting'): Record[] { + return [ + { hook_event_name: 'UserPromptSubmit', turn_id: 'turn-1', prompt: 'Do work' }, + state === 'done' + ? { hook_event_name: 'Stop', turn_id: 'turn-1' } + : { + hook_event_name: 'PreToolUse', + turn_id: 'turn-1', + tool_name: 'request_user_input', + tool_input: { questions: [{ id: 'choice', question: 'Which option?' }] } + }, + { hook_event_name: 'SessionStart', source: 'compact' } + ].map((payload) => ({ session_id: 'compact-session', ...payload })) +} diff --git a/src/shared/agent-hook-listener/providers/codex-compaction-status.test.ts b/src/shared/agent-hook-listener/providers/codex-compaction-status.test.ts new file mode 100644 index 000000000000..7f9d0b47323a --- /dev/null +++ b/src/shared/agent-hook-listener/providers/codex-compaction-status.test.ts @@ -0,0 +1,120 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { appendFileSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { normalizeAndAccept, PANE_KEY } from '../../agent-hook-listener-test-harness' +import { + createHookListenerState, + seedLegacyAgentStatusForTests, + type HookListenerState +} from '../listener-state' +import { pollCodexTranscriptStatus } from './codex-transcript-poll' +import { transcriptPollUpdate } from '../transcript-poll-policy' + +describe('Codex compaction status', () => { + let state: HookListenerState + let directory: string + let transcriptPath: string + + beforeEach(() => { + state = createHookListenerState() + directory = mkdtempSync(join(tmpdir(), 'codex-compaction-')) + transcriptPath = join(directory, 'rollout.jsonl') + writeFileSync(transcriptPath, event('task_started')) + }) + + afterEach(() => rmSync(directory, { recursive: true, force: true })) + + function event(type: string): string { + return `${JSON.stringify({ type: 'event_msg', payload: { type, turn_id: 'turn-1' } })}\n` + } + + function hook(payload: Record) { + return normalizeAndAccept(state, 'codex', { + session_id: 'session-1', + transcript_path: transcriptPath, + ...payload + }) + } + + it.each([undefined, 'turn-1'])( + 'keeps a finished turn done when compact carries turn %s', + (turnId) => { + hook({ hook_event_name: 'UserPromptSubmit', turn_id: 'turn-1', prompt: 'Do work' }) + appendFileSync(transcriptPath, event('task_complete')) + const done = hook({ hook_event_name: 'Stop', turn_id: 'turn-1' }) + + expect( + hook({ hook_event_name: 'SessionStart', source: 'compact', turn_id: turnId }) + ).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)).toBe(done) + expect(state.codexLeadStateByPaneKey.get(PANE_KEY)).toMatchObject({ + state: 'done', + turnId: 'turn-1' + }) + + expect( + hook({ hook_event_name: 'UserPromptSubmit', turn_id: 'turn-2', prompt: 'Next' })?.payload + ).toMatchObject({ state: 'working', prompt: 'Next' }) + } + ) + + it('keeps a compacted parent and live child until their own completions', () => { + hook({ hook_event_name: 'UserPromptSubmit', turn_id: 'turn-1', prompt: 'Do work' }) + const original = hook({ hook_event_name: 'SubagentStart', agent_id: 'child-1' }) + if (!original) { + throw new Error('Expected a working status') + } + const lead = state.codexLeadStateByPaneKey.get(PANE_KEY) + const transcript = state.codexSubagentTranscriptByPaneKey.get(PANE_KEY) + const roster = state.codexSubagentRosterByPaneKey.get(PANE_KEY) + + expect(hook({ hook_event_name: 'SessionStart', source: 'compact' })).toBeNull() + expect(state.codexLeadStateByPaneKey.get(PANE_KEY)).toBe(lead) + expect(state.codexSubagentTranscriptByPaneKey.get(PANE_KEY)).toBe(transcript) + expect(state.codexSubagentRosterByPaneKey.get(PANE_KEY)).toBe(roster) + expect(state.lastStatusByPaneKey.get(PANE_KEY)?.payload.subagents).toEqual([ + expect.objectContaining({ id: 'child-1', state: 'working' }) + ]) + + appendFileSync(transcriptPath, event('task_complete')) + const polled = pollCodexTranscriptStatus(state, original, { paneKey: PANE_KEY }, 'production') + if (!polled) { + throw new Error('Expected transcript completion') + } + expect(transcriptPollUpdate('codex', original, polled)).toMatchObject({ + hookEventName: 'Stop', + payload: { + state: 'working', + prompt: 'Do work', + mainAgent: { state: 'done' }, + subagents: [{ id: 'child-1', state: 'working' }] + } + }) + seedLegacyAgentStatusForTests(state, polled) + expect(state.codexLeadStateByPaneKey.get(PANE_KEY)).toMatchObject({ + state: 'done', + turnId: 'turn-1' + }) + const stopped = hook({ hook_event_name: 'SubagentStop', agent_id: 'child-1' }) + expect(stopped?.payload).toMatchObject({ state: 'done', mainAgent: { state: 'done' } }) + expect(stopped?.payload.subagents).toBeUndefined() + expect( + hook({ hook_event_name: 'UserPromptSubmit', turn_id: 'turn-2', prompt: 'Next' })?.payload + ).toMatchObject({ state: 'working', prompt: 'Next', mainAgent: { state: 'working' } }) + expect(state.codexLeadStateByPaneKey.get(PANE_KEY)?.turnId).toBe('turn-2') + }) + + it('preserves a pending human question across compaction', () => { + hook({ hook_event_name: 'UserPromptSubmit', turn_id: 'turn-1', prompt: 'Do work' }) + const waiting = hook({ + hook_event_name: 'PreToolUse', + turn_id: 'turn-1', + tool_name: 'request_user_input', + tool_input: { questions: [{ id: 'choice', question: 'Which option?' }] } + }) + expect(waiting?.payload.state).toBe('waiting') + expect(hook({ hook_event_name: 'SessionStart', source: 'compact' })).toBeNull() + expect(state.lastStatusByPaneKey.get(PANE_KEY)).toBe(waiting) + }) +}) diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index a1bdac79eed6..c03be28da98f 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -297,6 +297,10 @@ export function codexHookEventForEndedTurn( eventName: unknown, hookPayload: Record ): unknown { + // Compaction continues the same session; it must not reopen a finished turn or erase a live one. + if (eventName === 'SessionStart' && readString(hookPayload, 'source') === 'compact') { + return undefined + } const turnId = readString(hookPayload, 'turn_id') if ( !turnId || diff --git a/tests/e2e/agent-status-codex-compaction.unit.test.ts b/tests/e2e/agent-status-codex-compaction.unit.test.ts new file mode 100644 index 000000000000..f2ecf247616e --- /dev/null +++ b/tests/e2e/agent-status-codex-compaction.unit.test.ts @@ -0,0 +1,89 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { createTestStore, makeTab } from '../../src/renderer/src/store/slices/store-test-helpers' +import { buildWindowApi } from '../../src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures' +import type { AgentStatusSetData } from '../../src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures' +import { + codexCompactionSequence, + normalizeAndAccept, + PANE_KEY +} from '../../src/shared/agent-hook-listener-test-harness' +import { createHookListenerState } from '../../src/shared/agent-hook-listener/listener-state' +import { toAgentStatusIpcPayload } from '../../src/main/agent-hooks/server/server-status-identity' + +afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() + vi.resetModules() +}) + +it.each(['done', 'waiting'] as const)( + 'keeps compacted %s through desktop recovery', + async (state) => { + const now = 1_800_000_000_000 + vi.spyOn(Date, 'now').mockReturnValue(now) + const store = createTestStore() + store.setState({ + workspaceSessionReady: true, + tabsByWorktree: { 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1' })] } + }) + vi.doMock('../../src/renderer/src/store', () => ({ useAppStore: store })) + let listener: ((row: AgentStatusSetData) => void) | undefined + let snapshot: AgentStatusSetData[] = [] + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + listener = cb + return () => { + listener = undefined + } + }, + getSnapshot: async () => snapshot + }) + ) + const { registerAgentStatusIpcBridge } = + await import('../../src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge') + const unsubs: (() => void)[] = [] + let bridge = registerAgentStatusIpcBridge(unsubs) + const stopBridge = () => { + bridge.disposeAsyncState() + bridge.unsubscribeStore() + unsubs.splice(0).forEach((fn) => fn()) + } + try { + const hookState = createHookListenerState() + for (const payload of codexCompactionSequence(state)) { + const event = normalizeAndAccept(hookState, 'codex', payload) + if (!event) { + continue + } + const row = toAgentStatusIpcPayload({ + ...event, + tabId: 'tab-1', + worktreeId: 'wt-1', + receivedAt: now, + stateStartedAt: now + }) + if (row.providerSession?.key !== 'session_id') { + throw new Error('Expected Codex session_id metadata') + } + snapshot = [{ ...row, providerSession: { ...row.providerSession, key: 'session_id' } }] + listener?.(snapshot[0]) + } + const expected = { state, providerSession: { id: 'compact-session' } } + await vi.waitFor(() => + expect(store.getState().agentStatusByPaneKey[PANE_KEY]).toMatchObject(expected) + ) + stopBridge() + store.setState({ agentStatusByPaneKey: {} }) + bridge = registerAgentStatusIpcBridge(unsubs) + await vi.waitFor(() => + expect(store.getState().agentStatusByPaneKey[PANE_KEY]).toMatchObject(expected) + ) + store.getState().captureAllSleepingAgentSessions('quit') + expect(store.getState().sleepingAgentSessionsByPaneKey[PANE_KEY]).toMatchObject(expected) + } finally { + stopBridge() + } + } +) diff --git a/tests/e2e/codex-compaction-consumers.unit.test.ts b/tests/e2e/codex-compaction-consumers.unit.test.ts new file mode 100644 index 000000000000..00fff1f526e7 --- /dev/null +++ b/tests/e2e/codex-compaction-consumers.unit.test.ts @@ -0,0 +1,138 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { AgentHookServer } from '../../src/main/agent-hooks/server' +import { buildBody, postHookEvent } from '../../src/main/agent-hooks/server.test-fixtures' +import { + createTranscriptPane, + waitForTranscriptIdle +} from '../../src/main/runtime/agent-transcript-pane-test-harness' +import { OrcaRuntimeService } from '../../src/main/runtime/orca-runtime' +import type { ResolvedWorktreeSnapshot } from '../../src/main/runtime/runtime-resolved-worktree-cache' +import { storeWorktreeMeta } from '../../src/main/runtime/runtime-store-partial-fixture' +import { codexCompactionSequence } from '../../src/shared/agent-hook-listener-test-harness' +import { buildWorktree } from '../../src/cli/test-fixtures' + +const mocks = vi.hoisted(() => ({ + callMock: vi.fn(), + runtimeClientConstructorMock: vi.fn(), + serveOrcaAppMock: vi.fn(), + getDefaultUserDataPathMock: vi.fn(() => '/tmp') +})) +vi.mock('../../src/cli/runtime-client', async () => { + const { createRuntimeClientModuleMock } = await import('../../src/cli/index-test-harness.js') + return createRuntimeClientModuleMock(mocks) +}) +vi.mock('electron', () => ({ + BrowserWindow: { fromId: vi.fn(() => null) }, + webContents: { fromId: vi.fn(() => null) }, + ipcMain: { on: vi.fn(), removeListener: vi.fn() }, + app: { getPath: vi.fn(() => '/tmp') } +})) +import { main } from '../../src/cli/index' + +class CompactionRuntime extends OrcaRuntimeService { + protected override async listResolvedWorktreeSnapshot(): Promise { + const { git } = buildWorktree('/repo/app', 'feature') + return { + worktrees: [ + { + ...storeWorktreeMeta(), + ...git, + id: 'wt-1', + repoId: 'repo-1', + parentWorktreeId: null, + childWorktreeIds: [], + lineage: null, + git + } + ], + platformByRepoId: new Map() + } + } +} + +afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllEnvs() + vi.unstubAllGlobals() + vi.resetModules() +}) + +it.each(['done', 'waiting'] as const)( + 'keeps compacted %s through runtime consumers and CLI commands', + async (state) => { + const server = new AgentHookServer() + await server.start({ env: 'production' }) + const pane = await createTranscriptPane( + { + paneTitle: 'Codex', + foregroundProcess: 'codex', + launchAgent: 'codex', + data: '• Working (4s • esc to interrupt)\r\n' + }, + { + getAgentStatusSnapshot: () => server.getStatusSnapshot(), + getAgentProviderSessionRowsForPane: () => server.getStatusSnapshot() + }, + CompactionRuntime + ) + const { runtime, handle } = pane + vi.stubEnv('ORCA_PAIRING_CODE', '') + vi.stubEnv('ORCA_ENVIRONMENT', '') + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + const exitCode = process.exitCode + try { + for (const payload of codexCompactionSequence(state)) { + const body = buildBody(payload, { launchToken: 'transcript-launch' }) + expect((await postHookEvent(server, body, '/hook/codex')).status).toBe(204) + } + const tabs = await runtime.listMobileSessionTabs('id:wt-1') + expect(tabs.tabs[0]).toMatchObject({ + agentStatus: { + state, + providerSession: { id: 'compact-session' }, + ...(state === 'waiting' + ? { + toolName: 'request_user_input', + interactivePrompt: expect.stringContaining('Which option?') + } + : {}) + } + }) + expect(runtime.getOrchestrationFleetAgentStatusSnapshot()[0].activity).toMatchObject({ + state, + paneKey: server.getStatusSnapshot()[0].paneKey + }) + expect(runtime.getExactWorkerProviderSession(handle, 0)).toMatchObject({ + agent: 'codex', + providerSession: { id: 'compact-session' } + }) + mocks.callMock.mockImplementation(async (method: string) => { + if (method === 'worktree.ps') { + return { ok: true, result: await runtime.getWorktreePs() } + } + if (method === 'terminal.wait') { + return { ok: true, result: { wait: await waitForTranscriptIdle(pane, 2_000) } } + } + throw new Error(`Unexpected RPC: ${method}`) + }) + await main(['worktree', 'ps', '--json'], '/repo/app') + expect( + JSON.parse(String(log.mock.calls.at(-1)?.[0])).result.worktrees[0].agents[0] + ).toMatchObject({ state, prompt: 'Do work' }) + await main( + ['terminal', 'wait', '--terminal', handle, '--for', 'tui-idle', '--json'], + '/repo/app' + ) + expect(JSON.parse(String(log.mock.calls.at(-1)?.[0]))).toMatchObject( + state === 'done' + ? { result: { wait: { satisfied: true } } } + : { ok: false, error: { message: 'timeout' } } + ) + expect(process.exitCode).toBe(state === 'waiting' ? 1 : exitCode) + } finally { + process.exitCode = exitCode + runtime.onPtyExit('pty-1', 0) + server.stop() + } + } +)