From 6af7a38684b9f8dbc753aa25657b35e1aeadd604 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 12 Sep 2026 03:59:43 +0300 Subject: [PATCH] =?UTF-8?q?fix(bridge):=20gate=20tool-input-available=20on?= =?UTF-8?q?=20clean=20execution=20=E2=80=94=20invalid=20tool=20inputs=20ne?= =?UTF-8?q?ver=20reach=20the=20dock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live crash (modal-realism session): GLM double-encoded the quiz options array as a JSON STRING inside the tool-call arguments. The bridge emitted tool-input-available at toolcall_end — BEFORE validation runs — so the client briefly saw the malformed quiz as a pending question and the dock crashed in normalizeOptions ((string).map). The later tool-input-error chunk could not revive the tree. The old SDK gated this with InvalidToolInputError: validation ran before the input-available chunk. The bridge now holds the part in input-streaming until tool_execution_end and only then emits tool-input-available (clean) or tool-input-error (invalid, structural JSON Schema failure or the zod re-run inside execute). Server-side behavior was already correct (the record shows the error part + a successful model retry) — this is purely the wire-ordering hole. Pinned in chat-flow.test.ts: the refinement-layer retry (correctAnswer mismatch) and the exact live structural shape (string options) both assert the invalid call never becomes input-available on the wire. Gates: tsc, lint, build, 306/306. --- src/server/pi/ui-stream.ts | 42 +++++++++++++----------- tests/chat-flow.test.ts | 66 +++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 19 deletions(-) diff --git a/src/server/pi/ui-stream.ts b/src/server/pi/ui-stream.ts index 66f8725..257952b 100644 --- a/src/server/pi/ui-stream.ts +++ b/src/server/pi/ui-stream.ts @@ -17,9 +17,12 @@ import { QUESTION_TOOLS } from "../../core/pending.ts"; * NEVER tool-output-available (the client throws on an output for a call it * owns). The synthetic "question posed" result lives in memory only; the * record's part stays output-less until the learner answers. - * - Errored calls (authoring validation) surface as tool-input-error — the - * exact part shape the SDK produced for InvalidToolInputError (the - * output-error state the chat flow asserts). + * - tool-input-available is emitted ONLY after the call cleared validation + * at the boundary (clean tool_execution_end) — the SDK's + * InvalidToolInputError gating. An invalid call surfaces as + * tool-input-error instead and NEVER becomes input-available, so a + * malformed question can never mount the dock (the live modal-realism + * crash: GLM double-encoded `options` as a JSON string). * - The persisted message is assembled from the SAME parts the client * assembles (the processUIMessageStream shapes), so every downstream * consumer (phases, plan blocks, pending, extraction) is unchanged. @@ -239,17 +242,12 @@ export function createPiTurnResponse(spec: PiTurnSpec): Response { case "toolcall_end": { const toolCall = streamEvent.toolCall; toolInputs.set(toolCall.id, { toolName: toolCall.name, input: toolCall.arguments }); + // Input stays input-streaming here: it only becomes available + // once the call clears validation at the execution boundary + // (tool_execution_end) — an invalid input must never reach + // input-available, where the dock would mount on it. const part = newToolParts.get(toolCall.id); - if (part) { - part.state = "input-available"; - part.input = toolCall.arguments; - } - send({ - type: "tool-input-available", - toolCallId: toolCall.id, - toolName: toolCall.name, - input: toolCall.arguments, - }); + if (part) part.input = toolCall.arguments; break; } } @@ -274,12 +272,20 @@ export function createPiTurnResponse(spec: PiTurnSpec): Response { input, errorText, }); - } else if ((QUESTION_TOOLS as readonly string[]).includes(toolName)) { - // Client-owned: the answer arrives via addToolOutput; - // the synthetic placeholder result never reaches wire - // or record. - break; } else { + // Validation cleared: only NOW does the input become available + // to the client (the InvalidToolInputError gating the SDK owned). + if (part) { + part.state = "input-available"; + part.input = input; + } + send({ type: "tool-input-available", toolCallId: event.toolCallId, toolName, input }); + if ((QUESTION_TOOLS as readonly string[]).includes(toolName)) { + // Client-owned: the answer arrives via addToolOutput; + // the synthetic placeholder result never reaches wire + // or record. + break; + } const output = (event.result as { details?: unknown } | undefined)?.details; if (part) { part.state = "output-available"; diff --git a/tests/chat-flow.test.ts b/tests/chat-flow.test.ts index 627ca61..5a3c3f9 100644 --- a/tests/chat-flow.test.ts +++ b/tests/chat-flow.test.ts @@ -263,7 +263,25 @@ describe("teacher chat flow (scripted model)", () => { const chat = chatStore.createChat("Test: bad quiz retry"); createdChats.push(chat.id); const firstUser = { id: "u1", role: "user", parts: [{ type: "text", text: "quiz me" }] }; - await drain(await handleChatRequest(post(chat.id, [firstUser]), agentFactory(engine))); + const body = await drain(await handleChatRequest(post(chat.id, [firstUser]), agentFactory(engine))); + + // WIRE pin (the live modal-realism crash): the invalid call must NEVER + // become input-available — the dock mounts on input-available parts and + // crashed on a malformed quiz before tool-input-error could replace it. + // The SDK gated this with InvalidToolInputError; the bridge gates it on + // clean tool_execution_end. + assert.ok( + !body.includes('"type":"tool-input-available","toolCallId":"call-bad"'), + "the invalid call never reached input-available on the wire", + ); + assert.ok( + body.includes('"type":"tool-input-error","toolCallId":"call-bad"'), + "the invalid call surfaced as tool-input-error", + ); + assert.ok( + body.includes('"type":"tool-input-available","toolCallId":"call-good"'), + "the valid retry DID become input-available", + ); // The model was re-prompted (the error did not end the turn)… assert.equal(calls.length, 2, "the invalid call triggered a second model prompt"); @@ -291,6 +309,52 @@ describe("teacher chat flow (scripted model)", () => { assert.equal(pending?.toolCallId, "call-good", "the RETRIED quiz is the pending one"); assert.deepEqual(pending?.input, QUIZ_INPUT); }); + + it("a structurally invalid quiz (the live double-encoded options string) never reaches the dock and retries clean", async () => { + // The exact live modal-realism crash shape: GLM occasionally serializes + // a nested array as a JSON STRING inside the arguments. The structural + // layer (JSON Schema: options must be array) rejects the call at the + // boundary — it must surface as tool-input-error, never as a pending + // question, and the model's retry must become the pending quiz. + const STRINGY_QUIZ_INPUT = { + question: "Lewis's indexical analysis of 'actual' — which is it?", + options: JSON.stringify([ + { label: "Like 'I'", value: "indexical" }, + { label: "Like a title", value: "unique" }, + ]), // ← the bug: a string, not an array + correctAnswer: "indexical", + explanation: "Indexical: every world is actual to its own inhabitants.", + }; + + const steps: FauxResponseStep[] = [ + () => fauxAssistantMessage([fauxToolCall("quiz", STRINGY_QUIZ_INPUT, { id: "call-stringy" })], { stopReason: "toolUse" }), + () => fauxAssistantMessage([fauxToolCall("quiz", QUIZ_INPUT, { id: "call-clean" })], { stopReason: "toolUse" }), + ]; + const { engine } = scriptedTeacher(() => steps); + + const chat = chatStore.createChat("Test: string options"); + createdChats.push(chat.id); + const firstUser = { id: "u1", role: "user", parts: [{ type: "text", text: "quiz me" }] }; + const body = await drain(await handleChatRequest(post(chat.id, [firstUser]), agentFactory(engine))); + + assert.ok( + !body.includes('"type":"tool-input-available","toolCallId":"call-stringy"'), + "the structurally invalid quiz never became input-available on the wire", + ); + assert.ok( + body.includes('"type":"tool-input-error","toolCallId":"call-stringy"'), + "the invalid quiz surfaced as tool-input-error", + ); + + const saved = chatStore.getChat(chat.id)!; + const failed = saved.messages[1]!.parts.find( + (p) => (p as { toolCallId?: string }).toolCallId === "call-stringy", + ); + assert.equal((failed as { state?: string }).state, "output-error", "persisted as an error part"); + assert.match(String((failed as { errorText?: string }).errorText), /options/); + const pending = findPendingToolCall(saved.messages); + assert.equal(pending?.toolCallId, "call-clean", "the retried quiz is the pending one"); + }); }); describe("chat store", () => {