From ca791f40009088ebe6677a770a7d96a4ba0d5862 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Wed, 10 Jun 2026 19:34:27 +0530 Subject: [PATCH] =?UTF-8?q?opentui(v6):=20trust=20gateway=20payload.error?= =?UTF-8?q?=20=E2=80=94=20drop=20client-side=20result=20sniffing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui-opentui/src/logic/store.ts | 25 +------------------------ ui-opentui/src/test/tools.test.tsx | 16 ++++++++++------ 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/ui-opentui/src/logic/store.ts b/ui-opentui/src/logic/store.ts index bebde04741e..eee43e2d68f 100644 --- a/ui-opentui/src/logic/store.ts +++ b/ui-opentui/src/logic/store.ts @@ -228,29 +228,6 @@ function readOptNum(payload: { readonly [k: string]: unknown }, key: string): nu return typeof v === 'number' ? v : undefined } -/** - * A failed tool's failure signal, derived from the raw `result`: hermes tools - * return `{"error": "…"}` JSON on failure, and the gateway never sets a - * top-level `error` on tool.complete — so this is THE live trigger for the - * failed lifecycle state (Epic 2.5). Flattened to one line for the header - * subtitle; absent/empty/non-dict results yield undefined (not failed). - */ -function resultError(result: unknown): string | undefined { - let v: unknown = result - if (typeof v === 'string') { - try { - v = JSON.parse(v) - } catch { - return undefined - } - } - if (!v || typeof v !== 'object' || Array.isArray(v)) return undefined - const e = (v as Record)['error'] - if (typeof e !== 'string') return undefined - const flat = e.replace(/\s+/g, ' ').trim() - return flat ? flat.slice(0, 400) : undefined -} - /** Render a raw tool `result` for display: strings as-is, anything else pretty * JSON — both then go through the same envelope-strip pipeline as result_text. */ function stringifyResult(v: unknown): string | undefined { @@ -673,7 +650,7 @@ export function createSessionStore() { const name = readStr(event.payload, 'name') // explicit payload error wins; else derive from the `{"error": …}` result // convention (the only failure signal the live gateway actually ships). - const error = readStr(event.payload, 'error') ?? resultError(event.payload['result']) + const error = readStr(event.payload, 'error') const summary = readStr(event.payload, 'summary') // `result_text` is verbose-gated, but the raw `result` is ALWAYS sent — // when the verbose text is absent, derive the display body from `result` diff --git a/ui-opentui/src/test/tools.test.tsx b/ui-opentui/src/test/tools.test.tsx index 25663a7926f..7526cb5d2e1 100644 --- a/ui-opentui/src/test/tools.test.tsx +++ b/ui-opentui/src/test/tools.test.tsx @@ -450,21 +450,25 @@ describe('tool lifecycle states — running / done / failed (Epic 2.5)', () => { } }) - test('store: a `{"error": …}` result derives part.error (the gateway never ships a top-level error)', () => { + test('store: part.error comes ONLY from payload.error (gateway owns the result convention)', () => { + // The gateway derives failure from the result convention server-side + // (tui_gateway _tool_error_from_result) and ships payload.error; the + // client must NOT sniff results itself (false positives on tools whose + // legitimate output embeds an "error" key — review finding, Epic 2.5). const store = createSessionStore() seedTool( store, { tool_id: 'l2', name: 'read_file' }, - { tool_id: 'l2', name: 'read_file', result: { error: 'File not found:\n /nope.txt' } } + { tool_id: 'l2', name: 'read_file', error: 'File not found: /nope.txt' } ) const last = store.state.messages[store.state.messages.length - 1] const part = last?.parts?.find((p): p is ToolPartState => p.type === 'tool' && p.id === 'l2') - expect(part?.error).toBe('File not found: /nope.txt') // flattened to one line - // …and a clean result stays un-failed + expect(part?.error).toBe('File not found: /nope.txt') + // a result that EMBEDS an error key, without payload.error, stays un-failed seedTool( store, - { tool_id: 'l3', name: 'terminal' }, - { tool_id: 'l3', name: 'terminal', result: { exit_code: 0, output: 'ok' } } + { tool_id: 'l3', name: 'web_search' }, + { tool_id: 'l3', name: 'web_search', result: { results: [1], error: 'some shards failed' } } ) const part3 = store.state.messages[store.state.messages.length - 1]?.parts?.find( (p): p is ToolPartState => p.type === 'tool' && p.id === 'l3'