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..d3e618600 --- /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`). 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. + +## 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..623485010 --- /dev/null +++ b/.claude/skills/verify-debatelab/recipes/workspace-autosave.spec.ts @@ -0,0 +1,130 @@ +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"); + // 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.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."; + // 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.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(); + }); +});