diff --git a/cli/src/codex/shared/projection.test.ts b/cli/src/codex/shared/projection.test.ts index 376bcf16..4cbfb630 100644 --- a/cli/src/codex/shared/projection.test.ts +++ b/cli/src/codex/shared/projection.test.ts @@ -5,6 +5,24 @@ import { codexPlanProposalId } from './plan'; import { extractNotifySummary } from '@hapi/protocol/messages'; describe('shared history projection', () => { + it.each([undefined, 'root'])('projects command outcomes for live output and replay (parent: %s)', async parentThreadId => { + const send = vi.fn(); + const session = { getMetadata: () => ({}), sendAgentMessage: send } as unknown as ApiSessionClient; + const projection = new SharedCodexProjection(session, 'thread', async () => {}, parentThreadId); + const items = [ + { id: 'failed', type: 'commandExecution', command: 'sleep 100', status: 'failed', exitCode: -1, durationMs: 2000 }, + { id: 'cancelled', type: 'commandExecution', command: 'sleep 100', status: 'cancelled' }, + { id: 'exit', type: 'commandExecution', command: 'exit 2', status: 'completed', exitCode: 2 }, + { id: 'ok', type: 'commandExecution', command: 'echo ok', status: 'completed', exitCode: 0 } + ]; + for (const item of items) await projection.notification('item/completed', { threadId: 'thread', turnId: 'turn', item }); + const results = () => send.mock.calls.map(([body]) => parentThreadId ? body.message : body).filter(body => body.type === 'tool-call-result'); + expect(results().map(body => body.is_error)).toEqual([true, true, true, false]); + expect(results()[0].output).toMatchObject({ command: 'sleep 100', exit_code: -1, duration_ms: 2000 }); + projection.reset(); send.mockClear(); + await projection.history({ turns: [{ id: 'turn', status: 'completed', items }] }); + expect(results().map(body => body.is_error)).toEqual([true, true, true, false]); + }); it.each([undefined, 'root'])('publishes only final summaries in live output and history replay (parent: %s)', async parentThreadId => { const send = vi.fn(); const session = { getMetadata: () => ({}), sendAgentMessage: send } as unknown as ApiSessionClient; diff --git a/cli/src/codex/shared/projection.ts b/cli/src/codex/shared/projection.ts index 0ba17267..2f417cf4 100644 --- a/cli/src/codex/shared/projection.ts +++ b/cli/src/codex/shared/projection.ts @@ -145,7 +145,10 @@ export class SharedCodexProjection { else if (event.type === 'exec_command_begin' && callId) { send({ type: 'tool-call', name: 'CodexBash', callId, input: event }, key); } else if (event.type === 'exec_command_end' && callId) { - send({ type: 'tool-call-result', callId, output: { ...event, stdout: event.output } }, key); + const failed = ['failed', 'cancelled', 'canceled', 'declined', 'interrupted'].includes(String(event.status)) + || (typeof event.exit_code === 'number' && event.exit_code !== 0) + || Boolean(event.error); + send({ type: 'tool-call-result', callId, output: { ...event, stdout: event.output }, is_error: failed }, key); } else if (event.type === 'patch_apply_begin' && callId) { send({ type: 'tool-call', name: 'CodexPatch', callId, input: { changes: event.changes, auto_approved: event.auto_approved } }, key); } else if (event.type === 'patch_apply_end' && callId) { diff --git a/cli/src/codex/utils/appServerEventConverter.test.ts b/cli/src/codex/utils/appServerEventConverter.test.ts index 3252fa7a..39a6bd99 100644 --- a/cli/src/codex/utils/appServerEventConverter.test.ts +++ b/cli/src/codex/utils/appServerEventConverter.test.ts @@ -215,6 +215,16 @@ describe('AppServerEventConverter', () => { }]); }); + it('preserves command identity when completion arrives without a start', () => { + const converter = new AppServerEventConverter(); + const events = converter.handleNotification('item/completed', { + item: { id: 'late-command', type: 'commandExecution', command: 'node server.js', cwd: '/repo', + status: 'failed', exitCode: -1, durationMs: 16_636_720 } + }); + expect(events).toEqual([expect.objectContaining({ type: 'exec_command_end', call_id: 'late-command', + command: 'node server.js', cwd: '/repo', status: 'failed', exit_code: -1, duration_ms: 16_636_720 })]); + }); + it('maps MCP tool call items', () => { const converter = new AppServerEventConverter(); diff --git a/cli/src/codex/utils/appServerEventConverter.ts b/cli/src/codex/utils/appServerEventConverter.ts index 0e33467b..6b556ed6 100644 --- a/cli/src/codex/utils/appServerEventConverter.ts +++ b/cli/src/codex/utils/appServerEventConverter.ts @@ -1067,6 +1067,9 @@ export class AppServerEventConverter { if (method === 'item/completed') { const meta = this.commandMeta.get(itemId) ?? {}; + // Completion snapshots can arrive without a start (reconnect or late shutdown). + const command = extractCommand(item.command ?? item.cmd ?? item.args); + const cwd = asString(item.cwd ?? item.workingDirectory ?? item.working_directory); const output = asString(item.aggregatedOutput ?? item.aggregated_output ?? item.output ?? item.result ?? item.stdout) ?? this.commandOutputBuffers.get(itemId); const stderr = asString(item.stderr); @@ -1079,6 +1082,8 @@ export class AppServerEventConverter { type: 'exec_command_end', call_id: itemId, ...meta, + ...(command ? { command } : {}), + ...(cwd ? { cwd } : {}), ...(output ? { output } : {}), ...(stderr ? { stderr } : {}), ...(error ? { error } : {}), diff --git a/e2e/late-codex-command-results.spec.ts b/e2e/late-codex-command-results.spec.ts new file mode 100644 index 00000000..71fe9bf4 --- /dev/null +++ b/e2e/late-codex-command-results.spec.ts @@ -0,0 +1,29 @@ +import { expect, test } from '@playwright/test' +import { mkdir } from 'node:fs/promises' +import { join } from 'node:path' + +for (const mobile of [false, true]) { + test(`late command results retain identity, outcome and reliable timing (${mobile ? 'mobile' : 'desktop'})`, async ({ page }, testInfo) => { + test.setTimeout(120_000) + await page.setViewportSize(mobile ? { width: 412, height: 915 } : { width: 1280, height: 1000 }) + await page.goto('/e2e-fixtures/late-codex-command-fixture.html', { timeout: 90_000, waitUntil: 'domcontentloaded' }) + const failed = page.getByTestId('failed') + await expect(failed.getByText('node failed-server.js', { exact: false }).first()).toBeVisible() + await expect(failed.locator('.text-red-600').first()).toBeVisible() + await expect(failed.getByText('4h 37m', { exact: false }).first()).toBeVisible() + await expect(failed.getByText('Started', { exact: true })).toHaveCount(0) + const unknown = page.getByTestId('unknown') + await expect(unknown.getByText('Started', { exact: true })).toHaveCount(0) + await expect(unknown.getByText('Duration', { exact: true })).toHaveCount(0) + await expect(unknown.getByText('0.0s', { exact: true })).toHaveCount(0) + await expect(page.getByTestId('success').getByText('2.5s', { exact: true })).toBeVisible() + const directory = process.env.HAPI_TASK_SCREENSHOTS ?? testInfo.outputDir + await mkdir(directory, { recursive: true }) + await page.screenshot({ path: join(directory, `late-command-${mobile ? 'mobile' : 'desktop'}-${testInfo.project.name}.png`), fullPage: true }) + // Details use the same native duration and still expose the command output. + await failed.getByRole('button').first().click() + await expect(page.getByRole('dialog')).toBeVisible() + await expect(page.getByRole('dialog').getByText('Started', { exact: true })).toHaveCount(0) + await expect(page.getByRole('dialog').getByText('4h 37m', { exact: false }).first()).toBeVisible() + }) +} diff --git a/playwright.late-command.config.ts b/playwright.late-command.config.ts new file mode 100644 index 00000000..d08d909c --- /dev/null +++ b/playwright.late-command.config.ts @@ -0,0 +1,11 @@ +import { defineConfig, devices } from '@playwright/test' +import base from './playwright.config' + +export default defineConfig({ ...base, + testMatch: ['late-codex-command-results.spec.ts'], + projects: [ + { name: 'chromium', use: { ...devices['Desktop Chrome'], launchOptions: { args: ['--no-sandbox'] } } }, + { name: 'firefox', use: { ...devices['Desktop Firefox'] } }, + { name: 'webkit', use: { ...devices['Desktop Safari'] } }, + ] +}) diff --git a/web/e2e-fixtures/late-codex-command-fixture.html b/web/e2e-fixtures/late-codex-command-fixture.html new file mode 100644 index 00000000..f2f80300 --- /dev/null +++ b/web/e2e-fixtures/late-codex-command-fixture.html @@ -0,0 +1,2 @@ + +Late Codex command results
diff --git a/web/e2e-fixtures/late-codex-command-fixture.tsx b/web/e2e-fixtures/late-codex-command-fixture.tsx new file mode 100644 index 00000000..29b479eb --- /dev/null +++ b/web/e2e-fixtures/late-codex-command-fixture.tsx @@ -0,0 +1,38 @@ +import ReactDOM from 'react-dom/client' +import '../src/index.css' +import { normalizeDecryptedMessage } from '../src/chat/normalize' +import { reduceChatBlocks } from '../src/chat/reducer' +import { ToolCard } from '../src/components/ToolCard/ToolCard' +import { I18nProvider } from '../src/lib/i18n-context' +import type { ApiClient } from '../src/api/client' +import type { DecryptedMessage } from '../src/types/api' + +// Recorded wire shape through the real normalization, reducer and ToolCard. +// No Hub, Runner, real agent or production session data is used here. +const variants = [ + { id: 'failed', title: 'Earlier server stopped during archival', output: { status: 'failed', exit_code: -1, duration_ms: 16_636_720 } }, + { id: 'unknown', title: 'Start and duration unavailable', output: { status: 'cancelled', exit_code: -1 } }, + { id: 'success', title: 'Completed command with native duration', output: { status: 'completed', exit_code: 0, duration_ms: 2500 } }, +] + +function Fixture() { + return
+

Archived Codex session

+

Fixture verification · earlier command results received during shutdown

+

Assistant: The requested work is complete.

+ {variants.map(({ id, title, output }) => { + const message: DecryptedMessage = { id, seq: 1, localId: null, createdAt: 1_793_208_961_727, + content: { role: 'agent', content: { type: 'codex', data: { type: 'tool-call-result', callId: id, + output: { type: 'exec_command_end', command: `node ${id}-server.js`, cwd: '/workspace/web', ...output } } } } } + const normalized = normalizeDecryptedMessage(message)! + const block = reduceChatBlocks([normalized], null).blocks[0] + return
+

{title}

+ {block.kind === 'tool-call' && {}} block={block} />} +
+ })} +
+} + +ReactDOM.createRoot(document.getElementById('root')!).render() diff --git a/web/e2e/late-codex-command-session.spec.ts b/web/e2e/late-codex-command-session.spec.ts new file mode 100644 index 00000000..28a3d9e5 --- /dev/null +++ b/web/e2e/late-codex-command-session.spec.ts @@ -0,0 +1,168 @@ +import { expect, test } from '@playwright/test' +import { createServer, type ServerResponse } from 'node:http' +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { randomUUID } from 'node:crypto' +import { installHapiAuth, getHapiBaseUrl } from './helpers/hapi-live' + +test.beforeEach(async ({ page }) => { + await page.addInitScript(() => localStorage.setItem('hapi.fue.v1.rich-composer-mentions', '1')) +}) + +test('stored shutdown result remains identifiable in the archived session UI', async ({ page, request }) => { + const sessionId = process.env.HAPI_LATE_COMMAND_EXISTING_SESSION + test.skip(process.env.HAPI_LIVE !== '1' || !sessionId, 'Requires the session created by this task native test') + test.setTimeout(120_000) + const directory = process.env.HAPI_LATE_COMMAND_TEST_DIR! + const base = process.env.HAPI_LATE_COMMAND_LOCAL_URL ?? getHapiBaseUrl() + const accessToken = process.env.HAPI_ACCESS_TOKEN! + const auth = await request.post(`${base}/api/auth`, { data: { accessToken } }) + expect(auth.ok()).toBe(true) + const headers = { Authorization: `Bearer ${(await auth.json()).token}` } + const session = await request.get(`${base}/api/sessions/${sessionId}`, { headers }) + expect((await session.json()).session.active).toBe(false) + const response = await request.get(`${base}/api/sessions/${sessionId}/messages?limit=200`, { headers }) + const rows = (await response.json()).messages + const end = rows.find((row: { content: { content?: { data?: { output?: { type?: string } } } } }) => + row.content?.content?.data?.output?.type === 'exec_command_end') + expect(end.content.content.data).toMatchObject({ is_error: true, output: { status: 'failed', exit_code: -1 } }) + expect(end.content.content.data.output.duration_ms).toBeGreaterThan(0) + await installHapiAuth(page, base, accessToken) + for (const mobile of [true, false]) { + await page.setViewportSize(mobile ? { width: 412, height: 915 } : { width: 1440, height: 1000 }) + await page.goto(`${base}/sessions/${sessionId}`, { timeout: 90_000, waitUntil: 'domcontentloaded' }) + await expect(page.getByText('This session is inactive.', { exact: false })).toBeVisible() + const card = page.getByRole('button').filter({ hasText: 'LATE_COMMAND_' }).first() + await card.scrollIntoViewIfNeeded() + await expect(card).toBeVisible() + await expect(card.locator('.text-red-600').first()).toBeVisible() + await expect(card.getByText('0.0s', { exact: true })).toHaveCount(0) + await page.screenshot({ path: join(directory, 'screenshots', `native-stored-command-${mobile ? 'mobile' : 'desktop'}.png`), fullPage: true }) + await card.click() + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible() + await expect(dialog.getByText('Duration', { exact: true })).toBeVisible() + await page.screenshot({ path: join(directory, 'screenshots', `native-stored-command-detail-${mobile ? 'mobile' : 'desktop'}.png`), fullPage: true }) + await page.keyboard.press('Escape') + } + await writeFile(join(directory, 'native-stored-evidence.json'), JSON.stringify({ sessionId, result: end }, null, 2)) +}) + +test('archiving a real Codex session preserves a late background command result', async ({ page, request }, testInfo) => { + const directory = process.env.HAPI_LATE_COMMAND_TEST_DIR + test.skip(process.env.HAPI_LIVE !== '1' || !directory, 'Requires an isolated Full task environment and CODEX_HOME') + test.setTimeout(360_000) + const base = process.env.HAPI_LATE_COMMAND_LOCAL_URL ?? getHapiBaseUrl() + const accessToken = process.env.HAPI_ACCESS_TOKEN! + const auth = await request.post(`${base}/api/auth`, { data: { accessToken } }) + expect(auth.ok()).toBe(true) + const headers = { Authorization: `Bearer ${(await auth.json()).token}` } + const api = async (path: string, data?: unknown) => { + const response = data === undefined ? await request.get(`${base}/api${path}`, { headers }) + : await request.post(`${base}/api${path}`, { headers, data, timeout: 90_000 }) + expect(response.ok(), `${path}: ${await response.text()}`).toBe(true) + return response.json() + } + const marker = `LATE_COMMAND_${randomUUID()}` + let requestedCommand = false + const modelRequests: unknown[] = [] + const event = (response: ServerResponse, body: { type: string; [key: string]: unknown }) => + response.write(`event: ${body.type}\ndata: ${JSON.stringify(body)}\n\n`) + const provider = createServer((incoming, response) => { + const chunks: Buffer[] = [] + incoming.on('data', chunk => chunks.push(Buffer.from(chunk))) + incoming.on('end', () => { + if (!incoming.url?.endsWith('/responses')) { response.writeHead(404).end(); return } + const input = JSON.parse(Buffer.concat(chunks).toString()) + modelRequests.push(input) + const id = `resp_${randomUUID()}` + response.writeHead(200, { 'Content-Type': 'text/event-stream' }) + event(response, { type: 'response.created', response: { id } }) + if (!requestedCommand) { + requestedCommand = true + const tool = input.tools?.find((tool: { name?: string }) => tool.name === 'exec_command') + const name = tool?.name ?? 'exec_command' + const item = { type: 'function_call', id: `fc_${randomUUID()}`, call_id: `call_${randomUUID()}`, + name, arguments: JSON.stringify({ cmd: `node -e "console.log('${marker}');setInterval(()=>{},1000)"`, yield_time_ms: 1000, max_output_tokens: 100 }) } + event(response, { type: 'response.output_item.done', item }) + } else { + event(response, { type: 'response.output_item.done', item: { type: 'message', role: 'assistant', + id: `msg_${id}`, content: [{ type: 'output_text', text: 'BACKGROUND_SERVER_READY' }] } }) + } + event(response, { type: 'response.completed', response: { id, usage: { input_tokens: 12, output_tokens: 3, total_tokens: 15 } } }) + response.end() + }) + }) + await new Promise(resolve => provider.listen(0, '127.0.0.1', resolve)) + const port = (provider.address() as { port: number }).port + let sessionId: string | undefined + try { + const home = join(directory!, 'codex-home') + const cwd = join(directory!, `native-command-work-${randomUUID()}`) + await mkdir(home, { recursive: true }); await mkdir(cwd, { recursive: true }) + await writeFile(join(home, 'config.toml'), `model = "mock-model"\nmodel_provider = "mock_provider"\napproval_policy = "never"\nsandbox_mode = "danger-full-access"\ncheck_for_update_on_startup = false\n[model_providers.mock_provider]\nname = "Isolated late command test"\nbase_url = "http://127.0.0.1:${port}/v1"\nwire_api = "responses"\nrequires_openai_auth = false\nsupports_websockets = false\nrequest_max_retries = 0\nstream_max_retries = 0\n[analytics]\nenabled = false\n[feedback]\nenabled = false\n`) + const { machines } = await api('/machines') + expect(machines).toHaveLength(1) + expect(machines[0].active).toBe(true) + const spawn = await api(`/machines/${machines[0].id}/spawn`, { directory: cwd, agent: 'codex', model: 'mock-model', permissionMode: 'yolo' }) + if (spawn.type === 'success') sessionId = spawn.sessionId + else { + expect(spawn.message, JSON.stringify(spawn)).toContain('startup is still unconfirmed') + // The Runner deliberately keeps a slow-starting process tracked. + // Wait for that same session rather than starting a duplicate. + await expect.poll(async () => { + const { sessions } = await api('/sessions') + sessionId = sessions.find((session: { id: string; active: boolean; metadata?: { path?: string } }) => + session.active && session.metadata?.path === cwd)?.id + return Boolean(sessionId) + }, { timeout: 90_000 }).toBe(true) + } + const detail = async () => (await api(`/sessions/${sessionId}`)).session + const messages = async () => (await api(`/sessions/${sessionId}/messages?limit=200`)).messages + await expect.poll(async () => (await detail()).active, { timeout: 40_000 }).toBe(true) + await api(`/sessions/${sessionId}/messages`, { text: 'Start the background server.', localId: randomUUID() }) + await expect.poll(async () => JSON.stringify(await messages()), { timeout: 60_000 }).toContain('BACKGROUND_SERVER_READY') + const original = (await messages()).find((row: { content: { content?: { data?: { type?: string; input?: { command?: string } } } } }) => + row.content?.content?.data?.type === 'tool-call' && row.content.content.data.input?.command?.includes(marker)) + expect(original, 'native command started').toBeTruthy() + for (let i = 0; i < 5; i++) { + await api(`/sessions/${sessionId}/messages`, { text: `Follow-up ${i}`, localId: randomUUID() }) + await expect.poll(async () => (await detail()).thinking, { timeout: 30_000 }).toBe(false) + } + const archived = await request.post(`${base}/api/sessions/${sessionId}/archive`, { headers, data: {}, timeout: 90_000 }) + // Windows cleanup can exceed the existing Hub RPC budget (#2055/#2126). + // Verify the actual lifecycle and persisted result independently of that response. + await testInfo.attach('archive-response', { body: JSON.stringify({ status: archived.status(), body: await archived.text() }), contentType: 'application/json' }) + await expect.poll(async () => (await detail()).active, { timeout: 60_000 }).toBe(false) + const rows = await messages() + const callId = original.content.content.data.callId + const end = rows.find((row: { content: { content?: { data?: { type?: string; callId?: string } } } }) => + row.content?.content?.data?.type === 'tool-call-result' && row.content.content.data.callId === callId) + expect(end, 'shutdown persisted the original command result').toBeTruthy() + expect(end.content.content.data.is_error).toBe(true) + expect(end.content.content.data.output.duration_ms).toBeGreaterThan(0) + await installHapiAuth(page, base, accessToken) + await page.setViewportSize({ width: 412, height: 915 }) + await page.goto(`${base}/sessions/${sessionId}`, { timeout: 90_000, waitUntil: 'domcontentloaded' }) + await expect(page.getByText('This session is inactive.', { exact: false })).toBeVisible() + const card = page.getByRole('button').filter({ hasText: marker }).first() + await expect(card).toBeVisible() + await expect(card.locator('.text-red-600').first()).toBeVisible() + await expect(card.getByText('0.0s', { exact: true })).toHaveCount(0) + await page.screenshot({ path: join(directory!, 'screenshots', 'native-archived-command-mobile.png'), fullPage: true }) + await page.setViewportSize({ width: 1440, height: 1000 }) + await expect(card).toBeVisible() + await page.screenshot({ path: join(directory!, 'screenshots', 'native-archived-command-desktop.png'), fullPage: true }) + await testInfo.attach('native-command-evidence', { body: JSON.stringify({ sessionId, callId, startSeq: original.seq, endSeq: end.seq, + result: end.content.content.data, modelRequests: modelRequests.length }), contentType: 'application/json' }) + await writeFile(join(directory!, 'native-command-evidence.json'), JSON.stringify({ sessionId, callId, startSeq: original.seq, + endSeq: end.seq, archiveStatus: archived.status(), result: end.content.content.data }, null, 2)) + // Keep a cleanup/API failure visible even if the result and UI checks pass. + expect(archived.status()).toBe(200) + } finally { + if (sessionId) await api(`/sessions/${sessionId}/archive`, {}).catch(() => {}) + await writeFile(join(directory!, 'native-model-requests.json'), JSON.stringify(modelRequests, null, 2)) + provider.closeAllConnections() + await new Promise(resolve => provider.close(() => resolve())) + } +}) diff --git a/web/playwright.live.config.ts b/web/playwright.live.config.ts index 8c7efc24..739a9fd3 100644 --- a/web/playwright.live.config.ts +++ b/web/playwright.live.config.ts @@ -5,7 +5,7 @@ const chromePath = process.env.PLAYWRIGHT_CHROME_PATH /** Live hub session tests — no Vite; hits HAPI_URL with HAPI_LIVE=1. */ export default defineConfig({ testDir: './e2e', - testMatch: ['mermaid-lightbox-session.spec.ts', 'latex-bracket-math-session.spec.ts', 'codex-fork-while-running.spec.ts'], + testMatch: ['mermaid-lightbox-session.spec.ts', 'latex-bracket-math-session.spec.ts', 'codex-fork-while-running.spec.ts', 'late-codex-command-session.spec.ts'], timeout: 120_000, expect: { timeout: 20_000 }, fullyParallel: false, diff --git a/web/src/chat/codexCommandResult.test.ts b/web/src/chat/codexCommandResult.test.ts new file mode 100644 index 00000000..1f126c06 --- /dev/null +++ b/web/src/chat/codexCommandResult.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest' +import { normalizeDecryptedMessage } from './normalize' +import { reduceChatBlocks } from './reducer' +import { toolDurationMs } from '@/components/ToolCard/toolDuration' +import { getToolTimingDetails } from '@/components/ToolCard/ToolCard' +import { getToolGroupTiming } from '@/components/ToolCard/ToolGroupCard' +import { reconcileChatBlocks } from './reconcile' +import type { ToolCallBlock } from './types' +import type { DecryptedMessage } from '@/types/api' + +function message(data: unknown, createdAt = 20_000): DecryptedMessage { + return { id: `m-${createdAt}`, seq: createdAt, localId: null, createdAt, + content: { role: 'agent', content: { type: 'codex', data } } } +} + +function reduce(messages: DecryptedMessage[]) { + const normalized = messages.map(normalizeDecryptedMessage).filter(value => value !== null) + const block = reduceChatBlocks(normalized, null).blocks.find(block => block.kind === 'tool-call') + if (!block || block.kind !== 'tool-call') throw new Error('missing command card') + return block.tool +} + +const command = 'bun run vite --host 127.0.0.1 --port 3422' +const result = (output: Record, extra = {}) => message({ + type: 'tool-call-result', callId: 'command', output: { type: 'exec_command_end', command, ...output }, ...extra +}) + +describe('late Codex command results', () => { + it.each([{ status: 'failed' }, { exit_code: -1 }, { status: 'cancelled' }, { status: 'declined' }, { error: 'terminated' }])( + 'preserves native failure without a top-level flag: %j', output => { + expect(reduce([result(output)]).state).toBe('error') + }) + + it('recovers identity and native duration without inventing a start timestamp', () => { + const tool = reduce([result({ exit_code: -1, duration_ms: 16_636_720, cwd: '/repo/web' })]) + expect(tool).toMatchObject({ name: 'CodexBash', input: { command, cwd: '/repo/web' }, startedAt: null }) + expect(toolDurationMs(tool)).toBe(16_636_720) + expect(getToolTimingDetails(tool, 25_000)).toEqual({ startedAt: null, completedAt: 20_000, durationMs: 16_636_720 }) + }) + + it.each([undefined, -1, NaN, Infinity, '100'])('does not manufacture duration from an invalid native value: %s', duration_ms => { + const tool = reduce([result({ status: 'completed', exit_code: 0, duration_ms })]) + expect(tool.state).toBe('completed') + expect(toolDurationMs(tool)).toBeNull() + expect(getToolTimingDetails(tool, 25_000).startedAt).toBeNull() + }) + + it('preserves a genuine zero native duration and explicit error flag', () => { + const tool = reduce([result({ exit_code: 0, duration_ms: 0 }, { is_error: true })]) + expect(tool.state).toBe('error') + expect(toolDurationMs(tool)).toBe(0) + }) + + it.each([false, true])('pairs a start loaded later and preserves native duration (result first: %s)', reverse => { + const start = message({ type: 'tool-call', name: 'CodexBash', callId: 'command', input: { command, cwd: '/original' } }, 10_000) + const end = result({ exit_code: 0, duration_ms: 8_000 }) + const tool = reduce(reverse ? [end, start] : [start, end]) + expect(tool).toMatchObject({ name: 'CodexBash', startedAt: 10_000, completedAt: 20_000, state: 'completed' }) + expect(tool.input).toMatchObject({ cwd: '/original' }) + expect(toolDurationMs(tool)).toBe(8_000) + }) + + it('does not interpret unrelated tool output as a Codex command result', () => { + const tool = reduce([message({ type: 'tool-call-result', callId: 'other', output: { status: 'failed', exit_code: -1, command, duration_ms: 100 } })]) + expect(tool.name).toBe('Tool') + expect(tool.state).toBe('completed') + }) + + it('does not manufacture an aggregate start or duration for a group with an unknown start', () => { + const tool = reduce([result({ exit_code: -1, duration_ms: 10_000 })]) + const block: ToolCallBlock = { kind: 'tool-call', id: 'command', localId: null, createdAt: 20_000, tool, children: [] } + expect(getToolGroupTiming([block], 25_000)).toEqual({ startedAt: null, completedAt: 20_000, durationMs: null, running: false }) + }) + + it('reconciles native timing updates without retaining a stale card', () => { + const tool = reduce([result({ exit_code: 0, duration_ms: 100 })]) + const before: ToolCallBlock = { kind: 'tool-call', id: 'command', localId: null, createdAt: 20_000, tool, children: [] } + const after = { ...before, tool: { ...tool, executionDurationMs: 200 } } + const reconciled = reconcileChatBlocks([after], new Map([[before.id, before]])) + expect(reconciled.blocks[0]).toBe(after) + }) +}) diff --git a/web/src/chat/normalizeAgent.ts b/web/src/chat/normalizeAgent.ts index 87643f84..adeddd7c 100644 --- a/web/src/chat/normalizeAgent.ts +++ b/web/src/chat/normalizeAgent.ts @@ -1265,6 +1265,18 @@ export function normalizeAgentRecord( if (data.type === 'tool-call-result' && typeof data.callId === 'string') { const uuid = asString(data.id) ?? messageId + const command = isObject(data.output) && data.output.type === 'exec_command_end' ? data.output : null + const commandFailed = command !== null && ( + ['failed', 'cancelled', 'canceled', 'declined', 'interrupted'].includes(String(command.status)) + || (typeof command.exit_code === 'number' && command.exit_code !== 0) + || Boolean(command.error) + ) + const duration = command?.duration_ms + const commandExecution = command ? { + input: Object.fromEntries(['command', 'cwd', 'command_actions', 'command_source'] + .filter(key => command[key] !== undefined).map(key => [key, command[key]])), + ...(typeof duration === 'number' && Number.isFinite(duration) && duration >= 0 ? { durationMs: duration } : {}) + } : undefined return { id: messageId, localId, @@ -1275,7 +1287,8 @@ export function normalizeAgentRecord( type: 'tool-result', tool_use_id: data.callId, content: data.output, - is_error: Boolean(data.is_error), + is_error: Boolean(data.is_error) || commandFailed, + ...(commandExecution ? { commandExecution } : {}), uuid, parentUUID: null }], diff --git a/web/src/chat/reconcile.ts b/web/src/chat/reconcile.ts index 601adabd..570eb091 100644 --- a/web/src/chat/reconcile.ts +++ b/web/src/chat/reconcile.ts @@ -217,6 +217,8 @@ function areToolCallsEqual(left: ToolCallBlock, right: ToolCallBlock, childrenSa && left.tool.description === right.tool.description && left.tool.createdAt === right.tool.createdAt && left.tool.startedAt === right.tool.startedAt + && left.tool.startUnavailable === right.tool.startUnavailable + && left.tool.executionDurationMs === right.tool.executionDurationMs && left.tool.completedAt === right.tool.completedAt && left.tool.execStartedAt === right.tool.execStartedAt && left.tool.execCompletedAt === right.tool.execCompletedAt diff --git a/web/src/chat/reducerTimeline.ts b/web/src/chat/reducerTimeline.ts index 3dbe5faf..b19b77df 100644 --- a/web/src/chat/reducerTimeline.ts +++ b/web/src/chat/reducerTimeline.ts @@ -1045,6 +1045,7 @@ export function reduceTimeline( // null exec timestamp is a no-op, leaving exec start unset so // toolDurationMs falls back to hub times on both sides. setEarliestStartedAt(block, msg.createdAt) + if (block.tool.startUnavailable) block.tool = { ...block.tool, startUnavailable: false } setEarliestExecStartedAt(block, msg.agentTimestamp ?? null) if (isSubagentToolName(c.name) && !context.consumedGroupIds.has(msg.id)) { @@ -1099,6 +1100,7 @@ export function reduceTimeline( return permissionFromResult ?? permissionEntry?.permission })() + const existing = toolBlocksById.get(c.tool_use_id) const block = ensureToolBlock(blocks, toolBlocksById, c.tool_use_id, { createdAt: msg.createdAt, invokedAt: msg.invokedAt, @@ -1106,8 +1108,8 @@ export function reduceTimeline( model: msg.model, localId: msg.localId, meta: msg.meta, - name: permissionEntry?.toolName ?? 'Tool', - input: permissionEntry?.input ?? null, + name: permissionEntry?.toolName ?? (c.commandExecution ? 'CodexBash' : 'Tool'), + input: existing?.tool.input ?? permissionEntry?.input ?? c.commandExecution?.input ?? null, description: null, permission // NOTE: no agentTimestamp seed here. execStartedAt must @@ -1121,6 +1123,10 @@ export function reduceTimeline( block.tool = { ...block.tool, + ...(c.commandExecution ? { + executionDurationMs: c.commandExecution.durationMs ?? block.tool.executionDurationMs, + ...(!existing ? { startedAt: null, startUnavailable: true } : {}) + } : {}), result: c.content, completedAt: msg.createdAt, // Only a real Claude timestamp — never the hub receive diff --git a/web/src/chat/types.ts b/web/src/chat/types.ts index 24038e2e..02d133d1 100644 --- a/web/src/chat/types.ts +++ b/web/src/chat/types.ts @@ -80,6 +80,7 @@ export type ToolResult = { tool_use_id: string content: unknown is_error: boolean + commandExecution?: { input: Record; durationMs?: number } uuid: string parentUUID: string | null permissions?: ToolResultPermission @@ -198,6 +199,10 @@ export type ChatToolCall = { createdAt: number startedAt: number | null completedAt: number | null + /** Native command duration, independent of message delivery timing. */ + executionDurationMs?: number + /** A result-only command has no observed invocation timestamp. */ + startUnavailable?: boolean /** * Execution-machine timestamps (from `NormalizedMessage.agentTimestamp`) * for the tool_use/tool_result entries, when available. Kept separate diff --git a/web/src/components/ToolCard/ToolCard.tsx b/web/src/components/ToolCard/ToolCard.tsx index a4b03480..c7529f90 100644 --- a/web/src/components/ToolCard/ToolCard.tsx +++ b/web/src/components/ToolCard/ToolCard.tsx @@ -77,7 +77,7 @@ export function getToolTimingDetails(tool: ChatToolCall, now: number): { const active = tool.state === 'running' const hasExecPair = tool.execStartedAt != null && tool.execCompletedAt != null - const startedAt = active || !hasExecPair + const startedAt = tool.startUnavailable ? null : active || !hasExecPair ? (tool.startedAt ?? tool.createdAt) : tool.execStartedAt const completedAt = active diff --git a/web/src/components/ToolCard/ToolGroupCard.tsx b/web/src/components/ToolCard/ToolGroupCard.tsx index e859c4b2..933dbe0b 100644 --- a/web/src/components/ToolCard/ToolGroupCard.tsx +++ b/web/src/components/ToolCard/ToolGroupCard.tsx @@ -25,7 +25,8 @@ export function getToolGroupTiming(tools: ToolCallBlock[], now: number): { .filter((tool) => tool.tool.state !== 'pending') .map((tool) => tool.tool.startedAt ?? tool.tool.createdAt) .filter((value): value is number => Number.isFinite(value)) - const startedAt = startedValues.length > 0 ? Math.min(...startedValues) : null + const startedAt = !tools.some(tool => tool.tool.startUnavailable) && startedValues.length > 0 + ? Math.min(...startedValues) : null const running = tools.some((tool) => tool.tool.state === 'running') const allFinished = tools.length > 0 && tools.every((tool) => tool.tool.state === 'completed' || tool.tool.state === 'error') const completedValues = allFinished diff --git a/web/src/components/ToolCard/toolDuration.ts b/web/src/components/ToolCard/toolDuration.ts index 699630c1..15163769 100644 --- a/web/src/components/ToolCard/toolDuration.ts +++ b/web/src/components/ToolCard/toolDuration.ts @@ -2,7 +2,9 @@ import type { ChatToolCall } from '@/chat/types' /** * Wall-clock duration of a tool call in milliseconds, or null when it cannot be - * derived. Uses the Claude entry's own execution-machine timestamps + * derived. A validated native command duration takes precedence over delivery + * timing. Result-only commands without one have unknown duration. Otherwise uses + * the Claude entry's own execution-machine timestamps * (`execStartedAt`/`execCompletedAt`) — which reflect the true tool execution * time without the hub receive/queue overhead — but only when *both* are * present. This both-or-neither rule is deliberate: mixing one real Claude @@ -19,6 +21,8 @@ export function toolDurationMs(tool: ChatToolCall): number | null { const useExec = tool.execStartedAt != null && tool.execCompletedAt != null const end = useExec ? tool.execCompletedAt : tool.completedAt if (end == null) return null + if (tool.executionDurationMs != null) return tool.executionDurationMs + if (tool.startUnavailable) return null const start = useExec ? tool.execStartedAt : (tool.startedAt ?? tool.createdAt) if (start == null) return null const duration = end - start