From abacb9634efec4ddfb48c590f0c28cb1f4c74467 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 7 Oct 2026 14:55:12 +0000 Subject: [PATCH 1/2] fix(workspace): ignore empty outgoing messages in durable acks Hocuspocus passes empty outgoing messages to onOutgoingMessage. The workspace editor decoded each one in durableMutationFromMessage, which threw "Unexpected end of array" as an uncaught page error. Empty messages carry no document write, so the function now returns null. Adds the workspace-autosave verify recipe, which holds one save_workspace_artifact call in flight while typing continues in Crossfire and Sparring opened from a workspace, and checks that the page sends no other save meanwhile and that Postgres keeps the newest text (the ordering fix from #89). Workspaces need the collaboration service, so `verify.sh up` now records VERIFY_COLLAB in drive.env and the recipe skips with a reason unless the stack ran `up --collab`. Co-Authored-By: Claude Opus 5.5 (1M context) --- .claude/skills/verify-debatelab/SKILL.md | 3 + .claude/skills/verify-debatelab/bin/verify.sh | 2 + .../verify-debatelab/features/README.md | 1 + .../features/workspace-autosave.md | 30 +++++ .../recipes/workspace-autosave.spec.ts | 126 ++++++++++++++++++ src/lib/workspace/durable-ack.ts | 2 + src/test/workspace/durable-ack.test.ts | 10 ++ 7 files changed, 174 insertions(+) create mode 100644 .claude/skills/verify-debatelab/features/workspace-autosave.md create mode 100644 .claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts create mode 100644 src/test/workspace/durable-ack.test.ts diff --git a/.claude/skills/verify-debatelab/SKILL.md b/.claude/skills/verify-debatelab/SKILL.md index c23f5c4cc..2f5aa8ad1 100644 --- a/.claude/skills/verify-debatelab/SKILL.md +++ b/.claude/skills/verify-debatelab/SKILL.md @@ -93,10 +93,13 @@ Every result, source and practice round belongs to a Current Debate or is explic $H drive all # every recipe, both viewports $H drive recipes/ai-tools --project desktop-chromium # one file, one viewport $H drive e2e/full-stack/billing # a maintained full-stack spec +$H drive recipes/workspace-autosave # needs up --collab; skips on a plain up $H drive e2e/workspace-collaboration # starts its own collaboration process; do not use up --collab $H drive e2e/live/ --project desktop-chromium # live mode only ``` +`recipes/workspace-autosave` needs the collaboration service from `up --collab` and skips with that reason on any other stack, including in `drive all`. `e2e/workspace-collaboration` is the opposite: it starts its own service, so drive it on a stack without `--collab`. + Fake mode also runs `e2e/full-stack/`. Live mode runs the recipes, `e2e/live/`, and only the provider-free `e2e/full-stack/` tests (the config's `grepInvert` skips the ones asserting fake text). To prove a feature without a recipe, copy `recipes/case-generation.spec.ts` to `recipes/.spec.ts` and use the selectors from the matching file in `features/`. Recipes are maintained files. Keep the ones worth keeping. diff --git a/.claude/skills/verify-debatelab/bin/verify.sh b/.claude/skills/verify-debatelab/bin/verify.sh index cbc82ce8a..5a1a5e47d 100755 --- a/.claude/skills/verify-debatelab/bin/verify.sh +++ b/.claude/skills/verify-debatelab/bin/verify.sh @@ -163,6 +163,8 @@ cmd_up() { printf 'VERIFY_MODE=%s\n' "$([ "$live" = true ] && echo live || echo fake)" printf 'LIVE_MODEL=%s\n' "$([ "$live" = true ] && echo "$LIVE_MODEL")" printf 'VERIFY_STRIPE=%s\n' "$([ -n "$stripe_key" ] && echo test || echo placeholder)" + # Recipes that need the collaboration service skip unless `up --collab` started it. + printf 'VERIFY_COLLAB=%s\n' "$collab" [ -z "$stripe_key" ] || printf 'STRIPE_TEST_SECRET_KEY=%s\n' "$stripe_key" # e2e/workspace-collaboration.spec.ts starts its own collaboration process with these. printf 'COLLABORATION_DATABASE_URL=%s\n' "$db_url" diff --git a/.claude/skills/verify-debatelab/features/README.md b/.claude/skills/verify-debatelab/features/README.md index 6fb8e1f13..89c97d1a2 100644 --- a/.claude/skills/verify-debatelab/features/README.md +++ b/.claude/skills/verify-debatelab/features/README.md @@ -46,6 +46,7 @@ Each feature file starts with an H1 title and one paragraph describing the user- - [Current Debate](./current-debate.md): start, the first-debate offer and the different-motion choice, sources and results scoped to a debate, switching, and the first onboarding step (`recipes/current-debate.spec.ts`); debate context review and pins (`e2e/full-stack/debate-context.spec.ts`). - [Practice](./practice.md): a full Lincoln-Douglas sparring round with verdict, and Crossfire in both directions with debrief, each filed in its debate (`recipes/practice.spec.ts`). - [Voice rounds](./voice-rounds.md): spoken Sparring and Crossfire on the browser voice, with no ElevenLabs key (`recipes/voice-rounds.spec.ts`). +- [Workspace autosave](./workspace-autosave.md): a slow Crossfire or Sparring save from a Workspace never overwrites newer text (`recipes/workspace-autosave.spec.ts`). Needs `up --collab`; skips without it. - [Account data](./account-data.md): Download my data and Delete account, each proven against a bystander (`recipes/account-data.spec.ts`); Delete All My Data scoped against a bystander (`e2e/full-stack/account-data.spec.ts`). - Account: output language reaching the model from a new device, flowchart node delete, save and load, and the speech timer (`recipes/account.spec.ts`). - Billing: expired account blocked, promo code restores access (`recipes/billing.spec.ts`); signed, forged, and replayed Stripe webhooks (`e2e/full-stack/billing.spec.ts`). diff --git a/.claude/skills/verify-debatelab/features/workspace-autosave.md b/.claude/skills/verify-debatelab/features/workspace-autosave.md new file mode 100644 index 000000000..f891d0447 --- /dev/null +++ b/.claude/skills/verify-debatelab/features/workspace-autosave.md @@ -0,0 +1,30 @@ +# Workspace autosave + +Crossfire and Sparring opened from a Workspace save their state to `workspace_artifacts` through the `save_workspace_artifact` RPC on every change. Saves can finish out of order, and a slow older save must not overwrite a newer one: the row ends with the newest text. Workspaces are hidden in production unless `VITE_COLLABORATION_URL` is set, so this is verifiable only on the local stack. + +## Sub-features + +- `ws-crossfire-autosave` keeps the newest Crossfire resolution when an older save lands last. +- `ws-sparring-autosave` keeps the newest Sparring speech draft when an older save lands last. + +## How to get to it (user POV) + +- `/app/workspaces` → fill `Motion` → `Create workspace` → open a round (`R1`) → expand `Practice` in the working panel → `Crossfire` or `Sparring`. The tool shows `Shared workspace session`. + +## Driving it with verify.sh + Playwright + +Preconditions: + +- The stack was started with `bin/verify.sh up --collab`. On a stack without it the recipe skips both tests with the reason `Workspaces need the collaboration service: run verify.sh up --collab`, so `drive all` stays green on a plain `up`. Report a skip as unverified. +- Doctor passes. The recipe is `recipes/workspace-autosave.spec.ts`. + +- **Open the tool.** Create a workspace for the motion, click `R1`, and on a phone click `Working panel` first (one pane at a time). Open the `Practice` details if closed and click `Crossfire` or `Sparring` (`exact: true`). +- **Hold one save.** `page.route("**/rest/v1/rpc/save_workspace_artifact")` holds the first request whose `p_progress` matches a partial value (Crossfire: a resolution ending in ` n`; Sparring: a draft of `Coding`) while typing continues. The page keeps one save in flight, so no other save may be sent while it is held (`sentWhileHeld()` is 0); newer changes wait in the queue. +- **Release and check.** Let the held save through and wait for its answer. Polling `admin.from("workspace_artifacts")` for the workspace and tool must return one row with the full text, and it must still hold that text once every save the page sent has been answered. +- **Sparring.** Click `Start round` and wait for the textbox `Draft for ` (`AI_TIMEOUT`). Click `Pause clock` before typing, because a running clock saves every tick and would hide a stale save. + +## Gotchas + +- `e2e/workspace-collaboration.spec.ts` starts its own collaboration process on port 12345 and must not run against `up --collab`. This recipe is the opposite: it needs `up --collab` and skips without it. Don't drive both on one stack. +- `up` always gives the client `VITE_COLLABORATION_URL`, so Workspace routes render without `--collab` but the editor never connects. The recipe keys its skip off `VERIFY_COLLAB` in `drive.env`, not off the page. +- The Workspace editor reports uncaught page errors through the fixture. Hocuspocus hands empty outgoing messages to `onOutgoingMessage`; `durableMutationFromMessage` ignores them (`src/lib/workspace/durable-ack.ts`). diff --git a/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts b/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts new file mode 100644 index 000000000..953117ead --- /dev/null +++ b/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts @@ -0,0 +1,126 @@ +import type { Page, Request, Route } from "@playwright/test"; +import { admin, AI_TIMEOUT, expect, test } from "./fixtures"; + +// Crossfire and Sparring launched from a workspace save their state to +// workspace_artifacts on every change. A slow save must not land after a newer +// one and leave older text in the row (fixed in #89). Each recipe holds one +// save in flight while typing continues, then checks the row has the newest text. +// Workspaces need the collaboration service, which only `verify.sh up --collab` starts. +test.skip(process.env.VERIFY_COLLAB !== "true", "Workspaces need the collaboration service: run verify.sh up --collab"); + +const motion = "This House would require coding classes in every secondary school."; + +type SaveBody = { p_progress: Record }; + +async function openWorkspaceTool(page: Page, tool: "Crossfire" | "Sparring") { + await page.goto("/app/workspaces"); + await page.getByRole("textbox", { name: "Motion" }).fill(motion); + await page.getByRole("button", { name: "Create workspace" }).click(); + await expect(page).toHaveURL(/\/app\/workspaces\/[0-9a-f-]+$/, { timeout: 20_000 }); + const workspaceId = page.url().split("/").at(-1)!; + await page.getByRole("button", { name: "R1" }).click(); + // On a phone the workspace shows one pane at a time. + const workingPanel = page.getByRole("button", { name: "Working panel", exact: true }); + if (await workingPanel.isVisible()) await workingPanel.click(); + const practice = page.locator("summary").filter({ hasText: "Practice" }); + if (!await practice.evaluate((element) => (element.parentElement as HTMLDetailsElement).open)) await practice.click(); + await page.getByRole("button", { name: tool, exact: true }).click(); + await expect(page.getByText(/Shared workspace session/)).toBeVisible(); + return workspaceId; +} + +/** + * Holds the first save whose progress matches `slow` until `release()`. The page + * sends one save at a time, so `sentWhileHeld()` must stay empty; if it does not, + * the held save would reach the database after newer ones and store older text. + */ +async function holdSave(page: Page, slow: (progress: Record) => boolean) { + let held: Request | undefined; + let holding = false; + const sent: Request[] = []; + const whileHeld: Request[] = []; + let release!: () => void; + const released = new Promise((resolve) => { release = resolve; }); + await page.route("**/rest/v1/rpc/save_workspace_artifact", async (route: Route) => { + const request = route.request(); + sent.push(request); + const body = request.postDataJSON() as SaveBody; + if (!held && slow(body.p_progress)) { + held = request; + holding = true; + await released; + } else if (holding) { + whileHeld.push(request); + } + await route.continue(); + }); + return { + wasHeld: () => Boolean(held), + sentWhileHeld: () => whileHeld.length, + /** Lets the held save through and waits for its answer. */ + release: async () => { + release(); + await held!.response(); + holding = false; + }, + /** Waits until every save the page has sent so far has an answer. */ + settled: () => Promise.all(sent.map((request) => request.response())), + }; +} + +async function storedProgress(workspaceId: string, tool: string) { + const { data, error } = await admin.from("workspace_artifacts").select("progress,request_id").eq("workspace_id", workspaceId).eq("tool", tool); + if (error) throw error; + return data; +} + +test("ws-crossfire-autosave: a slow Crossfire save does not overwrite newer text", async ({ signedIn: page, evidence }) => { + const workspaceId = await openWorkspaceTool(page, "Crossfire"); + const resolution = page.getByTestId("crossfire-resolution"); + await expect(resolution).toHaveValue(motion); + + const saves = await holdSave(page, (progress) => String((progress.config as { resolution?: string })?.resolution).endsWith(" n")); + await resolution.focus(); + await resolution.press("Control+End"); + await resolution.pressSequentially(" now, starting next year", { delay: 40 }); + const expected = `${motion} now, starting next year`; + await expect(resolution).toHaveValue(expected); + expect(saves.wasHeld()).toBe(true); + expect(saves.sentWhileHeld()).toBe(0); + await saves.release(); + await evidence.step("Crossfire: held one save in flight while the resolution kept changing"); + + const resolutions = async () => (await storedProgress(workspaceId, "crossfire")) + .map((row) => (row.progress as { config: { resolution: string } }).config.resolution); + await expect.poll(resolutions, { timeout: 15_000 }).toEqual([expected]); + await saves.settled(); + expect(await resolutions()).toEqual([expected]); + await evidence.shot("ws-crossfire-autosave"); +}); + +test("ws-sparring-autosave: a slow Sparring save does not overwrite a newer draft", async ({ signedIn: page, evidence }) => { + const workspaceId = await openWorkspaceTool(page, "Sparring"); + await expect(page.getByLabel("Motion")).toHaveValue(motion); + await page.getByRole("button", { name: "Start round" }).click(); + const draft = page.getByRole("textbox", { name: /^Draft for / }); + await expect(draft).toBeVisible({ timeout: AI_TIMEOUT }); + // A running clock saves the session every tick, which would hide a stale save. + const pause = page.getByRole("button", { name: "Pause clock" }); + if (await pause.isVisible()) await pause.click(); + await expect(page.getByRole("button", { name: "Resume clock" })).toBeVisible(); + + const saves = await holdSave(page, (progress) => progress.draft === "Coding"); + const text = "Coding classes teach every student to break problems into steps."; + await draft.pressSequentially(text, { delay: 40 }); + await expect(draft).toHaveValue(text); + expect(saves.wasHeld()).toBe(true); + expect(saves.sentWhileHeld()).toBe(0); + await saves.release(); + await evidence.step("Sparring: held one save in flight while the speech draft kept changing"); + + const drafts = async () => (await storedProgress(workspaceId, "sparring")).map((row) => (row.progress as { draft: string }).draft); + await expect.poll(drafts, { timeout: 15_000 }).toEqual([text]); + await saves.settled(); + expect(await drafts()).toEqual([text]); + await evidence.shot("ws-sparring-autosave"); +}); diff --git a/src/lib/workspace/durable-ack.ts b/src/lib/workspace/durable-ack.ts index ca1450fbb..8c719eae2 100644 --- a/src/lib/workspace/durable-ack.ts +++ b/src/lib/workspace/durable-ack.ts @@ -1,6 +1,8 @@ import { createDecoder, readVarString, readVarUint, readVarUint8Array } from "lib0/decoding"; export async function durableMutationFromMessage(message: Uint8Array) { + // Hocuspocus also reports empty outgoing messages; none of them is a document write. + if (message.byteLength === 0) return null; const decoder = createDecoder(message); readVarString(decoder); if (readVarUint(decoder) !== 0) return null; diff --git a/src/test/workspace/durable-ack.test.ts b/src/test/workspace/durable-ack.test.ts new file mode 100644 index 000000000..74a894e0b --- /dev/null +++ b/src/test/workspace/durable-ack.test.ts @@ -0,0 +1,10 @@ +import { describe, expect, it } from "vitest"; +import { durableMutationFromMessage } from "@/lib/workspace/durable-ack"; + +describe("workspace durable acknowledgements", () => { + // Hocuspocus passes empty outgoing messages to onOutgoingMessage; decoding one + // threw "Unexpected end of array" as an uncaught page error in the workspace editor. + it("ignores the empty messages the provider reports as outgoing", async () => { + await expect(durableMutationFromMessage(new Uint8Array())).resolves.toBeNull(); + }); +}); From fd542e80d4a1a8e36fd74413628748239a92b011 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 7 Oct 2026 15:01:18 +0000 Subject: [PATCH 2/2] test(verify): hold the autosave only after its value is sent The save queue keeps only the newest pending save, so typing straight past the held value could merge it away and fail wasHeld() on a correct app. Type up to the value, wait until it is held, then type the rest. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../verify-debatelab/features/workspace-autosave.md | 2 +- .../recipes/workspace-autosave.spec.ts | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.claude/skills/verify-debatelab/features/workspace-autosave.md b/.claude/skills/verify-debatelab/features/workspace-autosave.md index f891d0447..d3e618600 100644 --- a/.claude/skills/verify-debatelab/features/workspace-autosave.md +++ b/.claude/skills/verify-debatelab/features/workspace-autosave.md @@ -19,7 +19,7 @@ Preconditions: - Doctor passes. The recipe is `recipes/workspace-autosave.spec.ts`. - **Open the tool.** Create a workspace for the motion, click `R1`, and on a phone click `Working panel` first (one pane at a time). Open the `Practice` details if closed and click `Crossfire` or `Sparring` (`exact: true`). -- **Hold one save.** `page.route("**/rest/v1/rpc/save_workspace_artifact")` holds the first request whose `p_progress` matches a partial value (Crossfire: a resolution ending in ` n`; Sparring: a draft of `Coding`) while typing continues. The page keeps one save in flight, so no other save may be sent while it is held (`sentWhileHeld()` is 0); newer changes wait in the queue. +- **Hold one save.** `page.route("**/rest/v1/rpc/save_workspace_artifact")` holds the first request whose `p_progress` matches a partial value (Crossfire: a resolution ending in ` n`; Sparring: a draft of `Coding`). The recipe types up to that value and waits until the save is held before typing the rest: the queue keeps only the newest pending save, so a value typed straight through can be merged away and never sent. The page keeps one save in flight, so no other save may be sent while it is held (`sentWhileHeld()` is 0); newer changes wait in the queue. - **Release and check.** Let the held save through and wait for its answer. Polling `admin.from("workspace_artifacts")` for the workspace and tool must return one row with the full text, and it must still hold that text once every save the page sent has been answered. - **Sparring.** Click `Start round` and wait for the textbox `Draft for ` (`AI_TIMEOUT`). Click `Pause clock` before typing, because a running clock saves every tick and would hide a stale save. diff --git a/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts b/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts index 953117ead..623485010 100644 --- a/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts +++ b/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts @@ -82,10 +82,12 @@ test("ws-crossfire-autosave: a slow Crossfire save does not overwrite newer text const saves = await holdSave(page, (progress) => String((progress.config as { resolution?: string })?.resolution).endsWith(" n")); await resolution.focus(); await resolution.press("Control+End"); - await resolution.pressSequentially(" now, starting next year", { delay: 40 }); + // The queue keeps only the newest pending save, so stop at the held value until it is sent. + await resolution.pressSequentially(" n", { delay: 40 }); + await expect.poll(saves.wasHeld).toBe(true); + await resolution.pressSequentially("ow, starting next year", { delay: 40 }); const expected = `${motion} now, starting next year`; await expect(resolution).toHaveValue(expected); - expect(saves.wasHeld()).toBe(true); expect(saves.sentWhileHeld()).toBe(0); await saves.release(); await evidence.step("Crossfire: held one save in flight while the resolution kept changing"); @@ -111,9 +113,11 @@ test("ws-sparring-autosave: a slow Sparring save does not overwrite a newer draf const saves = await holdSave(page, (progress) => progress.draft === "Coding"); const text = "Coding classes teach every student to break problems into steps."; - await draft.pressSequentially(text, { delay: 40 }); + // The queue keeps only the newest pending save, so stop at the held value until it is sent. + await draft.pressSequentially("Coding", { delay: 40 }); + await expect.poll(saves.wasHeld).toBe(true); + await draft.pressSequentially(text.slice("Coding".length), { delay: 40 }); await expect(draft).toHaveValue(text); - expect(saves.wasHeld()).toBe(true); expect(saves.sentWhileHeld()).toBe(0); await saves.release(); await evidence.step("Sparring: held one save in flight while the speech draft kept changing");