From 112b58611ebcda2e90de2bc09132caa32eac38bc Mon Sep 17 00:00:00 2001 From: muthanii Date: Sun, 2 Aug 2026 09:11:13 +0300 Subject: [PATCH 1/5] fix: compact the Yjs update log instead of growing it forever Every document change appended a row to yjs_updates and nothing ever removed one, so replay cost and storage grew for the life of a board. Snapshots were already being written; they just were not being used to retire the history they contain. Storing a snapshot now prunes in the same transaction. The ordering is what makes it safe: read the update log's high-water mark BEFORE encoding, then delete only at or below that mark. Hocuspocus applies an update to the document before onChange persists it, so any row at or below the mark is provably inside the snapshot encoded afterwards, while rows appended during the encode land above it and survive. Replaying a redundant update is a no-op; losing a live one is not, so the bias is deliberate. Snapshots are capped too, keeping the newest few as a manual fallback. They are the larger leak of the two, each being the whole document rather than one delta. Co-Authored-By: Claude Opus 5 --- apps/ws/src/persistence.ts | 27 ++++++++++- apps/ws/src/stores.ts | 95 ++++++++++++++++++++++++++++++++++---- 2 files changed, 112 insertions(+), 10 deletions(-) diff --git a/apps/ws/src/persistence.ts b/apps/ws/src/persistence.ts index 27c4038..3b69389 100644 --- a/apps/ws/src/persistence.ts +++ b/apps/ws/src/persistence.ts @@ -9,6 +9,13 @@ * conservative: Yjs updates are commutative and idempotent, so re-applying * ones already contained in the snapshot is a no-op, and updates appended * after the snapshot was cut are never lost. + * + * Storing a snapshot also COMPACTS: the updates that snapshot subsumes are + * deleted in the same transaction, so the log is bounded by what arrives + * between two debounced snapshots rather than growing for the life of the + * board. Compaction is deliberately conservative in the same direction as + * replay — it prunes only what a snapshot provably contains, so the failure + * mode is keeping a redundant row, never dropping a live one. */ import type { Extension, @@ -71,9 +78,25 @@ export class PostgresPersistence implements Extension { async onStoreDocument({ document, documentName }: onStoreDocumentPayload): Promise { try { + // Read the high-water mark BEFORE encoding. Hocuspocus applies an update + // to the document before onChange writes it, so every row at or below + // this id is already inside the state encoded next. Rows appended while + // we encode land above the mark and are kept — replaying an update the + // snapshot already contains is a no-op, losing one is not. + const compactThrough = await this.snapshots.latestUpdateId(documentName); const state = Y.encodeStateAsUpdate(document); - await this.snapshots.storeSnapshot(documentName, state); - this.log.debug("snapshot stored", { boardId: documentName, bytes: state.byteLength }); + const { prunedUpdates, prunedSnapshots } = await this.snapshots.storeSnapshot( + documentName, + state, + compactThrough, + ); + this.log.debug("snapshot stored", { + boardId: documentName, + bytes: state.byteLength, + compactThrough, + prunedUpdates, + prunedSnapshots, + }); } catch (error) { this.log.error("failed to store yjs snapshot", { boardId: documentName, diff --git a/apps/ws/src/stores.ts b/apps/ws/src/stores.ts index a88d5c1..61ca61b 100644 --- a/apps/ws/src/stores.ts +++ b/apps/ws/src/stores.ts @@ -7,7 +7,7 @@ * SECURITY: the agents queries select display-safe columns ONLY. Credential * ciphertext, IVs, tags, and signing secrets are never read by this process. */ -import { and, eq, inArray, sql } from "drizzle-orm"; +import { and, eq, inArray, lte, sql } from "drizzle-orm"; import type { Db } from "@quorum/db/client"; import { @@ -28,6 +28,24 @@ import type { VoteValue } from "@quorum/shared/schemas/vote"; import { serializeError, type Logger } from "./log"; +/** Snapshots kept per board beyond the newest, as a manual recovery fallback. */ +const SNAPSHOT_RETENTION = 3; + +/** + * postgres-js returns the deleted rows as an array; other drivers surface a + * `count`/`rowCount`. Read whichever is present so the store does not depend on + * the driver's result shape. + */ +function rowsAffected(result: unknown): number { + if (Array.isArray(result)) return result.length; + if (typeof result === "object" && result !== null) { + const record = result as { count?: unknown; rowCount?: unknown }; + if (typeof record.count === "number") return record.count; + if (typeof record.rowCount === "number") return record.rowCount; + } + return 0; +} + // --------------------------------------------------------------------------- // Interfaces // --------------------------------------------------------------------------- @@ -67,13 +85,35 @@ export interface BoardStore { removeAgent(agentId: string): Promise; } +/** What one compacting snapshot write removed. Zeroes when nothing was pruned. */ +export interface SnapshotWriteResult { + prunedUpdates: number; + prunedSnapshots: number; +} + export interface SnapshotStore { loadLatestSnapshot(boardId: string): Promise<{ seq: number; snapshot: Uint8Array } | null>; /** The full raw update log for the board, oldest first. */ listUpdates(boardId: string): Promise; appendUpdate(boardId: string, update: Uint8Array): Promise; - /** Store a snapshot at the next monotonic seq for the board. */ - storeSnapshot(boardId: string, snapshot: Uint8Array): Promise; + /** + * Highest update id currently stored for the board, or null when the log is + * empty. Read BEFORE encoding a snapshot: Hocuspocus applies an update to the + * document before onChange persists it, so a row at or below this id is + * guaranteed to be inside a snapshot encoded afterwards. Rows appended during + * the encode land above the mark and survive. + */ + latestUpdateId(boardId: string): Promise; + /** + * Store a snapshot at the next monotonic seq, and in the SAME transaction + * drop the updates it subsumes plus snapshots older than the retention + * window. Pass `compactThrough: null` to write without compacting. + */ + storeSnapshot( + boardId: string, + snapshot: Uint8Array, + compactThrough: number | null, + ): Promise; } export interface AuditVote { @@ -214,11 +254,50 @@ export function createStores(db: Db, log: Logger): Stores { await db.insert(yjsUpdates).values({ boardId, update: Buffer.from(update) }); }, - async storeSnapshot(boardId, snapshot) { - await db.insert(yjsSnapshots).values({ - boardId, - snapshot: Buffer.from(snapshot), - seq: sql`coalesce((select max(${yjsSnapshots.seq}) from ${yjsSnapshots} where ${yjsSnapshots.boardId} = ${boardId}), 0) + 1`, + async latestUpdateId(boardId) { + const row = await db.query.yjsUpdates.findFirst({ + columns: { id: true }, + where: (table, operators) => operators.eq(table.boardId, boardId), + orderBy: (table, operators) => operators.desc(table.id), + }); + return row?.id ?? null; + }, + + async storeSnapshot(boardId, snapshot, compactThrough) { + // One transaction: a snapshot must never be pruned against unless it is + // itself durable, or a crash between the two statements loses history. + return db.transaction(async (tx) => { + await tx.insert(yjsSnapshots).values({ + boardId, + snapshot: Buffer.from(snapshot), + seq: sql`coalesce((select max(${yjsSnapshots.seq}) from ${yjsSnapshots} where ${yjsSnapshots.boardId} = ${boardId}), 0) + 1`, + }); + // Nothing to prune when the log is empty, but snapshot retention still + // runs below — an idle board keeps writing snapshots on every store, + // and those are the larger rows. + const prunedUpdates = + compactThrough === null + ? 0 + : rowsAffected( + await tx + .delete(yjsUpdates) + .where(and(eq(yjsUpdates.boardId, boardId), lte(yjsUpdates.id, compactThrough))), + ); + // Keep a few older snapshots as a manual fallback if the newest one is + // ever unreadable; without a cap they are the bigger leak of the two, + // since each is the whole document rather than one delta. + const prunedSnapshots = await tx + .delete(yjsSnapshots) + .where( + and( + eq(yjsSnapshots.boardId, boardId), + lte( + yjsSnapshots.seq, + sql`coalesce((select max(${yjsSnapshots.seq}) from ${yjsSnapshots} where ${yjsSnapshots.boardId} = ${boardId}), 0) - ${SNAPSHOT_RETENTION}`, + ), + ), + ); + return { prunedUpdates, prunedSnapshots: rowsAffected(prunedSnapshots) }; }); }, }; From 3cf78e8cf2be93c41987d39eb9d078283112a467 Mon Sep 17 00:00:00 2001 From: muthanii Date: Sun, 2 Aug 2026 09:11:13 +0300 Subject: [PATCH 2/5] test: cover Yjs replay and update-log compaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PostgresPersistence had no tests. These pin the ordering rule compaction depends on, including the case that matters most: an update appended while the snapshot is being encoded must survive. Writing them caught a real defect — snapshot retention was skipped whenever the update log happened to be empty, so an idle board still accumulated snapshots without bound. Co-Authored-By: Claude Opus 5 --- apps/ws/test/persistence.test.ts | 204 +++++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 apps/ws/test/persistence.test.ts diff --git a/apps/ws/test/persistence.test.ts b/apps/ws/test/persistence.test.ts new file mode 100644 index 0000000..f881592 --- /dev/null +++ b/apps/ws/test/persistence.test.ts @@ -0,0 +1,204 @@ +/** + * Yjs persistence: replay correctness and update-log compaction. + * + * Compaction is the risky half — deleting history that a snapshot does not + * actually contain loses board state permanently, and Yjs will not tell you. + * These tests pin the ordering rule that makes it safe. + */ +import { describe, expect, it, vi } from "vitest"; +import * as Y from "yjs"; + +import { PostgresPersistence } from "../src/persistence"; +import type { Logger } from "../src/log"; +import type { SnapshotStore, SnapshotWriteResult } from "../src/stores"; + +const BOARD = "brd_compaction_test"; + +function silentLogger(): Logger { + const logger: Logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + child: () => logger, + }; + return logger; +} + +interface UpdateRow { + id: number; + update: Uint8Array; +} + +/** + * In-memory SnapshotStore with the same semantics as the Postgres one: + * monotonic update ids, monotonic snapshot seqs, and a compacting write that + * deletes only rows at or below the mark it is given. + */ +function memoryStore() { + let nextUpdateId = 0; + let nextSeq = 0; + const updates: UpdateRow[] = []; + const snapshots: { seq: number; snapshot: Uint8Array }[] = []; + /** Runs between latestUpdateId() and storeSnapshot(), simulating concurrency. */ + let duringEncode: (() => void) | null = null; + + const store: SnapshotStore = { + async loadLatestSnapshot() { + const latest = snapshots.at(-1); + return latest === undefined ? null : { seq: latest.seq, snapshot: latest.snapshot }; + }, + async listUpdates() { + return updates.map((row) => row.update); + }, + async appendUpdate(_boardId, update) { + nextUpdateId += 1; + updates.push({ id: nextUpdateId, update }); + }, + async latestUpdateId() { + const mark = updates.at(-1)?.id ?? null; + duringEncode?.(); + duringEncode = null; + return mark; + }, + async storeSnapshot(_boardId, snapshot, compactThrough): Promise { + nextSeq += 1; + snapshots.push({ seq: nextSeq, snapshot }); + + let prunedUpdates = 0; + if (compactThrough !== null) { + const keep = updates.filter((row) => row.id > compactThrough); + prunedUpdates = updates.length - keep.length; + updates.splice(0, updates.length, ...keep); + } + + const cutoff = nextSeq - 3; + const keptSnapshots = snapshots.filter((row) => row.seq > cutoff); + const prunedSnapshots = snapshots.length - keptSnapshots.length; + snapshots.splice(0, snapshots.length, ...keptSnapshots); + + return { prunedUpdates, prunedSnapshots }; + }, + }; + + return { + store, + updates, + snapshots, + /** Schedule a write that lands after the mark is read but before the snapshot. */ + onEncode(fn: () => void) { + duringEncode = fn; + }, + }; +} + +/** A doc with `text` typed into it, plus the update that produced it. */ +function docWith(text: string): { doc: Y.Doc; update: Uint8Array } { + const doc = new Y.Doc(); + doc.getText("body").insert(0, text); + return { doc, update: Y.encodeStateAsUpdate(doc) }; +} + +describe("PostgresPersistence compaction", () => { + it("prunes the updates a snapshot already contains", async () => { + const mem = memoryStore(); + const persistence = new PostgresPersistence(mem.store, silentLogger()); + const { doc, update } = docWith("hello"); + + await persistence.onChange({ documentName: BOARD, update } as never); + expect(mem.updates).toHaveLength(1); + + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + + expect(mem.updates).toHaveLength(0); + expect(mem.snapshots).toHaveLength(1); + }); + + it("keeps an update that arrives after the mark is taken", async () => { + const mem = memoryStore(); + const persistence = new PostgresPersistence(mem.store, silentLogger()); + const { doc, update } = docWith("hello"); + await persistence.onChange({ documentName: BOARD, update } as never); + + // A collaborator types while the snapshot is being encoded. That row is + // NOT in the snapshot, so pruning it would lose their edit. + const late = docWith("late edit").update; + mem.onEncode(() => { + void mem.store.appendUpdate(BOARD, late); + }); + + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + + expect(mem.updates).toHaveLength(1); + expect(mem.updates[0]?.update).toBe(late); + }); + + it("survives a round trip: state after compaction equals state before", async () => { + const mem = memoryStore(); + const persistence = new PostgresPersistence(mem.store, silentLogger()); + + const doc = new Y.Doc(); + doc.getText("body").insert(0, "first"); + await persistence.onChange({ + documentName: BOARD, + update: Y.encodeStateAsUpdate(doc), + } as never); + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + + // An edit after the snapshot — this one lives only in the update log. + doc.getText("body").insert(5, " and second"); + await persistence.onChange({ + documentName: BOARD, + update: Y.encodeStateAsUpdate(doc), + } as never); + + const reloaded = await persistence.onLoadDocument({ + document: new Y.Doc(), + documentName: BOARD, + } as never); + + expect(reloaded.getText("body").toString()).toBe("first and second"); + }); + + it("keeps the newest snapshots and drops the rest", async () => { + const mem = memoryStore(); + const persistence = new PostgresPersistence(mem.store, silentLogger()); + const { doc } = docWith("x"); + + for (let i = 0; i < 8; i += 1) { + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + } + + expect(mem.snapshots).toHaveLength(3); + expect(mem.snapshots.at(-1)?.seq).toBe(8); + }); + + it("writes the snapshot but prunes nothing when the log is empty", async () => { + const mem = memoryStore(); + const persistence = new PostgresPersistence(mem.store, silentLogger()); + const { doc } = docWith("x"); + + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + + expect(mem.snapshots).toHaveLength(1); + expect(mem.updates).toHaveLength(0); + }); + + it("leaves the log intact when the snapshot write fails", async () => { + const mem = memoryStore(); + const failing: SnapshotStore = { + ...mem.store, + async storeSnapshot() { + throw new Error("postgres is down"); + }, + }; + const persistence = new PostgresPersistence(failing, silentLogger()); + const { doc, update } = docWith("hello"); + await persistence.onChange({ documentName: BOARD, update } as never); + + // Must not throw — a failed snapshot cannot take the board down. + await persistence.onStoreDocument({ document: doc, documentName: BOARD } as never); + + expect(mem.updates).toHaveLength(1); + }); +}); From 19508da1dcba953f1efe37733cc023322c7da1ce Mon Sep 17 00:00:00 2001 From: muthanii Date: Sun, 2 Aug 2026 09:11:13 +0300 Subject: [PATCH 3/5] ci: actually enforce the 250KB bundle budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build step's comment claimed it enforced the budget. Nothing did — it ran `next build` and checked only that the build succeeded, so the board route could have crossed the limit silently. Next reports First Load JS gzipped (verified: a chunk reported as 54.2 kB measures 53.0 KB gzipped and 169 KB raw), so its own figures compare directly against the budget. The gate parses them rather than recomputing from .next/, which keeps the number CI enforces identical to the number the table shows. `shell: bash` on the build step is load-bearing: it brings `-eo pipefail`, so piping through tee cannot mask a failed build. Current state: 15 routes, worst is /b/[boardId] at 245.0 kB — 5 kB of headroom. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 17 ++++- .gitignore | 3 + apps/web/scripts/check-bundle-budget.mjs | 80 ++++++++++++++++++++++++ eslint.config.mjs | 7 +++ 4 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 apps/web/scripts/check-bundle-budget.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71b63ab..735c77e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,8 +31,9 @@ jobs: - run: pnpm test - # Catches env-parse regressions and enforces the 250KB bundle budget. - # Values are throwaway: nothing here connects to a real service. + # Catches env-parse regressions. Values are throwaway: nothing here + # connects to a real service. The build output is captured so the next + # step can enforce the bundle budget against Next's own figures. - name: Build web working-directory: apps/web env: @@ -42,4 +43,14 @@ jobs: CREDENTIALS_ENCRYPTION_KEY: Y2ktcGxhY2Vob2xkZXItbm90LWEtcmVhbC1zZWNyZXQ= NEXT_PUBLIC_WS_URL: ws://localhost:3001 NEXT_PUBLIC_APP_URL: http://localhost:3000 - run: pnpm build + # `shell: bash` is load-bearing: it runs with `-eo pipefail`, so a failed + # build still fails the step. The default shell would report tee's exit + # code and let a broken build through. + shell: bash + run: pnpm build | tee build.log + + # The 250KB gzipped initial-JS budget (CLAUDE.md §1). Previously the + # comment above claimed the build enforced this; nothing did. + - name: Check bundle budget + working-directory: apps/web + run: node scripts/check-bundle-budget.mjs build.log diff --git a/.gitignore b/.gitignore index a588693..23c1fbb 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ blob-report/ .DS_Store *.log .gstack/ + +# Captured build output for the CI bundle-budget gate. +build.log diff --git a/apps/web/scripts/check-bundle-budget.mjs b/apps/web/scripts/check-bundle-budget.mjs new file mode 100644 index 0000000..b31f172 --- /dev/null +++ b/apps/web/scripts/check-bundle-budget.mjs @@ -0,0 +1,80 @@ +#!/usr/bin/env node +/** + * Enforce the initial-JS budget from CLAUDE.md §1: <250KB gzipped. + * + * Reads the route table Next prints at the end of `next build` and fails if any + * route's First Load JS exceeds the budget. Next's reported figures are + * gzipped (verified: a chunk reported as 54.2 kB measures 53.0 KB under gzip + * and 169 KB raw), so they compare directly against the budget with no + * conversion. + * + * Parsing Next's own output rather than recomputing from .next/ is deliberate: + * the number the team reads in CI is then exactly the number being enforced, + * and there is no second implementation of "which chunks count" to drift. + * + * Usage: next build | tee build.log && node scripts/check-bundle-budget.mjs build.log + */ +import { readFileSync } from "node:fs"; + +const BUDGET_KB = 250; + +const logPath = process.argv[2]; +if (!logPath) { + console.error("usage: check-bundle-budget.mjs "); + process.exit(2); +} + +const text = readFileSync(logPath, "utf8"); + +/** "245 kB" / "1.2 MB" / "149 B" → KB. */ +function toKb(value, unit) { + const n = Number(value); + if (unit === "B") return n / 1024; + if (unit === "kB") return n; + if (unit === "MB") return n * 1024; + return Number.NaN; +} + +// Route rows look like: ├ ƒ /b/[boardId] 107 kB 245 kB +// The LAST size on the row is First Load JS; the first is the route's own JS. +const ROUTE_ROW = /^[┌├└]\s+[^\s]*\s*(\/\S*)\s+.*?([\d.]+)\s*(B|kB|MB)\s*$/; + +const routes = []; +for (const line of text.split("\n")) { + const match = ROUTE_ROW.exec(line.trimEnd()); + if (match) routes.push({ route: match[1], firstLoadKb: toKb(match[2], match[3]) }); +} + +if (routes.length === 0) { + console.error( + "check-bundle-budget: found no route rows in the build output.\n" + + "Did the build succeed, and was its stdout captured to the file passed in?", + ); + process.exit(2); +} + +const over = routes.filter((r) => r.firstLoadKb > BUDGET_KB); +const worst = routes.reduce((a, b) => (b.firstLoadKb > a.firstLoadKb ? b : a)); + +for (const { route, firstLoadKb } of over) { + console.error(`OVER BUDGET ${route} ${firstLoadKb.toFixed(1)} kB > ${BUDGET_KB} kB gzipped`); +} + +if (over.length > 0) { + console.error( + `\n${over.length} route(s) exceed the ${BUDGET_KB}KB initial-JS budget (CLAUDE.md §1).\n` + + "Run `pnpm analyze` to see what landed in the bundle.", + ); + process.exit(1); +} + +const headroom = BUDGET_KB - worst.firstLoadKb; +console.log( + `bundle budget OK — ${routes.length} routes, worst is ${worst.route} at ` + + `${worst.firstLoadKb.toFixed(1)} kB gzipped (${headroom.toFixed(1)} kB headroom).`, +); +if (headroom < 15) { + console.log( + `NOTE: under 15 kB of headroom. Check \`pnpm analyze\` before adding a client dependency.`, + ); +} diff --git a/eslint.config.mjs b/eslint.config.mjs index 8afd58b..3947950 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -17,6 +17,13 @@ export default tseslint.config( }, js.configs.recommended, ...tseslint.configs.recommended, + { + // Build/CI scripts run under plain Node, so they legitimately use the Node + // globals the browser/TS config knows nothing about. Declared inline rather + // than pulling in the `globals` package for two names. + files: ["**/scripts/**/*.mjs"], + languageOptions: { globals: { process: "readonly", console: "readonly" } }, + }, { rules: { "@typescript-eslint/no-explicit-any": "error", From 5ead1ec4db5f1b197ef5e5b55adba0c1b2aaabd1 Mon Sep 17 00:00:00 2001 From: muthanii Date: Sun, 2 Aug 2026 09:11:13 +0300 Subject: [PATCH 4/5] docs: correct the Known gaps list Two entries were no longer true and one is now fixed: rate limiting shipped in 667b1ac, the e2e suite runs (18/18, repeatedly), and the Yjs update log is compacted as of this branch. A gaps list that understates the product is as misleading as one that overstates it. Replaced with the gaps that are actually open, including several surfaced by recent QA and DX passes but never written down: ws as a single point of failure, the shared rate-limit bucket when no proxy header is present, the absent Y.Doc migration path, and agent-protocol being unpublished with no version on the wire. Co-Authored-By: Claude Opus 5 --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3273200..20f00d9 100644 --- a/README.md +++ b/README.md @@ -211,11 +211,12 @@ The Playwright suite needs the stack running (`docker compose up -d` and `pnpm d Being honest about what is not done: -- **No rate limiting.** `POST /api/boards` will mint a new guest identity and board on every unauthenticated request. This needs a Redis-backed limiter before any public deployment. -- **The e2e suite has never been executed.** It is written and typechecks, but has not been run against live services. -- **Never deployed.** No production environment has been stood up; the deploy targets in the stack table are intended, not proven. -- **Bundle headroom is thin.** The board route is 245KB first-load JS against a 250KB budget. Run `pnpm analyze` before adding client-side dependencies. -- **The Yjs update log grows unbounded.** Snapshots are written, but the append-only update log has no compaction yet. +- **Never deployed.** No production environment has been stood up; the deploy targets in the stack table are intended, not proven. [`DEPLOYING.md`](DEPLOYING.md) is a runbook that has never been executed end to end, and nothing in it is covered by a test. +- **Bundle headroom is thin.** The board route is 245KB first-load JS against the 250KB gzipped budget — 5KB, or 2%. CI now fails if any route crosses it, but the next client-side dependency is very likely to. Run `pnpm analyze` first. +- **`ws` is a single point of failure.** One Fly machine by design: board state lives in that process's memory, and sharing it across instances needs `@hocuspocus/extension-redis`, which is not installed. A deploy or crash disconnects every board. Clients reconnect and Yjs re-syncs, but in-flight awareness is lost. +- **Rate limits share one bucket without a proxy header.** `clientIpFromRequest` falls back to a literal `"unknown"`, so every caller that arrives without `x-forwarded-for` is counted together. Vercel always sets it, so the deployed web path is fine — but the failure mode is a global lockout rather than blocking one abuser. +- **No Y.Doc schema migration path.** Live documents cannot be recreated from Postgres, and there is no mechanism for evolving their shape. This is the constraint most likely to hurt later. +- **`@quorum/agent-protocol` is not published.** It is dependency-free and meant to be vendored, but there is no npm package and no version field on the wire, so an agent cannot detect contract drift. ## Contributing From 894792d59c5ea118cfde3d44b9cbc3084673193c Mon Sep 17 00:00:00 2001 From: muthanii Date: Sun, 2 Aug 2026 09:20:38 +0300 Subject: [PATCH 5/5] fix: compaction reported pruning nothing while actually pruning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught by reading the live ws log against the row counts in Postgres: "prunedUpdates: 0, prunedSnapshots: 0" on every write, while the tables showed the log emptied and snapshots capped. The deletes were correct; the telemetry was lying. postgres-js returns an Array SUBCLASS whose real figure is on `.count` and whose `.length` is 0 without a RETURNING clause, so the Array.isArray branch matched first and reported zero every time. Checking count/rowCount before the array fallback fixes it — now the same run logs prunedUpdates: 1 alongside a table that agrees. The unit tests missed this because the in-memory fake returns plain values and never modelled the driver's result shape, so the regression test pins the shape itself. Co-Authored-By: Claude Opus 5 --- apps/ws/src/stores.ts | 13 ++++--- apps/ws/test/stores.regression-1.test.ts | 43 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 apps/ws/test/stores.regression-1.test.ts diff --git a/apps/ws/src/stores.ts b/apps/ws/src/stores.ts index 61ca61b..06d515a 100644 --- a/apps/ws/src/stores.ts +++ b/apps/ws/src/stores.ts @@ -32,17 +32,20 @@ import { serializeError, type Logger } from "./log"; const SNAPSHOT_RETENTION = 3; /** - * postgres-js returns the deleted rows as an array; other drivers surface a - * `count`/`rowCount`. Read whichever is present so the store does not depend on - * the driver's result shape. + * Rows affected by a DELETE, across driver result shapes. + * + * Order matters: postgres-js returns an Array SUBCLASS that carries the real + * figure on `.count`, and without a RETURNING clause its `.length` is 0. An + * `Array.isArray` check first therefore reports every delete as having removed + * nothing — which is exactly what the compaction log claimed before this. */ -function rowsAffected(result: unknown): number { - if (Array.isArray(result)) return result.length; +export function rowsAffected(result: unknown): number { if (typeof result === "object" && result !== null) { const record = result as { count?: unknown; rowCount?: unknown }; if (typeof record.count === "number") return record.count; if (typeof record.rowCount === "number") return record.rowCount; } + if (Array.isArray(result)) return result.length; return 0; } diff --git a/apps/ws/test/stores.regression-1.test.ts b/apps/ws/test/stores.regression-1.test.ts new file mode 100644 index 0000000..d95e219 --- /dev/null +++ b/apps/ws/test/stores.regression-1.test.ts @@ -0,0 +1,43 @@ +/** + * Regression: compaction reported "prunedUpdates: 0" on every write while + * actually deleting rows, because postgres-js returns an Array SUBCLASS whose + * `.length` is 0 without a RETURNING clause and whose real figure sits on + * `.count`. An Array.isArray branch that ran first swallowed it, so the log + * said nothing was ever pruned. + * Found by /devex-review follow-up on 2026-08-02 by reading live ws output + * against the actual row counts in Postgres. + */ +import { describe, expect, it } from "vitest"; + +import { rowsAffected } from "../src/stores"; + +/** postgres-js: an Array subclass carrying `count`, with no rows in it. */ +class PostgresJsResult extends Array { + count: number; + constructor(count: number) { + super(); + this.count = count; + } +} + +describe("rowsAffected", () => { + it("reads count off a postgres-js result even though it is an empty array", () => { + expect(rowsAffected(new PostgresJsResult(7))).toBe(7); + }); + + it("still reports zero when a postgres-js delete matched nothing", () => { + expect(rowsAffected(new PostgresJsResult(0))).toBe(0); + }); + + it("reads rowCount from a node-postgres style result", () => { + expect(rowsAffected({ rowCount: 4 })).toBe(4); + }); + + it("falls back to array length for drivers that return the deleted rows", () => { + expect(rowsAffected([{ id: 1 }, { id: 2 }])).toBe(2); + }); + + it.each([null, undefined, 0, "3", {}])("returns 0 for the unusable result %j", (value) => { + expect(rowsAffected(value)).toBe(0); + }); +});