From 1ff60b4681119b84035fd601609f340712345e06 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Tue, 27 Oct 2026 18:39:42 +0200 Subject: [PATCH] fix: recover opening refusals after view teardown --- components/session-core.ts | 25 +++++-- tests/opening-delivery.test.ts | 124 ++++++++++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 7 deletions(-) diff --git a/components/session-core.ts b/components/session-core.ts index 8c53aa4..4a3c19e 100644 --- a/components/session-core.ts +++ b/components/session-core.ts @@ -258,6 +258,9 @@ export class SessionModel { // The single live consume (POST response or reattach stream): a newer // consume cancels the older — one reader at a time. private consumeAbort: AbortController | null = null; + /** Retain the latest request across stop(): an opening POST still owns + * its refusal after view detachment, but never after a newer request. */ + private latestRequest: AbortController | null = null; /** Synchronous ready check for the gate (send/read mount logic). */ readyRef = true; @@ -555,6 +558,7 @@ export class SessionModel { this.consumeAbort?.abort(); const controller = new AbortController(); this.consumeAbort = controller; + this.latestRequest = controller; // REPLAY ONTO A CLEAN SLATE (owner, optimistic round 3, 2026-10-02): // the reattach stream replays the turn from its FIRST event — an // aborted earlier attempt (React StrictMode's simulated remount) @@ -645,6 +649,7 @@ export class SessionModel { this.consumeAbort?.abort(); const controller = new AbortController(); this.consumeAbort = controller; + this.latestRequest = controller; this.setError(null); this.setStatus("submitted"); if (recovering && input.type === "answer") { @@ -741,11 +746,21 @@ export class SessionModel { await this.resumeStream(true); return true; } catch (err) { - if (this.consumeAbort !== controller || controller.signal.aborted) return accepted; - // Restore the question, or the new-session opening draft, when - // acceptance failed/is unknown. Other text sends keep their existing - // optimistic presentation; remove only THIS attempt's local entries. - if (input.type === "answer" || (input.type === "text" && onAccepted && !accepted)) { + const failedOpening = input.type === "text" && onAccepted !== undefined && !accepted; + if (failedOpening) { + // The POST outlives stop(), including StrictMode's cleanup. Remove + // only its own failed optimistic bubble even if a newer request + // won; never touch canonical messages or another attempt's tail. + const ids = new Set(optimistic.map(m => m.id)); + this.mutate(() => { this.tail = this.tail.filter(m => !ids.has(m.id)); }); + } + const detachedOpening = failedOpening && this.consumeAbort === null && this.latestRequest === controller; + if ((this.consumeAbort !== controller || controller.signal.aborted) && !detachedOpening) return accepted; + // A detached opening refusal must still reach the replayed mount's + // existing setup recovery. A newer send/reattach ALWAYS owns errors + // and status instead, even if that request has since been stopped. + // Ordinary text and answer teardown behavior stays unchanged. + if (input.type === "answer") { const ids = new Set(optimistic.map(m => m.id)); this.mutate(() => { this.tail = this.tail.filter(m => !ids.has(m.id)); diff --git a/tests/opening-delivery.test.ts b/tests/opening-delivery.test.ts index 11926bc..35ef0d6 100644 --- a/tests/opening-delivery.test.ts +++ b/tests/opening-delivery.test.ts @@ -5,8 +5,9 @@ import { newUserMessage } from "../src/core/native-messages.ts"; function deferred() { let resolve!: (value: T) => void; - const promise = new Promise(done => { resolve = done; }); - return { promise, resolve }; + let reject!: (reason: unknown) => void; + const promise = new Promise((done, fail) => { resolve = done; reject = fail; }); + return { promise, resolve, reject }; } describe("new-session opening delivery acceptance", () => { @@ -60,6 +61,125 @@ describe("new-session opening delivery acceptance", () => { }); } + for (const failure of ["network", "503"] as const) { + it(`a teardown racing an opening ${failure} failure restores the bubble/error recovery state`, async () => { + const post = deferred(); + let notified = 0; + const envelope = JSON.stringify({ code: "models_not_configured", missing: ["strong"] }); + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async url => { + assert.equal(String(url), "/api/chat", "failure must not attach or resend"); + return post.promise; + } }); + const sending = m.send({ type: "text", text: "recover this opening" }, () => { notified++; }); + m.stop(); // StrictMode's cleanup detaches the consume, NOT this POST. + if (failure === "network") post.reject(new TypeError("opening connection lost")); + else post.resolve(new Response(envelope, { status: 503 })); + assert.equal(await sending, false); + assert.equal(notified, 0); + assert.equal(m.getState().nativeMessages.length, 0, "no stranded optimistic opening bubble"); + assert.equal(m.status, "error"); + assert.equal(m.getState().error?.message, failure === "network" ? "opening connection lost" : envelope, + "the raw refusal must reach the existing setup-recovery effect"); + }); + + it(`an older opening ${failure} failure removes only its own bubble while a newer send owns state`, async () => { + const first = deferred(); + const second = deferred(); + let posts = 0; + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async url => { + assert.equal(String(url), "/api/chat"); + return ++posts === 1 ? first.promise : second.promise; + } }); + const older = m.send({ type: "text", text: "older opening" }, () => assert.fail("not accepted")); + m.stop(); + const newer = m.send({ type: "text", text: "newer text" }); + if (failure === "network") first.reject(new TypeError("older lost acknowledgment")); + else first.resolve(new Response("older refusal", { status: 503 })); + assert.equal(await older, false); + assert.equal(m.status, "submitted", "older failure must not settle the newer send"); + assert.equal(m.getState().error, null); + assert.deepEqual(m.getState().nativeMessages.map(message => message.content), ["newer text"]); + second.resolve(new Response("newer failure", { status: 503 })); + assert.equal(await newer, false); + assert.equal(m.getState().error?.message, "newer failure"); + assert.deepEqual(m.getState().nativeMessages.map(message => message.content), ["newer text"], + "ordinary text retains its existing optimistic failure presentation"); + }); + } + + it("a newer send remains the owner even after it fails and is stopped", async () => { + const first = deferred(); + let posts = 0; + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async () => { + return ++posts === 1 ? first.promise : new Response("newer failure", { status: 503 }); + } }); + const older = m.send({ type: "text", text: "older opening" }, () => assert.fail("not accepted")); + m.stop(); + await m.send({ type: "text", text: "newer text" }); + m.stop(); + first.reject(new TypeError("older failure")); + assert.equal(await older, false); + assert.equal(m.status, "ready"); + assert.equal(m.getState().error?.message, "newer failure"); + assert.deepEqual(m.getState().nativeMessages.map(message => message.content), ["newer text"]); + }); + + it("a replacement reattach owns state instead of the older stopped opening POST", async () => { + const post = deferred(); + const record = deferred(); + const saved = newUserMessage("server-owned text"); + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async url => { + if (String(url) === "/api/chat") return post.promise; + if (String(url).endsWith("/stream")) return new Response("data: [DONE]\n\n"); + return record.promise; + } }); + const older = m.send({ type: "text", text: "older opening" }, () => assert.fail("not accepted")); + m.stop(); + const replacement = m.resumeStream(); + post.reject(new TypeError("older failure")); + assert.equal(await older, false); + assert.equal(m.status, "submitted"); + assert.equal(m.getState().error, null); + record.resolve(Response.json({ messages: [saved] })); + await replacement; + assert.equal(m.status, "ready"); + assert.deepEqual(m.getState().nativeMessages, [saved]); + assert.equal(m.getState().error, null); + }); + + it("a stopped replacement reattach still supersedes the older opening refusal", async () => { + const post = deferred(); + const record = deferred(); + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async url => { + return String(url) === "/api/chat" ? post.promise : record.promise; + } }); + const older = m.send({ type: "text", text: "older opening" }, () => assert.fail("not accepted")); + m.stop(); + const replacement = m.resumeStream(); + m.stop(); + post.resolve(new Response("older refusal", { status: 503 })); + assert.equal(await older, false); + assert.equal(m.status, "ready"); + assert.equal(m.getState().error, null); + assert.equal(m.getState().nativeMessages.length, 0); + record.resolve(Response.json({ messages: [] })); + await replacement; + assert.equal(m.status, "ready"); + assert.equal(m.getState().error, null); + }); + + it("a stopped ordinary text failure retains the pre-existing teardown behavior", async () => { + const post = deferred(); + const m = new SessionModel({ chatId: "c1", initialMessages: [], fetch: async () => post.promise }); + const sending = m.send({ type: "text", text: "ordinary text" }); + m.stop(); + post.reject(new TypeError("connection lost")); + assert.equal(await sending, false); + assert.equal(m.status, "ready"); + assert.equal(m.getState().error, null); + assert.deepEqual(m.getState().nativeMessages.map(message => message.content), ["ordinary text"]); + }); + it("post-acceptance attachment failure remains accepted and does not resend input", async () => { let posts = 0; let notified = 0;