diff --git a/src/main/codex/codex-structured-item-translation.test.ts b/src/main/codex/codex-structured-item-translation.test.ts index f0f843e4ed6..16fff5455f4 100644 --- a/src/main/codex/codex-structured-item-translation.test.ts +++ b/src/main/codex/codex-structured-item-translation.test.ts @@ -195,6 +195,152 @@ describe('codex item bodies', () => { }) }) + it('names a classified read command by its class and keeps the raw command', () => { + expect( + codexItemBody({ + type: 'commandExecution', + id: 'item-read', + command: "sed -n '1,200p' notes.txt", + cwd: '/repo', + status: 'completed', + exitCode: 0, + commandActions: [ + { + type: 'read', + command: "sed -n '1,200p' notes.txt", + name: 'notes.txt', + path: '/repo/notes.txt' + } + ] + }) + ).toEqual({ + kind: 'tool-call', + name: 'read', + input: { + command: "sed -n '1,200p' notes.txt", + cwd: '/repo', + path: '/repo/notes.txt', + name: 'notes.txt' + }, + state: 'completed' + }) + }) + + it('carries a classified search query so the row labels by term, not scan root', () => { + expect( + codexItemBody({ + type: 'commandExecution', + id: 'item-search', + command: 'rg -n --no-heading beta .', + cwd: '/repo', + status: 'inProgress', + commandActions: [ + { type: 'search', command: 'rg -n --no-heading beta .', query: 'beta', path: '.' } + ] + }) + ).toEqual({ + kind: 'tool-call', + name: 'search', + input: { command: 'rg -n --no-heading beta .', cwd: '/repo', query: 'beta', path: '.' }, + state: 'running' + }) + }) + + it('omits a null classified field rather than standing it in as a target', () => { + expect( + codexItemBody({ + type: 'commandExecution', + id: 'item-search-bare', + command: 'rg beta', + cwd: '/repo', + status: 'completed', + exitCode: 0, + commandActions: [{ type: 'search', command: 'rg beta', query: null, path: null }] + }) + ).toEqual({ + kind: 'tool-call', + name: 'search', + input: { command: 'rg beta', cwd: '/repo' }, + state: 'completed' + }) + }) + + it('names a classified listFiles command `list`', () => { + expect( + codexItemBody({ + type: 'commandExecution', + id: 'item-list', + command: 'ls', + cwd: '/repo', + status: 'completed', + exitCode: 0, + commandActions: [{ type: 'listFiles', command: 'ls', path: null }] + }) + ).toEqual({ + kind: 'tool-call', + name: 'list', + input: { command: 'ls', cwd: '/repo' }, + state: 'completed' + }) + }) + + it('skips unclassified actions to reach the first classified one', () => { + expect( + codexItemBody({ + type: 'commandExecution', + id: 'item-piped', + command: 'true && cat a.ts', + cwd: '/repo', + status: 'completed', + exitCode: 0, + commandActions: [ + { type: 'unknown', command: 'true' }, + { type: 'read', command: 'cat a.ts', name: 'a.ts', path: 'a.ts' } + ] + }) + ).toMatchObject({ name: 'read', input: { path: 'a.ts', name: 'a.ts' } }) + }) + + it('falls back to the unclassified shell row for absent or malformed commandActions', () => { + const shellRow = { + kind: 'tool-call', + name: 'shell', + input: { command: 'ls', cwd: '/tmp' }, + state: 'completed' + } + const base = { + type: 'commandExecution', + id: 'item-fallback', + command: 'ls', + cwd: '/tmp', + status: 'completed', + exitCode: 0 + } + + expect(codexItemBody(base)).toEqual(shellRow) + expect(codexItemBody({ ...base, commandActions: null })).toEqual(shellRow) + expect(codexItemBody({ ...base, commandActions: [] })).toEqual(shellRow) + expect( + codexItemBody({ ...base, commandActions: [{ type: 'unknown', command: 'ls' }] }) + ).toEqual(shellRow) + expect(codexItemBody({ ...base, commandActions: 'read' })).toEqual(shellRow) + expect(codexItemBody({ ...base, commandActions: [null, 7, 'read', {}, { type: 5 }] })).toEqual( + shellRow + ) + // The classification table is a Map because an object index answers + // `__proto__`/`constructor` with a truthy non-string tool name. + expect( + codexItemBody({ ...base, commandActions: [{ type: '__proto__', command: 'ls' }] }) + ).toEqual(shellRow) + expect( + codexItemBody({ ...base, commandActions: [{ type: 'constructor', command: 'ls' }] }) + ).toEqual(shellRow) + // The rollout-file shape is a different lane and never reaches app-server. + expect( + codexItemBody({ ...base, parsedCmd: [{ type: 'read', cmd: 'ls', path: 'a.ts' }] }) + ).toEqual(shellRow) + }) + it('accepts snake-case command completion output and preserves blob evidence', () => { const output = 'x'.repeat(1_100_000) const translated = codexJournalItem({ diff --git a/src/main/codex/codex-structured-item-translation.ts b/src/main/codex/codex-structured-item-translation.ts index 19052dca365..e82ba7a3d23 100644 --- a/src/main/codex/codex-structured-item-translation.ts +++ b/src/main/codex/codex-structured-item-translation.ts @@ -175,15 +175,58 @@ export type CodexJournalItem = { handled: boolean } +/** + * Codex's own classification of a shell call: the tool name to show, and the + * fields worth lifting into `input` for the shared label helper (`path` as a + * target, `query` as a search term, `name` as the read target's basename). + * A `Map`, not an object — an object index answers `__proto__` with a truthy + * non-string. Every other action type stays an unclassified `shell` row. + */ +const COMMAND_ACTION_CLASSES = new Map([ + ['read', { name: 'read', keys: ['path', 'name'] }], + ['search', { name: 'search', keys: ['query', 'path'] }], + ['listFiles', { name: 'list', keys: ['path'] }] +]) + +/** The first classified `commandActions` entry; null leaves the row exactly as + * a Codex that sends no classification renders it. */ +function commandActionFacts( + item: CodexThreadItem +): { name: string; fields: Record } | null { + const actions = item.commandActions + if (!Array.isArray(actions)) { + return null + } + for (const action of actions) { + const record = readRecord(action) + const type = readString(record, 'type') + const classified = type === null ? undefined : COMMAND_ACTION_CLASSES.get(type) + if (classified === undefined) { + continue + } + const fields: Record = {} + for (const key of classified.keys) { + const value = readString(record, key) + if (value !== null) { + fields[key] = value + } + } + return { name: classified.name, fields } + } + return null +} + function commandItem(item: CodexThreadItem): CodexJournalItem { const output = readFirstString(item, ['aggregatedOutput', 'aggregated_output']) const bounded = output === null ? null : boundInlineText(output, DEFAULT_JOURNAL_PAYLOAD_LIMITS) + const parsed = commandActionFacts(item) return { body: { kind: 'tool-call', - name: 'shell', + name: parsed?.name ?? 'shell', + // Raw command and cwd stay so the expanded view still shows what ran. input: boundToolInput( - { command: item.command ?? null, cwd: item.cwd ?? null }, + { command: item.command ?? null, cwd: item.cwd ?? null, ...parsed?.fields }, DEFAULT_JOURNAL_PAYLOAD_LIMITS ), state: commandState(item), diff --git a/src/shared/native-chat-tool-summary.test.ts b/src/shared/native-chat-tool-summary.test.ts index 57cc5eff074..5fad8fedaef 100644 --- a/src/shared/native-chat-tool-summary.test.ts +++ b/src/shared/native-chat-tool-summary.test.ts @@ -133,6 +133,26 @@ describe('describeToolInput', () => { 'https://example.com' ) expect(briefToolArg({ cmd: '', query: 'needle' })).toBe('needle') + // Inverted, so the skip is still exercised now that the search keys rank first. + expect(describeToolInput({ query: '', command: 'git status' })).toBe('git status') + expect(briefToolArg({ pattern: ' ', cmd: 'git status' })).toBe('git status') + }) + + it('labels a classified search row by its term, not the command that ran it', () => { + // Codex `commandActions` rows are the only input carrying both keys: the + // search term identifies the row, the raw command stays for the detail view. + const search = { command: 'rg -n --no-heading beta .', cwd: '/repo', query: 'beta', path: '.' } + + expect(describeToolInput(search)).toBe('beta') + expect(briefToolArg(search)).toBe('beta') + expect(toolFilePath(search)).toBeNull() + }) + + it('leaves a command-only input labelled by its command', () => { + // Bash and Codex's unclassified shell rows carry no search key at all. + expect(describeToolInput({ command: 'pnpm test', description: 'Run tests' })).toBe('pnpm test') + expect(briefToolArg({ command: 'pnpm test' })).toBe('pnpm test') + expect(describeToolInput({ cmd: 'git status --short' })).toBe('git status --short') }) }) diff --git a/src/shared/native-chat-tool-summary.ts b/src/shared/native-chat-tool-summary.ts index 9954521bed4..9ab3003938c 100644 --- a/src/shared/native-chat-tool-summary.ts +++ b/src/shared/native-chat-tool-summary.ts @@ -5,8 +5,10 @@ const MAX_PREVIEW_STRING_INPUT = 160 const MAX_PREVIEW_COLLECTION_ITEMS = 8 const MAX_PREVIEW_DEPTH = 2 const MAX_TOOL_RUN_SUMMARY_PARTS = 3 -const PRIMARY_ARG_KEYS = ['command', 'cmd', 'query', 'pattern', 'url', 'description'] as const -const BRIEF_ARG_KEYS = ['command', 'cmd', 'query', 'pattern'] as const +// Search term before command: a classified search row carries both, and the +// term is what identifies it. No other tool input supplies the two together. +const PRIMARY_ARG_KEYS = ['query', 'pattern', 'command', 'cmd', 'url', 'description'] as const +const BRIEF_ARG_KEYS = ['query', 'pattern', 'command', 'cmd'] as const export const MAX_TOOL_DETAIL_LENGTH = 4000 export type ToolInputDisplay = {