fix: address review of the problem guides

- EventSeats rejects a capacity that is not a whole, non-negative number
  with invalid_capacity, and hold stops at available <= 0. A capacity of
  1.5 let holds drive availability below zero.
- The sync route answers 422 for UnknownOperation, so a device stops
  retrying a queued write for an operation that a server deploy removed.
- The SQL room store uses a RoomState type and guards the stored JSON;
  loadRoom throws for an unexpected shape instead of returning it.
- The cross-process race test kills its child process after each test,
  so a failed run leaves no waiting process.

Each fix started from a failing test. The guides embed the new example
files and describe the new behavior.

Author: cardmagic

docs/guides/offline-first.md

@@ -189,6 +189,7 @@ import {
   IdempotencyConflict,
   InvalidPayload,
   receiveTransmitEnvelope,
+  UnknownOperation,
   type SolidObjectsRuntime,
   type TransmitEnvelope,
 } from "solid-objects"
@@ -209,7 +210,11 @@ export async function handleInspectionSync({
     await receiveTransmitEnvelope({ runtime, envelope })
     return Response.json({})
   } catch (error) {
-    if (error instanceof InvalidPayload || error instanceof IdempotencyConflict) {
+    if (
+      error instanceof InvalidPayload ||
+      error instanceof IdempotencyConflict ||
+      error instanceof UnknownOperation
+    ) {
       return new Response(null, { status: 422 })
     }
     throw error
@@ -223,7 +228,7 @@ Delivery through `receiveTransmitEnvelope` skips `authorizeMessage`. The route m
 
 The example gives those checks to `canWrite`. If that callback denies the request, the route answers 403 and does not enqueue the write. The browser policy cannot replace these server checks.
 
-The route answers 422 for `InvalidPayload` and `IdempotencyConflict`. The browser converts that response to `NonRetryableError` and stops retries for that write. This response prevents repeated attempts for an envelope that cannot apply.
+The route answers 422 for `InvalidPayload`, `IdempotencyConflict`, and `UnknownOperation`. An `UnknownOperation` occurs when a device queued a write for an operation that a later server deploy removed. The browser converts that response to `NonRetryableError` and stops retries for that write. This response prevents repeated attempts for an envelope that cannot apply.
 
 A Rails server can receive the same envelopes with `SolidObjects::Transmission.receive`. That method also skips message authorization. Authenticate the device before the call. Check its permission to write the actor.
 
@@ -265,6 +270,7 @@ The [route tests](../../test/guide-offline-first.test.ts) check these cases:
 - An authorized request receives 200 and adds the expected finding.
 - A repeated envelope receives 200 and leaves one finding.
 - An envelope with an empty operation receives 422.
+- An envelope for an operation that the server removed receives 422.
 
 These tests do not check every browser engine or every interruption during delivery. They also do not test `IdempotencyConflict` or the browser response to 422.
 

docs/guides/persistent-rooms.md

@@ -60,6 +60,8 @@ The test models a restart with two instances of `createMemoryRooms`. It starts a
 ```typescript
 import { DatabaseSync } from "node:sqlite"
 
+export type RoomState = { players: string[]; turnNumber: number }
+
 export function openRoomStore({ path }: { path: string }): DatabaseSync {
   const database = new DatabaseSync(path)
   database.exec(`
@@ -82,7 +84,7 @@ export function saveRoom({
 }: {
   database: DatabaseSync
   roomId: string
-  state: unknown
+  state: RoomState
   expectedVersion: number
   turnDeadline: number | null
 }): { saved: boolean } {
@@ -108,11 +110,13 @@ export function loadRoom({
 }: {
   database: DatabaseSync
   roomId: string
-}): { state: unknown; version: number } | undefined {
+}): { state: RoomState; version: number } | undefined {
   const row = database.prepare("SELECT state, version FROM rooms WHERE room_id = ?").get(roomId) as
     { state: string; version: number } | undefined
   if (!row) return undefined
-  return { state: JSON.parse(row.state), version: row.version }
+  const state: unknown = JSON.parse(row.state)
+  if (!isRoomState(state)) throw new Error(`stored room ${roomId} has an unexpected shape`)
+  return { state, version: row.version }
 }
 
 export function dueTurnDeadlines({
@@ -127,9 +131,16 @@ export function dueTurnDeadlines({
     .all(now) as { room_id: string }[]
   return rows.map((row) => row.room_id)
 }
+
+function isRoomState(value: unknown): value is RoomState {
+  if (typeof value !== "object" || value === null) return false
+  if (!("players" in value) || !Array.isArray(value.players)) return false
+  if (!value.players.every((player) => typeof player === "string")) return false
+  return "turnNumber" in value && Number.isSafeInteger(value.turnNumber)
+}
 ```
 
-The SQL design stores each room as JSON in a row with a `version` column and a turn deadline. `loadRoom` returns the state and its version. The caller passes that version to `saveRoom` as `expectedVersion`.
+The SQL design stores each room as JSON in a row with a `version` column and a turn deadline. `loadRoom` returns the state and its version. It checks the stored JSON against `RoomState` and throws when the shape is unexpected, so the caller never uses unchecked data. The caller passes that version to `saveRoom` as `expectedVersion`.
 
 `saveRoom` updates a row only when its version still matches. A successful update also increases the version. A write from an old version returns `{ saved: false }`. The caller then reloads the room and tries again. An old copy of the state cannot replace a newer copy.
 
@@ -356,7 +367,7 @@ In tests, `runtime.testing.runDueReminders({ now })` finds due reminders at the
 The [tests for persistent rooms](../../test/guide-persistent-rooms.test.ts) check these cases in this order:
 
 1. A new instance of the memory design has no room and no timer after a simulated restart.
-2. The SQL room survives a database reopen. A write from an old version fails, and the sweep finds the due room.
+2. The SQL room survives a database reopen. A write from an old version fails, and the sweep finds the due room. A stored room with an unexpected shape fails to load.
 3. Two submissions for one turn produce one accepted move and one `stale_turn` rejection.
 4. A move from the wrong player rejects with `not_your_turn`.
 5. A caller cannot submit a move for another player.

docs/guides/race-conditions.md

@@ -184,6 +184,9 @@ export class EventSeats extends Actor {
   setCapacity({ capacity, expectedRevision }: { capacity: number; expectedRevision: number }): {
     revision: number
   } {
+    if (!Number.isSafeInteger(capacity) || capacity < 0) {
+      this.reject("invalid_capacity", { message: "Capacity must be a whole number of seats" })
+    }
     if (expectedRevision !== this.revision) {
       this.reject("stale_revision", {
         message: "The event changed after you loaded it",
@@ -202,7 +205,7 @@ export class EventSeats extends Actor {
     if (Object.hasOwn(this.holds, holdId) || Object.hasOwn(this.sold, holdId)) {
       return { held: true, available: this.available }
     }
-    if (this.available === 0) return { held: false, available: 0 }
+    if (this.available <= 0) return { held: false, available: 0 }
 
     const expiresAt = Date.now() + HOLD_MILLISECONDS
     this.holds = { ...this.holds, [holdId]: { buyer, expiresAt } }
@@ -252,7 +255,7 @@ Calls for one event run one at a time, in order, across processes. Different eve
 
 An old form can overwrite newer data even when calls run in order. The calls run in order, but the second call still carries old data. The actor needs a rule that detects this conflict.
 
-`setCapacity` takes `expectedRevision` and compares it with `revision`. A mismatch rejects the change with `stale_revision`. An accepted capacity change increases the revision.
+`setCapacity` takes `expectedRevision` and compares it with `revision`. A mismatch rejects the change with `stale_revision`. An accepted capacity change increases the revision. A capacity that is not a whole, non-negative number rejects with `invalid_capacity`, so a fractional capacity cannot leave a part of a seat to hold.
 
 The test accepts a change from 10 seats to 12 seats. An old form then requests eight seats with the previous revision. The actor rejects that request and keeps the capacity at 12.
 
@@ -337,6 +340,7 @@ The [test file](../../test/guide-race-conditions.test.ts) checks these outcomes:
 - A late expiry after a confirmation changes nothing.
 - A confirmation after expiry rejects with `hold_expired`.
 - A capacity change from a stale form rejects with `stale_revision`.
+- A fractional or negative capacity rejects with `invalid_capacity`.
 - A buyer cannot change capacity.
 
 The assertions check both results and stored state. They show the behavior of these examples and their conflict cases.

examples/guides/offline-first/sync-route.ts

@@ -2,6 +2,7 @@ import {
   IdempotencyConflict,
   InvalidPayload,
   receiveTransmitEnvelope,
+  UnknownOperation,
   type SolidObjectsRuntime,
   type TransmitEnvelope,
 } from "solid-objects"
@@ -22,7 +23,11 @@ export async function handleInspectionSync({
     await receiveTransmitEnvelope({ runtime, envelope })
     return Response.json({})
   } catch (error) {
-    if (error instanceof InvalidPayload || error instanceof IdempotencyConflict) {
+    if (
+      error instanceof InvalidPayload ||
+      error instanceof IdempotencyConflict ||
+      error instanceof UnknownOperation
+    ) {
       return new Response(null, { status: 422 })
     }
     throw error

examples/guides/persistent-rooms/sql-room.ts

@@ -1,5 +1,7 @@
 import { DatabaseSync } from "node:sqlite"
 
+export type RoomState = { players: string[]; turnNumber: number }
+
 export function openRoomStore({ path }: { path: string }): DatabaseSync {
   const database = new DatabaseSync(path)
   database.exec(`
@@ -22,7 +24,7 @@ export function saveRoom({
 }: {
   database: DatabaseSync
   roomId: string
-  state: unknown
+  state: RoomState
   expectedVersion: number
   turnDeadline: number | null
 }): { saved: boolean } {
@@ -48,11 +50,13 @@ export function loadRoom({
 }: {
   database: DatabaseSync
   roomId: string
-}): { state: unknown; version: number } | undefined {
+}): { state: RoomState; version: number } | undefined {
   const row = database.prepare("SELECT state, version FROM rooms WHERE room_id = ?").get(roomId) as
     { state: string; version: number } | undefined
   if (!row) return undefined
-  return { state: JSON.parse(row.state), version: row.version }
+  const state: unknown = JSON.parse(row.state)
+  if (!isRoomState(state)) throw new Error(`stored room ${roomId} has an unexpected shape`)
+  return { state, version: row.version }
 }
 
 export function dueTurnDeadlines({
@@ -67,3 +71,10 @@ export function dueTurnDeadlines({
     .all(now) as { room_id: string }[]
   return rows.map((row) => row.room_id)
 }
+
+function isRoomState(value: unknown): value is RoomState {
+  if (typeof value !== "object" || value === null) return false
+  if (!("players" in value) || !Array.isArray(value.players)) return false
+  if (!value.players.every((player) => typeof player === "string")) return false
+  return "turnNumber" in value && Number.isSafeInteger(value.turnNumber)
+}

examples/guides/race-conditions/event-seats.ts

@@ -21,6 +21,9 @@ export class EventSeats extends Actor {
   setCapacity({ capacity, expectedRevision }: { capacity: number; expectedRevision: number }): {
     revision: number
   } {
+    if (!Number.isSafeInteger(capacity) || capacity < 0) {
+      this.reject("invalid_capacity", { message: "Capacity must be a whole number of seats" })
+    }
     if (expectedRevision !== this.revision) {
       this.reject("stale_revision", {
         message: "The event changed after you loaded it",
@@ -39,7 +42,7 @@ export class EventSeats extends Actor {
     if (Object.hasOwn(this.holds, holdId) || Object.hasOwn(this.sold, holdId)) {
       return { held: true, available: this.available }
     }
-    if (this.available === 0) return { held: false, available: 0 }
+    if (this.available <= 0) return { held: false, available: 0 }
 
     const expiresAt = Date.now() + HOLD_MILLISECONDS
     this.holds = { ...this.holds, [holdId]: { buyer, expiresAt } }

test/guide-offline-first.test.ts

@@ -113,4 +113,17 @@ describe("the inspection sync route", () => {
 
     expect(response.status).toBe(422)
   })
+
+  it("answers 422 for a write to an operation that the server removed", async () => {
+    const runtime = await serverRuntime()
+    const removed = { ...envelope, operation: "removedOperation" }
+
+    const response = await handleInspectionSync({
+      request: syncRequest({ device: "tablet-7", body: removed }),
+      runtime,
+      canWrite,
+    })
+
+    expect(response.status).toBe(422)
+  })
 })

test/guide-persistent-rooms.test.ts

@@ -74,21 +74,21 @@ describe("the native fix with a version column", () => {
     saveRoom({
       database: beforeRestart,
       roomId: "table-1",
-      state: { turn: 0 },
+      state: { players: ["ada"], turnNumber: 0 },
       expectedVersion: 0,
       turnDeadline: 1_000,
     })
     beforeRestart.close()
 
     const afterRestart = openRoomStore({ path })
     const loaded = loadRoom({ database: afterRestart, roomId: "table-1" })
-    expect(loaded).toEqual({ state: { turn: 0 }, version: 1 })
+    expect(loaded).toEqual({ state: { players: ["ada"], turnNumber: 0 }, version: 1 })
 
     expect(
       saveRoom({
         database: afterRestart,
         roomId: "table-1",
-        state: { turn: 1 },
+        state: { players: ["ada"], turnNumber: 1 },
         expectedVersion: 1,
         turnDeadline: 2_000,
       }),
@@ -97,7 +97,7 @@ describe("the native fix with a version column", () => {
       saveRoom({
         database: afterRestart,
         roomId: "table-1",
-        state: { turn: 9 },
+        state: { players: ["ada"], turnNumber: 9 },
         expectedVersion: 1,
         turnDeadline: 3_000,
       }),
@@ -107,6 +107,18 @@ describe("the native fix with a version column", () => {
   })
 })
 
+describe("a stored room with an unexpected shape", () => {
+  it("fails to load instead of returning unchecked state", () => {
+    const database = openRoomStore({ path: join(directory, "store.sqlite3") })
+    database
+      .prepare("INSERT INTO rooms (room_id, state, version, turn_deadline) VALUES (?, ?, 1, NULL)")
+      .run("table-9", JSON.stringify({ players: "ada" }))
+
+    expect(() => loadRoom({ database, roomId: "table-9" })).toThrow("unexpected shape")
+    database.close()
+  })
+})
+
 describe("the TurnRoom actor", () => {
   it("applies one of two submissions for the same turn", async () => {
     const runtime = await startRuntime()

test/guide-race-conditions.test.ts

@@ -25,12 +25,14 @@ const paymentJob: EventCaller = { userId: "payment-job", role: "system" }
 
 let directory: string
 const runtimes: SolidObjectsRuntime[] = []
+const children: ChildProcess[] = []
 
 beforeEach(async () => {
   directory = await mkdtemp(join(tmpdir(), "guide-race-"))
 })
 
 afterEach(async () => {
+  for (const child of children.splice(0)) child.kill()
   for (const runtime of runtimes.splice(0)) await runtime.close()
   await rm(directory, { recursive: true, force: true })
 })
@@ -43,7 +45,7 @@ async function startRuntime(): Promise<SolidObjectsRuntime> {
 }
 
 function startHoldProcess(): ChildProcess {
-  return fork(fileURLToPath(new URL("./fixtures/event-hold-process.ts", import.meta.url)), {
+  const child = fork(fileURLToPath(new URL("./fixtures/event-hold-process.ts", import.meta.url)), {
     execArgv: [
       "--experimental-transform-types",
       "--disable-warning=ExperimentalWarning",
@@ -52,6 +54,8 @@ function startHoldProcess(): ChildProcess {
     ],
     stdio: ["ignore", "inherit", "inherit", "ipc"],
   })
+  children.push(child)
+  return child
 }
 
 function nextMessage(child: ChildProcess): Promise<unknown> {
@@ -266,6 +270,18 @@ describe("the EventSeats actor", () => {
     ).toBe(12)
   })
 
+  it("rejects a capacity that is not a whole number of seats", async () => {
+    const runtime = await startRuntime()
+    const seats = runtime.ref(EventSeats, "concert").with({ authorizationContext: organizer })
+
+    await expect(seats.setCapacity({ capacity: 1.5, expectedRevision: 0 })).rejects.toMatchObject({
+      code: "invalid_capacity",
+    })
+    await expect(seats.setCapacity({ capacity: -1, expectedRevision: 0 })).rejects.toMatchObject({
+      code: "invalid_capacity",
+    })
+  })
+
   it("lets only an organizer change the capacity", async () => {
     const runtime = await startRuntime()