fix(native-chat): keep Codex MCP tool identity and web-search results on the row

Four fixes to the Codex MCP / web-search item bodies:

- Drop the title-casing display name. `get_forecast` became `Get Forecast`,
  which no longer matches the raw snake_case identifiers that the diff
  renderer, question parsers, and tool-input previews dispatch on, and does not
  match how the Claude lane or the sibling `shell`/`apply_patch`/`web_search`
  bodies name a tool. The row name is now `server/tool` verbatim, the bare
  `tool` when no server is given, and `mcp` when the item names no tool at all.
  Server-qualifying also stops an MCP tool that happens to be called
  `apply_patch` from hijacking the diff renderer.

- Pass the MCP call's own `arguments` as the tool input instead of wrapping it
  in `{server, tool, arguments}`. Row-label derivation only reads top-level
  keys, so the wrapper degraded every MCP row to a truncated raw JSON blob.
  A non-object `arguments` stays addressable under a key rather than being
  dropped; an absent one becomes null, which labels as empty rather than `{}`.

- Carry a web search's `results` as the call output, bounded like every other
  inline payload and omitted when there are none. They were being dropped
  entirely, which showed less than the generic fallback row it replaced.

- No streaming branches were added for these two item types: the Codex delta
  stream is a closed set of six methods that neither can reach, so such
  branches would be unreachable.
