From 0803fbd88de853f017f56c8ea8bcc87d8a5dc2a6 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 26 Sep 2026 02:13:18 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20the=20empty-new-session=20bug=20?= =?UTF-8?q?=E2=80=94=20three=20live=20client=20defects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-driven via the owner's dev server (browser tools), reproduced on /chat/new: send fires, optimistic appends, POST never happens, and the page shows the frozen empty state. Three compounding defects: 1. session-core: default fetch stored UNBOUND — the browser's native fetch requires this === Window, so every call threw 'Illegal invocation' and the POST /api/chat never fired. Node's undici has no receiver discipline, which is why 994/994 tests passed while the app was dead. Now defaulted through a receiver-preserving wrapper; pinned by a strict-this regression test (the browser's semantics simulated. 2. session.ts: state memo keyed on [model, forceRender] — the reducer dispatch is STABLE by contract, so the memo computed once per mount: no optimistic bubble, no stream events, no error banner. The view was frozen at first render. Rekeyed on a version counter bumped by the model's onChange. 3. view.ts: turn_end carries the clean 'Question posed' placeholder as a toolResult message (details: null) — applyResult flipped the quiz part to output-available with a null output and AnsweredCard crashed on answer.dismissed (killed the whole page render). Fork 2 now holds on the message-attachment path too: clean question results with null details are ignored; the AnsweredCard renders only the learner's real answer. Regression pin extended in session-core.test.ts. Verified live end to end: new session → text send → probe questions in the dock → ask answer → graded ✓ quiz card with explanation → reload renders the full transcript from the native record with the pending question intact. EOF ) --- components/session-core.ts | 6 +++++- components/session.ts | 16 ++++++++++----- src/core/view.ts | 13 +++++++++++- tests/session-core.test.ts | 41 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/components/session-core.ts b/components/session-core.ts index 309b321..03684fa 100644 --- a/components/session-core.ts +++ b/components/session-core.ts @@ -192,7 +192,11 @@ export class SessionModel { constructor(opts: SessionModelOptions) { this.chatId = opts.chatId; this.base = opts.initialMessages; - this.fetch = opts.fetch ?? globalThis.fetch; + // BOUND: the browser's native fetch is a Window method — storing it + // unbound makes every call throw "Illegal invocation" in the browser + // (Node's undici doesn't care, so the tests never saw it). Always go + // through a receiver-preserving wrapper. + this.fetch = opts.fetch ?? ((...args) => globalThis.fetch(...args)); this.pollMs = opts.pollMs ?? TURN_POLL_MS; this.waitMaxMs = opts.waitMaxMs ?? TURN_WAIT_MAX_MS; this.onChange = opts.onChange; diff --git a/components/session.ts b/components/session.ts index f9aac07..4e2e2c4 100644 --- a/components/session.ts +++ b/components/session.ts @@ -34,21 +34,27 @@ export function useChatSession({ chatId, initialMessages, initiallyInFlight }: U modelRef.current = new SessionModel({ chatId, initialMessages }); } const model = modelRef.current; - const [, forceRender] = useReducer((n: number) => n + 1, 0); + // The render VERSION — bumped by every model change, so the state memo + // below recomputes exactly when the model actually changed. (Using the + // reducer's dispatch as the memo dep was a freeze bug: dispatch is + // STABLE by contract, so the memo computed once per mount and the view + // never saw a single model update — no optimistic bubble, no stream, no + // error banner. The empty-session bug, 2026-09-26.) + const [version, bump] = useReducer((n: number) => n + 1, 0); useEffect(() => { - model.onChange = forceRender; + model.onChange = bump; return () => { model.onChange = undefined; }; - }, [model, forceRender]); + }, [model, bump]); // Reattach on mount (StrictMode-safe: a newer resume cancels the older). useEffect(() => { if (initiallyInFlight) void model.resumeStream(); }, [initiallyInFlight, model]); - // The view + state, derived per render from the model. - const state = useMemo(() => model.getState(), [model, forceRender] as const); + // The view + state, derived per model change from the model. + const state = useMemo(() => model.getState(), [model, version] as const); // The reattach settled terminal → the sync-and-continue seam (chat.tsx). const setReattachSettled = useCallback( diff --git a/src/core/view.ts b/src/core/view.ts index 20f3c0a..e439b06 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -222,7 +222,18 @@ function toolPartOf( } function applyResultTo(part: LhViewPart | undefined, result: ToolResultMessage): void { - if (part) applyResult(part, result); + if (part) { + // Fork 2 (owner, Wave D): the clean "Question posed" placeholder + // (details: null — tool-execute questionResult) is client-owned + // question territory. The dock mounts on the dangling call; the + // AnsweredCard renders only the learner's REAL answer (details set). + // Flipping the part to output-available on the placeholder would hand + // AnsweredCard a null and crash the render (the live crash, 2026-09-26). + const tool = part.type.startsWith("tool-") ? part.type.slice("tool-".length) : part.type; + const isQuestion = (QUESTION_TOOLS as readonly string[]).includes(tool); + if (isQuestion && !result.isError && result.details == null) return; + applyResult(part, result); + } // A result with no visible part (e.g. a call whose assistant message // rendered nothing) is dropped from the view — the record keeps it. } diff --git a/tests/session-core.test.ts b/tests/session-core.test.ts index e5c0daf..9032e90 100644 --- a/tests/session-core.test.ts +++ b/tests/session-core.test.ts @@ -115,6 +115,24 @@ function installFetch(routes: { } describe("postInputEvent (the 409 identity-retry)", { skip: process.env.NO_SLOW ? true : undefined }, () => { + it("the model's default fetch is RECEIVER-SAFE (the browser 'Illegal invocation' pin)", async () => { + // The live empty-session bug (2026-09-26): storing globalThis.fetch + // UNBOUND and calling it as this.fetch(...) throws "Illegal + // invocation" in the browser (native fetch requires this === Window; + // Node's undici doesn't care, so every test passed while the app was + // dead). Pin the browser's receiver discipline with a strict stub. + let receiverWasGlobal = false; + globalThis.fetch = (function (this: unknown, ...args: Parameters) { + if (this !== globalThis) throw new TypeError("Failed to execute 'fetch' on 'Window': Illegal invocation"); + receiverWasGlobal = true; + return Promise.resolve(sseResponse("[DONE]")); + }) as typeof fetch; + const m = model(); // NO injected fetch — exercises the default wrapper + const res = await m.postInputEvent({ type: "text", text: "hi" }); + assert.equal(res.status, 200); + assert.ok(receiverWasGlobal, "the default wrapper calls fetch with the global receiver"); + }); + it("passes non-409 responses through untouched", async () => { const calls = installFetch({ chatPOST: () => new Response("stream", { status: 200 }) }); const m = model({ pollMs: 10, waitMaxMs: 100 }); @@ -303,6 +321,29 @@ describe("send (one input event, verbatim consumption)", () => { assert.equal(parts[0]!.state, "input-available", "the clean placeholder is IGNORED — the dock mounts on it"); assert.equal(parts[1]!.state, "output-error"); assert.equal(parts[1]!.errorText, "correctAnswer does not match any option value"); + // turn_end carries the placeholder as a toolResult MESSAGE (details: + // null — tool-execute questionResult): it must not flip the part to + // output-available either — the AnsweredCard would read .dismissed off + // null and crash the whole render (the live crash, 2026-09-26). + push({ + type: "turn_end", + message: { role: "assistant", id: "a1", content: [{ type: "text", text: "" }], stopReason: "toolUse" }, + toolResults: [ + { + id: "tr_q1", + role: "toolResult", + toolCallId: "q1", + toolName: "quiz", + content: [{ type: "text", text: "Question posed — the turn ends here; the learner's graded answer arrives as this call's result." }], + details: null, + isError: false, + } as never, + ], + }); + await settle(); + const afterTurnEnd = m.getState().messages.flatMap((v) => v.parts).filter((p) => p.type === "tool-quiz"); + assert.equal(afterTurnEnd[0]!.state, "input-available", "the placeholder toolResult keeps the dock's mount state"); + assert.equal(afterTurnEnd[0]!.output, undefined, "no null output ever reaches the AnsweredCard"); push({ type: "agent_end", messages: [] }); push("[DONE]"); await done;