diff --git a/console/web/src/components/chat/ChatView.tsx b/console/web/src/components/chat/ChatView.tsx index 1dbcfb7e8..7af2065cb 100644 --- a/console/web/src/components/chat/ChatView.tsx +++ b/console/web/src/components/chat/ChatView.tsx @@ -38,6 +38,7 @@ import { useConversationsCtxOptional } from '@/lib/conversations-context' import { expandFileMentions, parseFileMentions } from '@/lib/file-mentions' import { formatStopReason } from '@/lib/format-stop-reason' import { newMessageId } from '@/lib/session-id' +import { isEmptyFcallInput } from '@/lib/sessions/entry-mapper' import { cn } from '@/lib/utils' import { fetchDefaultWorkingDir, validateWorkspaceDir } from '@/lib/working-dir' import { @@ -552,7 +553,7 @@ export function ChatView({ if (event.kind === 'fcall-start') { const existing = event.functionCallId ? messagesRef.current.find( - (message) => + (message): message is FunctionCallMessage => message.role === 'function-call' && message.functionCallId === event.functionCallId, ) @@ -564,6 +565,13 @@ export function ChatView({ functionCallId: event.functionCallId, sessionId: event.sessionId, filesystemAccess: event.filesystemAccess, + // The approval record's arguments_excerpt is the only source of + // args when the transcript row degraded to empty/streaming (the + // wrapper payload was dropped or still forming at snapshot time). + ...(isEmptyFcallInput(existing.input) && + !isEmptyFcallInput(event.input) + ? { input: event.input } + : {}), }) return } @@ -1065,12 +1073,23 @@ export function ChatView({ m.functionCallId === event.functionCallId, )?.id if (existing) { + const existingRow = messagesRef.current.find( + (m): m is FunctionCallMessage => + m.role === 'function-call' && m.id === existing, + ) onPatchMessage(conversationId, existing, { pendingApproval: event.pendingApproval, running: !event.pendingApproval, functionCallId: event.functionCallId, sessionId: event.sessionId, filesystemAccess: event.filesystemAccess, + // Backfill args from the approval record's excerpt when + // the row's transcript-derived input degraded to empty. + ...(existingRow && + isEmptyFcallInput(existingRow.input) && + !isEmptyFcallInput(event.input) + ? { input: event.input } + : {}), }) fcallId = existing break diff --git a/console/web/src/components/chat/coder/index.tsx b/console/web/src/components/chat/coder/index.tsx index 1f1557383..607dcd494 100644 --- a/console/web/src/components/chat/coder/index.tsx +++ b/console/web/src/components/chat/coder/index.tsx @@ -7,9 +7,14 @@ import { InfoView } from './InfoView' import { ListFolderView } from './ListFolderView' import { MovePreview, MoveView } from './MoveView' import { + createFileRequestSchema, + deleteFileRequestSchema, isCoderFunction, isCoderMutateFunction, + moveFileRequestSchema, + safeParseRequest, unwrapEnvelope, + updateFileRequestSchema, } from './parsers' import { ReadFileView } from './ReadFileView' import { SearchView } from './SearchView' @@ -76,15 +81,26 @@ function tryRenderPreview( ): React.ReactNode | null { if (!isCoderMutateFunction(message.functionId)) return null const input = unwrapEnvelope(message.input) + // Parse-check BEFORE building an element: the preview components render + // null for an unparseable request, but a non-null element here makes + // FunctionCallCard suppress its raw request pane (the null contract). switch (message.functionId) { case 'coder::create-file': - return + return safeParseRequest(createFileRequestSchema, input) ? ( + + ) : null case 'coder::update-file': - return + return safeParseRequest(updateFileRequestSchema, input) ? ( + + ) : null case 'coder::delete-file': - return + return safeParseRequest(deleteFileRequestSchema, input) ? ( + + ) : null case 'coder::move': - return + return safeParseRequest(moveFileRequestSchema, input) ? ( + + ) : null default: return null } diff --git a/console/web/src/components/chat/harness/index.tsx b/console/web/src/components/chat/harness/index.tsx index 85d0b5eed..2db9e47d2 100644 --- a/console/web/src/components/chat/harness/index.tsx +++ b/console/web/src/components/chat/harness/index.tsx @@ -1,7 +1,12 @@ import { SandboxErrorView } from '@/components/chat/sandbox/ErrorView' import { parseSandboxErrorDisplay } from '@/components/chat/sandbox/parsers' import type { FunctionCallMessage } from '@/types/chat' -import { isHarnessFunction, unwrapEnvelope } from './parsers' +import { + isHarnessFunction, + safeParseRequest, + spawnRequestSchema, + unwrapEnvelope, +} from './parsers' import { SpawnPreview, SpawnView } from './SpawnView' import { SubmitResultView } from './SubmitResultView' @@ -65,7 +70,13 @@ function tryRenderPreview( message: FunctionCallMessage, ): React.ReactNode | null { if (message.functionId !== 'harness::spawn') return null - return + const input = unwrapEnvelope(message.input) + // Parse-check BEFORE building an element: SpawnPreview renders null for + // an unparseable request, but a non-null element here makes + // FunctionCallCard suppress its raw request pane (the null contract). + return safeParseRequest(spawnRequestSchema, input) ? ( + + ) : null } export const HarnessToolView = { diff --git a/console/web/src/components/chat/sandbox/index.tsx b/console/web/src/components/chat/sandbox/index.tsx index 18af92ecd..8a7c4350a 100644 --- a/console/web/src/components/chat/sandbox/index.tsx +++ b/console/web/src/components/chat/sandbox/index.tsx @@ -13,7 +13,12 @@ import { FsSedView } from './FsSedView' import { FsStatView } from './FsStatView' import { FsWriteView } from './FsWriteView' import { ListView } from './ListView' -import { parseSandboxErrorDisplay, unwrapEnvelope } from './parsers' +import { + execRequestSchema, + parseSandboxErrorDisplay, + runRequestSchema, + unwrapEnvelope, +} from './parsers' import { RunPreview, RunView } from './RunView' import { StopView } from './StopView' @@ -118,11 +123,18 @@ function tryRenderPreview( ): React.ReactNode | null { if (!isSandboxFunction(message.functionId)) return null const input = unwrapEnvelope(message.input) + // Parse-check BEFORE building an element: the preview components render + // null for an unparseable request, but a non-null element here makes + // FunctionCallCard suppress its raw request pane (the null contract). switch (message.functionId) { case 'sandbox::exec': - return + return execRequestSchema.safeParse(input).success ? ( + + ) : null case 'sandbox::run': - return + return runRequestSchema.safeParse(input).success ? ( + + ) : null default: return null } diff --git a/console/web/src/components/chat/scrapling/index.tsx b/console/web/src/components/chat/scrapling/index.tsx index 35007a608..6c2b26be7 100644 --- a/console/web/src/components/chat/scrapling/index.tsx +++ b/console/web/src/components/chat/scrapling/index.tsx @@ -6,10 +6,15 @@ import { FetchPreview, FetchView } from './FetchView' import { MarkdownView } from './MarkdownView' import { ExtractView, FindSimilarView, QueryView } from './ParseViews' import { + crawlRequestSchema, ELEMENT_SEARCH_FUNCTION_IDS, FETCH_FUNCTION_IDS, + fetchRequestSchema, isScraplingFunction, SESSION_GATED_FUNCTION_IDS, + safeParseRequest, + screenshotRequestSchema, + sessionOpenRequestSchema, unwrapEnvelope, } from './parsers' import { ScreenshotPreview, ScreenshotView } from './ScreenshotView' @@ -130,21 +135,32 @@ function tryRenderPreview( ): React.ReactNode | null { if (!isScraplingFunction(message.functionId)) return null const input = unwrapEnvelope(message.input) + // Parse-check BEFORE building an element: the preview components render + // null for an unparseable request, but a non-null element here makes + // FunctionCallCard suppress its raw request pane (the null contract). + // SessionFetchPreview is exempt — it renders a header for any input. if (FETCH_FUNCTION_IDS.has(message.functionId)) { - return + return safeParseRequest(fetchRequestSchema, input) ? ( + + ) : null } if (message.functionId === 'scrapling::screenshot') { - return + return safeParseRequest(screenshotRequestSchema, input) ? ( + + ) : null } if (SESSION_GATED_FUNCTION_IDS.has(message.functionId)) { - return message.functionId === 'scrapling::session-open' ? ( - - ) : ( - - ) + if (message.functionId === 'scrapling::session-open') { + return safeParseRequest(sessionOpenRequestSchema, input) ? ( + + ) : null + } + return } if (message.functionId === 'scrapling::crawl') { - return + return safeParseRequest(crawlRequestSchema, input) ? ( + + ) : null } return null } diff --git a/console/web/src/components/chat/shell/__tests__/preview-contract.test.ts b/console/web/src/components/chat/shell/__tests__/preview-contract.test.ts new file mode 100644 index 000000000..f3826aadc --- /dev/null +++ b/console/web/src/components/chat/shell/__tests__/preview-contract.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import type { FunctionCallMessage } from '@/types/chat' +import { ShellToolView } from '../index' + +function pendingCall(functionId: string, input: unknown): FunctionCallMessage { + return { + id: 'm1', + role: 'function-call', + functionId, + input, + pendingApproval: true, + createdAt: 0, + } +} + +/* FunctionCallCard falls back to its raw request pane only when + tryRenderPreview returns null. An element that renders nothing would + leave a held call with no visible arguments at all (MOT-4101), so the + parse decision must happen here, not inside the preview component. */ +describe('ShellToolView.tryRenderPreview null contract', () => { + it('returns an element only when the request parses', () => { + expect( + ShellToolView.tryRenderPreview( + pendingCall('shell::exec', { command: 'ls -la' }), + ), + ).not.toBeNull() + expect( + ShellToolView.tryRenderPreview( + pendingCall('shell::kill', { job_id: 'job-1' }), + ), + ).not.toBeNull() + }) + + it('returns null for empty or unparseable requests', () => { + expect( + ShellToolView.tryRenderPreview(pendingCall('shell::exec', {})), + ).toBeNull() + // A raw string (unrecovered stringified payload) must not parse. + expect( + ShellToolView.tryRenderPreview(pendingCall('shell::exec', 'ls -la')), + ).toBeNull() + // argv-as-command is rejected by the schema (server rejects it too). + expect( + ShellToolView.tryRenderPreview( + pendingCall('shell::exec', { command: ['ls', '-la'] }), + ), + ).toBeNull() + expect( + ShellToolView.tryRenderPreview(pendingCall('shell::exec_bg', {})), + ).toBeNull() + expect( + ShellToolView.tryRenderPreview(pendingCall('shell::kill', {})), + ).toBeNull() + }) +}) diff --git a/console/web/src/components/chat/shell/index.tsx b/console/web/src/components/chat/shell/index.tsx index 6b31a923e..77ebf1cf4 100644 --- a/console/web/src/components/chat/shell/index.tsx +++ b/console/web/src/components/chat/shell/index.tsx @@ -18,6 +18,9 @@ import { ShellListView } from './ListView' import { isShellFunction, parseShellErrorDisplay, + shellExecBgRequestSchema, + shellExecRequestSchema, + shellKillRequestSchema, unwrapEnvelope, } from './parsers' import { ShellStatusView } from './StatusView' @@ -105,13 +108,24 @@ function tryRenderPreview( ): React.ReactNode | null { if (!isShellFunction(message.functionId)) return null const input = unwrapEnvelope(message.input) + // Honor the documented null contract BEFORE building an element: the + // preview components render null for an unparseable request, but a + // non-null element here makes FunctionCallCard treat the preview as + // present and suppress its raw request pane — a held call would show + // no arguments at all. switch (message.functionId) { case 'shell::exec': - return + return shellExecRequestSchema.safeParse(input).success ? ( + + ) : null case 'shell::exec_bg': - return + return shellExecBgRequestSchema.safeParse(input).success ? ( + + ) : null case 'shell::kill': - return + return shellKillRequestSchema.safeParse(input).success ? ( + + ) : null default: return null } diff --git a/console/web/src/components/chat/web/index.tsx b/console/web/src/components/chat/web/index.tsx index ebf7ba4ec..0268bdf12 100644 --- a/console/web/src/components/chat/web/index.tsx +++ b/console/web/src/components/chat/web/index.tsx @@ -2,7 +2,12 @@ import { SandboxErrorView } from '@/components/chat/sandbox/ErrorView' import { parseSandboxErrorDisplay } from '@/components/chat/sandbox/parsers' import type { FunctionCallMessage } from '@/types/chat' import { FetchPreview, FetchView } from './FetchView' -import { isWebFunction, unwrapEnvelope } from './parsers' +import { + fetchRequestSchema, + isWebFunction, + safeParseRequest, + unwrapEnvelope, +} from './parsers' export function WebFunctionIdLabel({ functionId }: { functionId: string }) { if (!functionId.startsWith('web::')) { @@ -48,9 +53,14 @@ function tryRenderPreview( ): React.ReactNode | null { if (!isWebFunction(message.functionId)) return null const input = unwrapEnvelope(message.input) + // Parse-check BEFORE building an element: FetchPreview renders null for + // an unparseable request, but a non-null element here makes + // FunctionCallCard suppress its raw request pane (the null contract). switch (message.functionId) { case 'web::fetch': - return + return safeParseRequest(fetchRequestSchema, input) ? ( + + ) : null default: return null } diff --git a/console/web/src/lib/sessions/entry-mapper.test.ts b/console/web/src/lib/sessions/entry-mapper.test.ts index 25eec48c1..766b0db5f 100644 --- a/console/web/src/lib/sessions/entry-mapper.test.ts +++ b/console/web/src/lib/sessions/entry-mapper.test.ts @@ -5,6 +5,7 @@ import { applyFcallPatch, clearTransientFlags, entrySegments, + isEmptyFcallInput, splitReactionTask, transcriptToMessages, triggerFiredSummary, @@ -271,6 +272,67 @@ describe('entrySegments', () => { }) }) + // The harness (`plan_calls`, harness/src/policy.rs) executes flattened + // wrapper args and recovers stringified payloads; the mapper must mirror + // it or the request pane reads `empty` for a call that visibly ran with + // arguments (MOT-4101). + it('hoists flattened wrapper arguments when payload is missing', () => { + const [call] = entrySegments( + assistantItem('e-a', [ + { + type: 'function_call', + id: 'fc-1', + function_id: 'agent_trigger', + arguments: { + function: 'shell::exec', + command: 'cargo fmt', + cwd: '/repo', + }, + }, + ]), + ) + expect(call).toMatchObject({ + functionId: 'shell::exec', + input: { command: 'cargo fmt', cwd: '/repo' }, + }) + }) + + it('recovers a stringified JSON-object payload, tolerating trailing braces', () => { + const parsed = entrySegments( + assistantItem('e-a', [ + { + type: 'function_call', + id: 'fc-1', + function_id: 'agent_trigger', + arguments: { + function: 'state::set', + payload: '{"key":"k","value":1}}', + }, + }, + ]), + )[0] + expect(parsed).toMatchObject({ + functionId: 'state::set', + input: { key: 'k', value: 1 }, + }) + + // A scalar string is NOT recovered — it stays visible as-is. + const scalar = entrySegments( + assistantItem('e-b', [ + { + type: 'function_call', + id: 'fc-2', + function_id: 'agent_trigger', + arguments: { function: 'state::get', payload: 'not json' }, + }, + ]), + )[0] + expect(scalar).toMatchObject({ + functionId: 'state::get', + input: 'not json', + }) + }) + it('maps a compaction custom entry to the compaction marker', () => { const [marker] = entrySegments({ entry_id: 'e-c', @@ -674,6 +736,53 @@ describe('applyEntryUpsert', () => { }) }) + it('keeps patched-in arguments when a snapshot re-derives with empty input', () => { + // The row was patched with the approval record's arguments_excerpt while + // the transcript block's wrapper carried no payload. + const local: Message = { + id: 'e-a:0', + role: 'function-call', + functionId: 'shell::exec', + input: { command: 'ls -la' }, + pendingApproval: true, + functionCallId: 'fc-1', + createdAt: 0, + } + const next = applyEntryUpsert( + [local], + assistantItem('e-a', [ + { + type: 'function_call', + id: 'fc-1', + function_id: 'agent_trigger', + arguments: { function: 'shell::exec' }, + }, + ]), + ) + expect(next[0]).toMatchObject({ + id: 'e-a:0', + input: { command: 'ls -la' }, + pendingApproval: true, + }) + + // A snapshot that DOES carry arguments stays the source of truth. + const richer = applyEntryUpsert( + [local], + assistantItem('e-a', [ + { + type: 'function_call', + id: 'fc-1', + function_id: 'agent_trigger', + arguments: { + function: 'shell::exec', + payload: { command: 'ls -la /tmp' }, + }, + }, + ]), + ) + expect(richer[0]).toMatchObject({ input: { command: 'ls -la /tmp' } }) + }) + it('infers running for unpaired calls while the session is working', () => { const messages = applyEntryUpsert( [], @@ -817,6 +926,20 @@ describe('transcriptToMessages — running inference on hydration', () => { }) }) +describe('isEmptyFcallInput', () => { + it('treats absent/empty/streaming-only inputs as empty, real args as not', () => { + expect(isEmptyFcallInput(undefined)).toBe(true) + expect(isEmptyFcallInput(null)).toBe(true) + expect(isEmptyFcallInput('')).toBe(true) + expect(isEmptyFcallInput([])).toBe(true) + expect(isEmptyFcallInput({})).toBe(true) + expect(isEmptyFcallInput({ _streaming: '{"comm' })).toBe(true) + expect(isEmptyFcallInput({ command: 'ls' })).toBe(false) + expect(isEmptyFcallInput('raw text')).toBe(false) + expect(isEmptyFcallInput(0)).toBe(false) + }) +}) + describe('applyFcallPatch / clearTransientFlags', () => { it('patches the row matching functionCallId and reports found', () => { const messages = transcriptToMessages([ diff --git a/console/web/src/lib/sessions/entry-mapper.ts b/console/web/src/lib/sessions/entry-mapper.ts index d1c2416be..64aad9785 100644 --- a/console/web/src/lib/sessions/entry-mapper.ts +++ b/console/web/src/lib/sessions/entry-mapper.ts @@ -233,6 +233,66 @@ function lifecycleNotice( return null } +/** + * Recover a JSON object from a stringified `payload`. Mirrors the harness's + * leading-value parse (`plan_calls`, harness/src/policy.rs): models that + * stringify the payload also tend to append stray closing braces, which a + * strict whole-string parse rejects as trailing data — so parse the balanced + * `{…}` prefix instead. Only objects are recovered (the schema says object); + * anything else returns undefined and the caller keeps the original string. + */ +function parseJsonObjectPrefix(s: string): unknown | undefined { + const t = s.trim() + if (!t.startsWith('{')) return undefined + let depth = 0 + let inString = false + let escaped = false + for (let i = 0; i < t.length; i++) { + const c = t[i] + if (inString) { + if (escaped) escaped = false + else if (c === '\\') escaped = true + else if (c === '"') inString = false + continue + } + if (c === '"') inString = true + else if (c === '{') depth++ + else if (c === '}') { + depth-- + if (depth === 0) { + try { + return JSON.parse(t.slice(0, i + 1)) as unknown + } catch { + return undefined + } + } + } + } + return undefined +} + +/** + * The arguments a resolved wrapper call actually ran with — mirrors the + * harness's `plan_calls` unwrapping (harness/src/policy.rs) so the request + * pane shows what executed: + * - `payload` object → verbatim; + * - `payload` stringified JSON object → parsed (models double-encode); + * - no `payload` key → the model flattened the target's arguments beside + * `function`; the harness hoists them, so dropping them here would render + * a call that visibly ran with arguments as `request · empty`. + */ +function wrapperInput(args: Record): unknown { + if ('payload' in args) { + const payload = args.payload + if (typeof payload === 'string') { + return parseJsonObjectPrefix(payload) ?? payload + } + return payload ?? {} + } + const { function: _target, ...rest } = args + return rest +} + /** The harness wraps every tool call in agent_trigger; unwrap for display. */ function unwrapFunctionCall( block: Extract, @@ -257,7 +317,10 @@ function unwrapFunctionCall( if (streaming !== undefined) { return { functionId: args.function, input: { _streaming: streaming } } } - return { functionId: args.function, input: args.payload ?? {} } + return { + functionId: args.function, + input: wrapperInput(args as Record), + } } if (streaming !== undefined) { return { @@ -568,6 +631,23 @@ function belongsToEntry(messageId: string, entryId: string): boolean { return messageId === entryId || messageId.startsWith(`${entryId}:`) } +/** + * True when a function-call `input` carries no reviewable arguments: absent, + * empty (`''`/`[]`/`{}`), or only the transient `_streaming` tail. Decides + * when a better source — the approval record's `arguments_excerpt`, a row + * already patched with it — should win over a degraded transcript snapshot. + */ +export function isEmptyFcallInput(v: unknown): boolean { + if (v === null || v === undefined) return true + if (typeof v === 'string') return v.length === 0 + if (Array.isArray(v)) return v.length === 0 + if (typeof v === 'object') { + const keys = Object.keys(v as Record) + return keys.length === 0 || keys.every((k) => k === '_streaming') + } + return false +} + /** Patchable transient state for a function-call row. */ export type FcallPatch = Partial< Pick< @@ -659,6 +739,13 @@ export function applyEntryUpsert( absorbedLocalIds.add(existing.id) return { ...segment, + // A degraded snapshot (streaming tail, dropped wrapper payload) must + // not clobber arguments the row already shows — e.g. the approval + // record's excerpt patched in while the call was held, or a complete + // snapshot arriving before a late redelivery (events are unordered). + ...(isEmptyFcallInput(segment.input) && !isEmptyFcallInput(existing.input) + ? { input: existing.input } + : {}), output: existing.output, durationMs: existing.durationMs, running: existing.running, diff --git a/console/web/src/types/chat.ts b/console/web/src/types/chat.ts index 8eecb6ced..f3d3b7442 100644 --- a/console/web/src/types/chat.ts +++ b/console/web/src/types/chat.ts @@ -199,6 +199,9 @@ export interface MessagePatch { streaming?: boolean durationMs?: number running?: boolean + /** Function-call arguments — patched from the approval record's + `arguments_excerpt` when the transcript-derived input is empty. */ + input?: unknown output?: unknown pendingApproval?: boolean /** Set during fcall-start dedupe so resolve handlers know which iii call to resolve. */