This commit is contained in:
Merge Sim
2026-09-05 01:05:38 -07:00
parent ca439ec7cf
commit 6136dce860
2 changed files with 103 additions and 25 deletions
@@ -305,7 +305,7 @@ describe('codex item bodies', () => {
})
})
it('gives an mcp tool call a typed body with its result as output', () => {
it('gives an mcp tool call a typed body with its own arguments as input', () => {
expect(
codexItemBody({
type: 'mcpToolCall',
@@ -318,22 +318,63 @@ describe('codex item bodies', () => {
})
).toEqual({
kind: 'tool-call',
name: 'Get Forecast',
input: { server: 'weather', tool: 'get_forecast', arguments: { city: 'Oslo' } },
// Server-qualified, and the arguments stay top level so the row label can
// read `query`/`command`/`file_path` out of them.
name: 'weather/get_forecast',
input: { city: 'Oslo' },
state: 'completed',
output: { head: '12C', byteLength: 3, truncated: false, digest: expect.any(String) }
})
})
it('keeps a namespaced mcp tool name byte-identical', () => {
for (const tool of ['mcp__server__tool', 'server/tool', 'ns.tool', 'urn:tool']) {
it('passes an mcp tool name through with no casing transform', () => {
// Downstream dispatch is exact-match on raw identifiers, so every shape —
// bare snake_case included — has to survive byte-identical.
for (const tool of ['get_forecast', 'mcp__server__tool', 'ns.tool', 'urn:tool', 'listTools']) {
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', tool, status: 'inProgress' }),
tool
).toMatchObject({ kind: 'tool-call', name: tool, state: 'running' })
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', server: 'srv', tool, status: 'inProgress' }),
tool
).toMatchObject({ kind: 'tool-call', name: `srv/${tool}`, state: 'running' })
}
})
it('falls back to the bare tool, then to `mcp`, when the item is under-specified', () => {
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 'get_forecast', status: 'inProgress' })
).toMatchObject({ name: 'get_forecast' })
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', server: '', tool: 'ping', status: 'completed' })
).toMatchObject({ name: 'ping' })
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', server: 'weather', status: 'completed' })
).toMatchObject({ name: 'mcp' })
})
it('keeps non-object mcp arguments addressable and absent ones empty', () => {
// `arguments` is arbitrary JSON upstream; a scalar or array must still reach
// the row rather than being dropped or unwrapped into a bare value.
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: 'raw text' })
).toMatchObject({ input: { arguments: 'raw text' } })
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: [1, 2] })
).toMatchObject({ input: { arguments: [1, 2] } })
// Null, not `{}`: an empty object labels the row with a literal `{}`.
expect(codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't' })).toEqual({
kind: 'tool-call',
name: 't',
input: null,
state: 'running'
})
expect(
codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: null })
).toMatchObject({ input: null })
})
it('reports an mcp error as a failed call carrying the server message', () => {
expect(
codexItemBody({
@@ -346,7 +387,7 @@ describe('codex item bodies', () => {
})
).toMatchObject({
kind: 'tool-call',
name: 'Ping',
name: 's/ping',
state: 'failed',
output: { head: 'server unreachable', truncated: false }
})
@@ -379,6 +420,37 @@ describe('codex item bodies', () => {
})
})
it('carries the web search hits as the call output', () => {
const results = [{ title: 'Orca 1.0', url: 'https://example.com/notes' }]
expect(
codexItemBody({
type: 'webSearch',
id: 'w',
query: 'orca release notes',
action: { type: 'search', query: 'orca release notes', queries: null },
results
})
).toMatchObject({
kind: 'tool-call',
name: 'web_search',
state: 'completed',
output: { head: JSON.stringify(results), truncated: false }
})
// Nothing to show is no output block at all, not an empty one.
for (const empty of [undefined, null, []]) {
expect(
codexItemBody({
type: 'webSearch',
id: 'w',
query: 'q',
action: { type: 'search' },
results: empty
}),
String(empty)
).not.toHaveProperty('output')
}
})
it('leaves subagent items on the generic row until a real renderer exists', () => {
expect(
codexJournalItem({
@@ -225,32 +225,34 @@ function fileChangeItem(item: CodexThreadItem): CodexJournalItem {
}
}
/** MCP tool names are namespaced as often as not (`mcp__server__tool`,
* `server/tool`, `ns.tool`) and must reach the row byte-identical; only a bare
* word is title-cased. */
function mcpToolDisplayName(name: string): string {
if (/[:./]|__/.test(name)) {
return name
}
const words = name.split(/[\s_-]+/).filter((word) => word.length > 0)
return words.length === 0
? name
: words.map((word) => word[0]!.toUpperCase() + word.slice(1)).join(' ')
/** The tool name reaches the row verbatim — downstream dispatch (diff renderer,
* question parsers, input previews) matches raw identifiers, so any casing
* transform would silently miss them. `server/` qualifies it so two servers
* exposing the same tool stay distinguishable and neither shadows a built-in. */
function mcpToolCallName(item: CodexThreadItem): string {
const tool = readString(item, 'tool')
const server = readString(item, 'server')
return tool === null ? 'mcp' : server === null ? tool : `${server}/${tool}`
}
/** Row-label derivation only reads top-level keys, so the call's own arguments
* have to be the input itself. `arguments` is arbitrary JSON upstream: a
* non-object stays addressable under a key rather than being dropped, and an
* absent one becomes null, which labels as empty instead of `{}`. */
function mcpToolArguments(value: unknown): unknown {
const plain = typeof value === 'object' && value !== null && !Array.isArray(value)
return plain ? value : value === null || value === undefined ? null : { arguments: value }
}
function mcpToolCallItem(item: CodexThreadItem): CodexJournalItem {
const failure = readString(readRecord(item.error), 'message')
const text = failure ?? readTextContent(readRecord(item.result), 'content')
const bounded = text === null ? null : boundInlineText(text, DEFAULT_JOURNAL_PAYLOAD_LIMITS)
const tool = readString(item, 'tool')
return {
body: {
kind: 'tool-call',
name: tool === null ? 'mcp' : mcpToolDisplayName(tool),
input: boundToolInput(
{ server: item.server ?? null, tool, arguments: item.arguments ?? null },
DEFAULT_JOURNAL_PAYLOAD_LIMITS
),
name: mcpToolCallName(item),
input: boundToolInput(mcpToolArguments(item.arguments), DEFAULT_JOURNAL_PAYLOAD_LIMITS),
state: failure === null ? commandState(item) : 'failed',
...(bounded === null ? {} : { output: bounded.bounded })
},
@@ -259,8 +261,11 @@ function mcpToolCallItem(item: CodexThreadItem): CodexJournalItem {
}
/** `webSearch` carries no status: Codex starts it with an empty query and a null
* action, then completes it with both, so `action` is the completion signal. */
* action, then completes it with both, so `action` is the completion signal.
* The hits arrive on `results` and are the call's output. */
function webSearchItem(item: CodexThreadItem): CodexJournalItem {
const hits = Array.isArray(item.results) && item.results.length > 0 ? item.results : null
const bounded = hits && boundInlineText(JSON.stringify(hits), DEFAULT_JOURNAL_PAYLOAD_LIMITS)
return {
body: {
kind: 'tool-call',
@@ -269,7 +274,8 @@ function webSearchItem(item: CodexThreadItem): CodexJournalItem {
{ query: readString(item, 'query'), action: item.action ?? null },
DEFAULT_JOURNAL_PAYLOAD_LIMITS
),
state: item.action === null || item.action === undefined ? 'running' : 'completed'
state: item.action === null || item.action === undefined ? 'running' : 'completed',
...(bounded === null ? {} : { output: bounded.bounded })
},
handled: true
}