fix: the empty-new-session bug — three live client defects

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
)
This commit is contained in:
amosblomqvist
2026-09-26 02:13:18 +03:00
parent f3003d357a
commit 0803fbd88d
4 changed files with 69 additions and 7 deletions
+5 -1
View File
@@ -192,7 +192,11 @@ export class SessionModel {
constructor(opts: SessionModelOptions) { constructor(opts: SessionModelOptions) {
this.chatId = opts.chatId; this.chatId = opts.chatId;
this.base = opts.initialMessages; 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.pollMs = opts.pollMs ?? TURN_POLL_MS;
this.waitMaxMs = opts.waitMaxMs ?? TURN_WAIT_MAX_MS; this.waitMaxMs = opts.waitMaxMs ?? TURN_WAIT_MAX_MS;
this.onChange = opts.onChange; this.onChange = opts.onChange;
+11 -5
View File
@@ -34,21 +34,27 @@ export function useChatSession({ chatId, initialMessages, initiallyInFlight }: U
modelRef.current = new SessionModel({ chatId, initialMessages }); modelRef.current = new SessionModel({ chatId, initialMessages });
} }
const model = modelRef.current; 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(() => { useEffect(() => {
model.onChange = forceRender; model.onChange = bump;
return () => { return () => {
model.onChange = undefined; model.onChange = undefined;
}; };
}, [model, forceRender]); }, [model, bump]);
// Reattach on mount (StrictMode-safe: a newer resume cancels the older). // Reattach on mount (StrictMode-safe: a newer resume cancels the older).
useEffect(() => { useEffect(() => {
if (initiallyInFlight) void model.resumeStream(); if (initiallyInFlight) void model.resumeStream();
}, [initiallyInFlight, model]); }, [initiallyInFlight, model]);
// The view + state, derived per render from the model. // The view + state, derived per model change from the model.
const state = useMemo<SessionState>(() => model.getState(), [model, forceRender] as const); const state = useMemo<SessionState>(() => model.getState(), [model, version] as const);
// The reattach settled terminal → the sync-and-continue seam (chat.tsx). // The reattach settled terminal → the sync-and-continue seam (chat.tsx).
const setReattachSettled = useCallback( const setReattachSettled = useCallback(
+12 -1
View File
@@ -222,7 +222,18 @@ function toolPartOf(
} }
function applyResultTo(part: LhViewPart | undefined, result: ToolResultMessage): void { 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 // A result with no visible part (e.g. a call whose assistant message
// rendered nothing) is dropped from the view — the record keeps it. // rendered nothing) is dropped from the view — the record keeps it.
} }
+41
View File
@@ -115,6 +115,24 @@ function installFetch(routes: {
} }
describe("postInputEvent (the 409 identity-retry)", { skip: process.env.NO_SLOW ? true : undefined }, () => { 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<typeof fetch>) {
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 () => { it("passes non-409 responses through untouched", async () => {
const calls = installFetch({ chatPOST: () => new Response("stream", { status: 200 }) }); const calls = installFetch({ chatPOST: () => new Response("stream", { status: 200 }) });
const m = model({ pollMs: 10, waitMaxMs: 100 }); 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[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]!.state, "output-error");
assert.equal(parts[1]!.errorText, "correctAnswer does not match any option value"); 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({ type: "agent_end", messages: [] });
push("[DONE]"); push("[DONE]");
await done; await done;