From 3a71068b486eaa0b38dc2532dc10469ba9350cf9 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 04:57:10 -0700 Subject: [PATCH 1/4] Add tests for putReadState monotonicity Covers the in-memory store (a stale write landing after a newer one must not move the cursor back) and a real-Postgres drizzle test proving the same for createDrizzleChatStore's SQL upsert. --- packages/chat/test/read-state.drizzle.test.ts | 131 ++++++++++++++++++ packages/chat/test/store.test.ts | 27 ++++ 2 files changed, 158 insertions(+) create mode 100644 packages/chat/test/read-state.drizzle.test.ts diff --git a/packages/chat/test/read-state.drizzle.test.ts b/packages/chat/test/read-state.drizzle.test.ts new file mode 100644 index 000000000..bb8e89a65 --- /dev/null +++ b/packages/chat/test/read-state.drizzle.test.ts @@ -0,0 +1,131 @@ +// DB-gated: skipped when no DATABASE_URL is reachable (a fresh +// checkout still runs the unit gates), mirroring `migrations.test.ts`. +// Runs against its own scratch database. +// +// `store.test.ts` proves `putReadState`'s monotonicity guard against +// the in-memory store. This exercises the real `createDrizzleChatStore` +// path, where the guard is a conditional `ON CONFLICT DO UPDATE ... SET` +// rather than an in-process comparison — proving the SQL itself never +// regresses a reader's cursor. +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { drizzle } from "drizzle-orm/postgres-js"; +import postgres from "postgres"; + +import { e2eDatabaseUrl } from "../../../scripts/e2e/harness"; +import { applyChatMigrations } from "../src/migrations"; +import { createDrizzleChatStore } from "../src/store"; + +function scratchUrlFor(e2eUrl: string): string { + const url = new URL(e2eUrl); + const database = url.pathname.replace(/^\//, ""); + url.pathname = `/${database}_chat_read_state_drizzle_test`; + return url.toString(); +} + +const databaseUrl = e2eDatabaseUrl(); +const describeIfDb = databaseUrl === undefined ? describe.skip : describe; + +const TENANT = "tnt_1"; +const WORKBENCH = "run_workbench1"; +const PRINCIPAL = "prn_alice"; + +describeIfDb("createDrizzleChatStore: putReadState monotonicity", () => { + const scratchUrl = scratchUrlFor( + databaseUrl ?? "postgres://localhost:5432/unused", + ); + const scratchTarget = new URL(scratchUrl); + const scratchDatabase = scratchTarget.pathname.replace(/^\//, ""); + + beforeAll(async () => { + const maintenanceUrl = new URL(scratchUrl); + maintenanceUrl.pathname = "/postgres"; + const maintenance = postgres(maintenanceUrl.toString(), { + max: 1, + onnotice: () => undefined, + }); + try { + await maintenance.unsafe(`DROP DATABASE IF EXISTS "${scratchDatabase}"`); + await maintenance.unsafe(`CREATE DATABASE "${scratchDatabase}"`); + } finally { + await maintenance.end(); + } + await applyChatMigrations(scratchUrl); + }); + + afterAll(async () => { + const maintenanceUrl = new URL(scratchUrl); + maintenanceUrl.pathname = "/postgres"; + const maintenance = postgres(maintenanceUrl.toString(), { + max: 1, + onnotice: () => undefined, + }); + try { + await maintenance.unsafe(`DROP DATABASE IF EXISTS "${scratchDatabase}"`); + } finally { + await maintenance.end(); + } + }); + + test("a stale write landing after a newer one never moves the cursor backward", async () => { + const sql = postgres(scratchUrl, { max: 5, onnotice: () => undefined }); + try { + const store = createDrizzleChatStore(drizzle(sql)); + + await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: PRINCIPAL, + lastSeenCreatedAt: new Date("2026-01-02T00:00:00.000Z"), + lastSeenId: "mail_2", + }); + + const result = await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: PRINCIPAL, + lastSeenCreatedAt: new Date("2026-01-01T00:00:00.000Z"), + lastSeenId: "mail_1", + }); + + expect(result.lastSeenId).toBe("mail_2"); + expect(result.lastSeenCreatedAt).toEqual( + new Date("2026-01-02T00:00:00.000Z"), + ); + + const stored = await store.getReadState(TENANT, WORKBENCH, PRINCIPAL); + expect(stored?.lastSeenId).toBe("mail_2"); + } finally { + await sql.end(); + } + }); + + test("a newer write still moves the cursor forward", async () => { + const sql = postgres(scratchUrl, { max: 5, onnotice: () => undefined }); + try { + const store = createDrizzleChatStore(drizzle(sql)); + + await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: "prn_bob", + lastSeenCreatedAt: new Date("2026-01-01T00:00:00.000Z"), + lastSeenId: "mail_1", + }); + + const result = await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: "prn_bob", + lastSeenCreatedAt: new Date("2026-01-03T00:00:00.000Z"), + lastSeenId: "mail_3", + }); + + expect(result.lastSeenId).toBe("mail_3"); + + const stored = await store.getReadState(TENANT, WORKBENCH, "prn_bob"); + expect(stored?.lastSeenId).toBe("mail_3"); + } finally { + await sql.end(); + } + }); +}); diff --git a/packages/chat/test/store.test.ts b/packages/chat/test/store.test.ts index 06bc0a0d8..5e47353a1 100644 --- a/packages/chat/test/store.test.ts +++ b/packages/chat/test/store.test.ts @@ -122,3 +122,30 @@ test("putReadState upserts a per-principal cursor without disturbing other princ const bob = await store.getReadState("tnt_1", "chn_1", "prn_bob"); expect(bob).toBeUndefined(); }); + +test("putReadState never moves the cursor backward when a stale write lands after a newer one", async () => { + const store = createInMemoryChatStore(); + await store.putReadState({ + tenantId: "tnt_1", + workbenchId: "chn_1", + principalId: "prn_alice", + lastSeenCreatedAt: new Date("2026-01-02T00:00:00.000Z"), + lastSeenId: "mail_2", + }); + + const result = await store.putReadState({ + tenantId: "tnt_1", + workbenchId: "chn_1", + principalId: "prn_alice", + lastSeenCreatedAt: new Date("2026-01-01T00:00:00.000Z"), + lastSeenId: "mail_1", + }); + + expect(result.lastSeenId).toBe("mail_2"); + expect(result.lastSeenCreatedAt).toEqual( + new Date("2026-01-02T00:00:00.000Z"), + ); + + const alice = await store.getReadState("tnt_1", "chn_1", "prn_alice"); + expect(alice?.lastSeenId).toBe("mail_2"); +}); From cac058ba106af46ac44d579fa36a792d4fcb5a94 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 04:57:28 -0700 Subject: [PATCH 2/4] putReadState: never move a reader's cursor backward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A slower "read up to X" write landing after a faster "read up to Y" regressed the stored cursor and flipped X..Y back to unread. Both store implementations now keep the later lastSeenCreatedAt on conflict — a conditional CASE WHEN in the Drizzle upsert, and an equivalent comparison in the in-memory store. Fixes CL-7131. --- packages/chat/src/store.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/chat/src/store.ts b/packages/chat/src/store.ts index 32279e628..ae82ba51c 100644 --- a/packages/chat/src/store.ts +++ b/packages/chat/src/store.ts @@ -10,7 +10,7 @@ // Routing against the interface (rather than a raw drizzle handle) keeps the // route layer testable with a plain in-memory fake, with no database and no // drizzle SQL-condition internals involved. -import { and, eq, inArray } from "drizzle-orm"; +import { and, eq, inArray, sql } from "drizzle-orm"; import type { PostgresJsDatabase } from "drizzle-orm/postgres-js"; import { participantsOf } from "./workbench-settings"; @@ -311,8 +311,8 @@ export function createDrizzleChatStore>( workbenchReadState.principalId, ], set: { - lastSeenCreatedAt: input.lastSeenCreatedAt, - lastSeenId: input.lastSeenId, + lastSeenCreatedAt: sql`CASE WHEN excluded.last_seen_created_at > ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_created_at ELSE ${workbenchReadState.lastSeenCreatedAt} END`, + lastSeenId: sql`CASE WHEN excluded.last_seen_created_at > ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_id ELSE ${workbenchReadState.lastSeenId} END`, }, }) .returning(); @@ -459,11 +459,20 @@ export function createInMemoryChatStore(): ChatStore { }, async putReadState(input) { - const row: ReadStateRow = { ...input }; - readStateByKey.set( - readStateKey(input.tenantId, input.workbenchId, input.principalId), - row, + const key = readStateKey( + input.tenantId, + input.workbenchId, + input.principalId, ); + const existing = readStateByKey.get(key); + if ( + existing !== undefined && + existing.lastSeenCreatedAt >= input.lastSeenCreatedAt + ) { + return existing; + } + const row: ReadStateRow = { ...input }; + readStateByKey.set(key, row); return row; }, From 7e2140fcc90049411bce06a2eba914c6e7caad09 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:27:40 -0700 Subject: [PATCH 3/4] Add same-millisecond monotonicity test for putReadState workbench_messages.created_at has millisecond precision and same-millisecond messages are expected, so the monotonicity guard must accept an equal-timestamp write, not just a strictly later one. Covers the in-memory store and the drizzle-backed store. --- packages/chat/test/read-state.drizzle.test.ts | 31 +++++++++++++++++++ packages/chat/test/store.test.ts | 25 +++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/chat/test/read-state.drizzle.test.ts b/packages/chat/test/read-state.drizzle.test.ts index bb8e89a65..a8c844b93 100644 --- a/packages/chat/test/read-state.drizzle.test.ts +++ b/packages/chat/test/read-state.drizzle.test.ts @@ -128,4 +128,35 @@ describeIfDb("createDrizzleChatStore: putReadState monotonicity", () => { await sql.end(); } }); + + test("a same-millisecond forward move to a different message still lands", async () => { + const sql = postgres(scratchUrl, { max: 5, onnotice: () => undefined }); + try { + const store = createDrizzleChatStore(drizzle(sql)); + const sameCreatedAt = new Date("2026-01-04T00:00:00.001Z"); + + await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: "prn_carol", + lastSeenCreatedAt: sameCreatedAt, + lastSeenId: "mail_4", + }); + + const result = await store.putReadState({ + tenantId: TENANT, + workbenchId: WORKBENCH, + principalId: "prn_carol", + lastSeenCreatedAt: sameCreatedAt, + lastSeenId: "mail_5", + }); + + expect(result.lastSeenId).toBe("mail_5"); + + const stored = await store.getReadState(TENANT, WORKBENCH, "prn_carol"); + expect(stored?.lastSeenId).toBe("mail_5"); + } finally { + await sql.end(); + } + }); }); diff --git a/packages/chat/test/store.test.ts b/packages/chat/test/store.test.ts index 5e47353a1..f7cbab837 100644 --- a/packages/chat/test/store.test.ts +++ b/packages/chat/test/store.test.ts @@ -149,3 +149,28 @@ test("putReadState never moves the cursor backward when a stale write lands afte const alice = await store.getReadState("tnt_1", "chn_1", "prn_alice"); expect(alice?.lastSeenId).toBe("mail_2"); }); + +test("putReadState still lands a same-millisecond forward move to a different message", async () => { + const store = createInMemoryChatStore(); + const sameCreatedAt = new Date("2026-01-02T00:00:00.001Z"); + await store.putReadState({ + tenantId: "tnt_1", + workbenchId: "chn_1", + principalId: "prn_alice", + lastSeenCreatedAt: sameCreatedAt, + lastSeenId: "mail_2", + }); + + const result = await store.putReadState({ + tenantId: "tnt_1", + workbenchId: "chn_1", + principalId: "prn_alice", + lastSeenCreatedAt: sameCreatedAt, + lastSeenId: "mail_3", + }); + + expect(result.lastSeenId).toBe("mail_3"); + + const alice = await store.getReadState("tnt_1", "chn_1", "prn_alice"); + expect(alice?.lastSeenId).toBe("mail_3"); +}); From 46bb2f763171aa34d2309e10918366fd13773519 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:28:00 -0700 Subject: [PATCH 4/4] putReadState: accept an equal-timestamp cursor as a forward move The guard used a strict > comparison, so a same-millisecond write to a later message in the same batch was dropped as a no-op and the route echoed back the older cursor. Only a strictly older timestamp is a no-op now; an equal timestamp lands the incoming write. --- packages/chat/src/store.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/chat/src/store.ts b/packages/chat/src/store.ts index ae82ba51c..71c8abc68 100644 --- a/packages/chat/src/store.ts +++ b/packages/chat/src/store.ts @@ -311,8 +311,8 @@ export function createDrizzleChatStore>( workbenchReadState.principalId, ], set: { - lastSeenCreatedAt: sql`CASE WHEN excluded.last_seen_created_at > ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_created_at ELSE ${workbenchReadState.lastSeenCreatedAt} END`, - lastSeenId: sql`CASE WHEN excluded.last_seen_created_at > ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_id ELSE ${workbenchReadState.lastSeenId} END`, + lastSeenCreatedAt: sql`CASE WHEN excluded.last_seen_created_at >= ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_created_at ELSE ${workbenchReadState.lastSeenCreatedAt} END`, + lastSeenId: sql`CASE WHEN excluded.last_seen_created_at >= ${workbenchReadState.lastSeenCreatedAt} THEN excluded.last_seen_id ELSE ${workbenchReadState.lastSeenId} END`, }, }) .returning(); @@ -467,7 +467,7 @@ export function createInMemoryChatStore(): ChatStore { const existing = readStateByKey.get(key); if ( existing !== undefined && - existing.lastSeenCreatedAt >= input.lastSeenCreatedAt + existing.lastSeenCreatedAt > input.lastSeenCreatedAt ) { return existing; }