From 736fe95c5605ac8e1f64b1393661cbdcd1266999 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 6 Oct 2026 04:20:34 -0700 Subject: [PATCH 1/2] fix(web/frames): a data chunk of a superseded response lands nowhere (C5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frames-rulings 1.2: a `data` chunk lands in its own response's table or nowhere. The integration rotates the address's table at the header and creates it at first use; the transport restamps every chunk with its response's version; but `createFrameHost.apply` routed `data` straight to the data hook with no version read — so a superseded response's late chunk was the first use and filled the table that was now the current response's (R4). The data path is now under the store's version guard like every other chunk: with the response announced to the store at its header (S-flush), a `data` chunk whose version is below the store's is dropped. Pins flipped to `test`: C5 (a), (b), (e). Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- .changeset/frames-data-response-scoped.md | 5 + packages/web/frames/src/frame-client.ts | 11 +- .../c05-data-response-scoped.spec.tsx | 328 +++++++++--------- 3 files changed, 170 insertions(+), 174 deletions(-) create mode 100644 .changeset/frames-data-response-scoped.md diff --git a/.changeset/frames-data-response-scoped.md b/.changeset/frames-data-response-scoped.md new file mode 100644 index 000000000..27153d38f --- /dev/null +++ b/.changeset/frames-data-response-scoped.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +frames: a `data` chunk is under the store's version guard like every other chunk — a superseded response's late data lands nowhere, never in the table that is now the current response's (contract C5 a, b, e). diff --git a/packages/web/frames/src/frame-client.ts b/packages/web/frames/src/frame-client.ts index d711c90c1..fa386580c 100644 --- a/packages/web/frames/src/frame-client.ts +++ b/packages/web/frames/src/frame-client.ts @@ -796,8 +796,17 @@ export function createFrameHost(options = {}) { } }, apply(chunk) { - // Data payloads are response-scoped; apply immediately, no store needed. + // Data payloads are response-scoped: they go to the data hook, not + // the store — but under the store's version guard like every other + // chunk (frames-rulings 1.2): a `data` chunk of a response the + // address has moved past lands nowhere, never in the table that is + // now the current response's (the transport restamps every chunk + // with its response's version; the integration rotates the table at + // the header and creates it at first use, so the first use must be + // the current response's). if (chunk.type === "data") { + const store = stores.get(chunk.id); + if (store && store.version !== undefined && chunk.version < store.version) return; options.applyData && options.applyData(chunk); return; } diff --git a/packages/web/test/consistency/c05-data-response-scoped.spec.tsx b/packages/web/test/consistency/c05-data-response-scoped.spec.tsx index 9e19eb73e..e353a895c 100644 --- a/packages/web/test/consistency/c05-data-response-scoped.spec.tsx +++ b/packages/web/test/consistency/c05-data-response-scoped.spec.tsx @@ -101,103 +101,89 @@ describe("C5 — data is response-scoped", () => { // trails v2's header, v2's record references `{$ref:"1"}`, v2's own data // for "1" arrives later. The fill must never show v1's value. // - // Observed on `next`: the fill mounts with text "old" (v1's value) and - // stays "old" after v2's data ("new") and `complete` arrive — the frame - // never re-resolves a record it already applied. Expected: the record - // waits (ref unresolved) until v2's data, then shows "new". - // Where it goes wrong: client.ts `beginStream` rotates by setting - // `tables.set(address, undefined)` at v2's header, and `tableFor` creates - // the table LAZILY at first use (`ensureTable`) — so v1's late `data` - // chunk, routed by `createFrameHost.apply` straight to `applyData` with - // no version check (the store guard only covers record writes), is the - // first use and lands in the table that is now v2's. The transport - // restamps chunks with the response's version but the data path never - // reads it; nothing associates a `data` chunk with the stream that - // carried it once the header has rotated the address's table. - test.fails( - "(a) v1's late data trails v2's header: v2's {$ref} never resolves to v1's value", - async () => { - const fid = freshFid("c5a"); - const getX = createServerReference(fid); - await sharedHost(); - const { held } = stubHeldFetch([WIRE, WIRE]); - const [v1, v2] = held; - const p1 = getX(1); - const p2 = getX(1); - // Both headers resolved: bump(A)=1 → onStream(A); bump(A)=2 → onStream(A). - expect(await p2).toBe(await p1); - v1.send(start(1)); - v2.send(start(1)); - await pump(1); - // v1's data, AFTER v2's header. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - // v2's record references the same ref id (ids restart per response). - v2.send(slot(1, "1")); - v2.send(html(1)); - await pump(1); - const { container, seen } = mountSite(p1); - await pump(); - // The record's ref is v2's; v2's data has not arrived: the fill waits. - expect(seen).toEqual([]); - expect(container.querySelector("li")).toBeNull(); - // v2's own data lands, then the stream completes. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); - v2.send(complete(1)); - v2.close(); - await pump(); - expect(seen).not.toContain("old"); - expect(container.querySelector("li")!.textContent).toBe("new"); - } - ); + // Was red on `next`: the fill mounted with "old" (v1's value) and stayed + // there — `beginStream` rotated the address's table at v2's header, the + // table was created lazily at first use, and v1's late `data` chunk went + // to `applyData` with no version check, so it was the first use and + // landed in v2's table. Green under frames-rulings 1.2: the data path is + // under the store's version guard like every other chunk — a `data` + // chunk of a response the address has moved past (the response's + // version, restamped by the transport, below the store's) lands nowhere. + test("(a) v1's late data trails v2's header: v2's {$ref} never resolves to v1's value", async () => { + const fid = freshFid("c5a"); + const getX = createServerReference(fid); + await sharedHost(); + const { held } = stubHeldFetch([WIRE, WIRE]); + const [v1, v2] = held; + const p1 = getX(1); + const p2 = getX(1); + // Both headers resolved: bump(A)=1 → onStream(A); bump(A)=2 → onStream(A). + expect(await p2).toBe(await p1); + v1.send(start(1)); + v2.send(start(1)); + await pump(1); + // v1's data, AFTER v2's header. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + // v2's record references the same ref id (ids restart per response). + v2.send(slot(1, "1")); + v2.send(html(1)); + await pump(1); + const { container, seen } = mountSite(p1); + await pump(); + // The record's ref is v2's; v2's data has not arrived: the fill waits. + expect(seen).toEqual([]); + expect(container.querySelector("li")).toBeNull(); + // v2's own data lands, then the stream completes. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); + v2.send(complete(1)); + v2.close(); + await pump(); + expect(seen).not.toContain("old"); + expect(container.querySelector("li")!.textContent).toBe("new"); + }); // Arm (b): the table rotation observed directly through the host's // resolver (what `#refsUnresolved`/`#resolveArgs` call with the frame's // address) — does v1's late data land in the table v2's refs resolve // from, and does it overwrite v2's own value once that has landed? // - // Observed on `next`: after v2's header, `resolve({$ref:"1"}, A)` reads - // "old" from v1's late chunk (expected undefined: v2 has delivered - // nothing); after v2's data ("new") a second late v1 chunk for "1" - // overwrites it to "old" again (expected "new"). Where it goes wrong: - // as in (a) — `tableFor(address)` is one table per ADDRESS at a time, - // keyed by nothing that names the response; `createJSONDataTable.apply` - // sets the key on every `initial` record, so whichever response's chunk - // arrives last owns the key. - test.fails( - "(b) table rotation: a superseded response's late data never lands in the current table", - async () => { - const fid = freshFid("c5b"); - const getX = createServerReference(fid); - const host = await sharedHost(); - const A = frameAddress(fid, [1]); - const { held } = stubHeldFetch([WIRE, WIRE]); - const [v1, v2] = held; - const p1 = getX(1); - const p2 = getX(1); - await p1; - await p2; - v1.send(start(1)); - v2.send(start(1)); - await pump(1); - expect(host.resolve({ $ref: "1" }, A)).toBeUndefined(); - // v1's late data after v2's header. - const v1Data = createDataSource(); - for (const c of v1Data.chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - const afterStaleData = host.resolve({ $ref: "1" }, A); - // v2's data lands. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); - await pump(1); - expect(host.resolve({ $ref: "1" }, A)).toBe("new"); - // Another straggler from v1 (a re-serialized key) after v2's value. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); - await pump(1); - const afterSecondStale = host.resolve({ $ref: "1" }, A); - expect(afterStaleData).toBeUndefined(); - expect(afterSecondStale).toBe("new"); - } - ); + // Was red on `next`: after v2's header, `resolve({$ref:"1"}, A)` read + // "old" from v1's late chunk, and a second late v1 chunk overwrote v2's + // "new" — one table per ADDRESS at a time, keyed by nothing that named + // the response, every `initial` record setting its key. Green: a stale + // response's data chunks are dropped at the host (see a). + test("(b) table rotation: a superseded response's late data never lands in the current table", async () => { + const fid = freshFid("c5b"); + const getX = createServerReference(fid); + const host = await sharedHost(); + const A = frameAddress(fid, [1]); + const { held } = stubHeldFetch([WIRE, WIRE]); + const [v1, v2] = held; + const p1 = getX(1); + const p2 = getX(1); + await p1; + await p2; + v1.send(start(1)); + v2.send(start(1)); + await pump(1); + expect(host.resolve({ $ref: "1" }, A)).toBeUndefined(); + // v1's late data after v2's header. + const v1Data = createDataSource(); + for (const c of v1Data.chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + const afterStaleData = host.resolve({ $ref: "1" }, A); + // v2's data lands. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) v2.send(c); + await pump(1); + expect(host.resolve({ $ref: "1" }, A)).toBe("new"); + // Another straggler from v1 (a re-serialized key) after v2's value. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) v1.send(c); + await pump(1); + const afterSecondStale = host.resolve({ $ref: "1" }, A); + expect(afterStaleData).toBeUndefined(); + expect(afterSecondStale).toBe("new"); + }); // Arm (c) (control): the normal order — v1 is complete before v2's header. // v1's data went to v1's table; v2's header rotates; v2's record waits for @@ -250,88 +236,84 @@ describe("C5 — data is response-scoped", () => { // stream. Then B-v1's late data for "1" lands, B-v2's record references // "1", B-v2's own data for "1" comes last. // - // Observed on `next`: the fill mounts with "old" (B-v1's value) under - // B-v2's record and stays "old" after B-v2's data. Expected: pending until - // B-v2's data, then "new". Where it goes wrong: as in (a) — the rotation - // at `onStream` is per address and the data path has no version. - test.fails( - "(e) through dynamic: A → B → A → B while B's first body is open; B-v1's late data never answers B-v2's record", - async () => { - const fid = freshFid("c5e"); - const getX = createServerReference(fid); - await sharedHost(); - // fetch order: A(v1), B(v1), A(v2), B(v2) - const { held, calls } = stubHeldFetch([WIRE, WIRE, WIRE, WIRE]); - const [a1, b1, a2, b2] = held; - const [n, setN] = createSignal(1); - const Site = dynamic(() => getX(n()) as any); - const seen: string[] = []; - let div!: HTMLDivElement; - const dispose = createRoot(d => { -
- …}> - { - createMemo(() => seen.push(p.text)); - return
  • {p.text}
    • ; - }} - /> -     -
    ; - document.body.appendChild(div); - return d; - }); - disposers.push(dispose); - await pump(); - a1.send(start(1)); - a1.send({ type: "html", id: WIRE, version: 1, html: "

    A

    " }); - a1.send(complete(1)); - a1.close(); - await pump(); - expect(div.querySelector("h1")!.textContent).toBe("A"); - // → B (v1), body held open after `start`. - setN(2); - await pump(); - b1.send(start(1)); - await pump(1); - // → A again (a fresh request; A's warm store re-materializes at once). - setN(1); - await pump(); - a2.send(start(1)); - a2.send(complete(1)); - a2.close(); - await pump(); - expect(div.querySelector("h1")!.textContent).toBe("A"); - // → B again while b1 is still open: B's v2 header rotates B's table. - setN(2); - await pump(); - expect(calls.length).toBe(4); - b2.send(start(1)); - await pump(1); - // B-v1's late data lands after B-v2's header. - for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) b1.send(c); - await pump(1); - // B-v2's record references "1"; its html mounts the occurrence. - b2.send(slot(1, "1")); - b2.send({ - type: "html", - id: WIRE, - version: 1, - html: `

    B

    ` - }); - await pump(); - expect(div.querySelector("h1")!.textContent).toBe("B"); - // B-v2 has delivered no data for "1": the record waits. - expect(seen).toEqual([]); - for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) b2.send(c); - b2.send(complete(1)); - b2.close(); - b1.close(); - await pump(); - expect(seen).not.toContain("old"); - expect(div.querySelector("li")!.textContent).toBe("new"); - } - ); + // Was red on `next`: the fill mounted with "old" (B-v1's value) under + // B-v2's record and stayed there — the rotation at `onStream` was per + // address and the data path had no version. Green: as in (a). + test("(e) through dynamic: A → B → A → B while B's first body is open; B-v1's late data never answers B-v2's record", async () => { + const fid = freshFid("c5e"); + const getX = createServerReference(fid); + await sharedHost(); + // fetch order: A(v1), B(v1), A(v2), B(v2) + const { held, calls } = stubHeldFetch([WIRE, WIRE, WIRE, WIRE]); + const [a1, b1, a2, b2] = held; + const [n, setN] = createSignal(1); + const Site = dynamic(() => getX(n()) as any); + const seen: string[] = []; + let div!: HTMLDivElement; + const dispose = createRoot(d => { +
    + …}> + { + createMemo(() => seen.push(p.text)); + return
  • {p.text}
    • ; + }} + /> +     +
    ; + document.body.appendChild(div); + return d; + }); + disposers.push(dispose); + await pump(); + a1.send(start(1)); + a1.send({ type: "html", id: WIRE, version: 1, html: "

    A

    " }); + a1.send(complete(1)); + a1.close(); + await pump(); + expect(div.querySelector("h1")!.textContent).toBe("A"); + // → B (v1), body held open after `start`. + setN(2); + await pump(); + b1.send(start(1)); + await pump(1); + // → A again (a fresh request; A's warm store re-materializes at once). + setN(1); + await pump(); + a2.send(start(1)); + a2.send(complete(1)); + a2.close(); + await pump(); + expect(div.querySelector("h1")!.textContent).toBe("A"); + // → B again while b1 is still open: B's v2 header rotates B's table. + setN(2); + await pump(); + expect(calls.length).toBe(4); + b2.send(start(1)); + await pump(1); + // B-v1's late data lands after B-v2's header. + for (const c of createDataSource().chunks(WIRE, 1, { "1": "old" })) b1.send(c); + await pump(1); + // B-v2's record references "1"; its html mounts the occurrence. + b2.send(slot(1, "1")); + b2.send({ + type: "html", + id: WIRE, + version: 1, + html: `

    B

    ` + }); + await pump(); + expect(div.querySelector("h1")!.textContent).toBe("B"); + // B-v2 has delivered no data for "1": the record waits. + expect(seen).toEqual([]); + for (const c of createDataSource().chunks(WIRE, 1, { "1": "new" })) b2.send(c); + b2.send(complete(1)); + b2.close(); + b1.close(); + await pump(); + expect(seen).not.toContain("old"); + expect(div.querySelector("li")!.textContent).toBe("new"); + }); // Arm (d): v2's data arrives FIRST, the site mounts and shows "new"; then // v1's late data for the same ref arrives. The mounted fill keeps "new" From 63b5e858503f2b1444ce6e07c95c1fb97bcfbff3 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Tue, 6 Oct 2026 04:39:49 -0700 Subject: [PATCH 2/2] web/frames: the data guard needs no undefined check (n < undefined is false) Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- packages/web/frames/src/frame-client.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/web/frames/src/frame-client.ts b/packages/web/frames/src/frame-client.ts index fa386580c..55bac57fd 100644 --- a/packages/web/frames/src/frame-client.ts +++ b/packages/web/frames/src/frame-client.ts @@ -806,7 +806,9 @@ export function createFrameHost(options = {}) { // the current response's). if (chunk.type === "data") { const store = stores.get(chunk.id); - if (store && store.version !== undefined && chunk.version < store.version) return; + // (`n < undefined` is false: a store with no version yet, or a chunk + // with none, guards nothing.) + if (store && chunk.version < store.version) return; options.applyData && options.applyData(chunk); return; }