From f3c9fef2f799a1442b711997dea17bcac083264f Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 26 Oct 2026 15:28:24 -0500 Subject: [PATCH 01/12] Add fast-check property tests for Workshop approvals, attachments and code changes Three end-to-end properties in packages/integration-tests replay generated sequences of real client operations against the Workshop over RPC, checked against a small model: - Approvals: staged writes decided in any order, with and without an auto-approval rule; the ledger, the fixture gatekeeper's state and the ledger-to-gatekeeper id mapping agree after every decision, nothing auto-applies past a pending manual write, and a second decision applies nothing. - Attachments: chats created, replied to and deleted with shared and same-named files; every attachment id returns its exact bytes in exactly the chat that holds it, including in a fresh chat and after deletion. - Code changes: two editors submit stale and retried changes to one draft; every insertion lands once, a retry is not re-applied, and accepting commits the text the change stream describes. FAST_CHECK_SEED pins the generated cases (src/property-runs.ts). CI sets it, the cached test task declares it in cache.env, and the env-passthrough guard accounts for it. --- .github/workflows/ci.yml | 3 + packages/integration-tests/README.md | 18 + .../__tests__/workshop-properties.test.ts | 436 ++++++++++++++++++ packages/integration-tests/package.json | 1 + .../integration-tests/src/property-runs.ts | 27 ++ packages/integration-tests/vite.config.ts | 13 +- pnpm-lock.yaml | 16 + scripts/env-passthrough.test.ts | 2 + 8 files changed, 512 insertions(+), 4 deletions(-) create mode 100644 packages/integration-tests/__tests__/workshop-properties.test.ts create mode 100644 packages/integration-tests/src/property-runs.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a239af9a0..810eeae4c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,10 +131,13 @@ jobs: # having run nothing. `--fail-if-no-match` makes that an error. # Sets `VITEST_MAX_WORKERS` only when the leg asks: an empty value would count as explicit # and leave every suite at vitest's own default. + # `FAST_CHECK_SEED` pins the property suites' generated cases, so a PR's result depends only on + # its code; a local run without it explores a fresh seed each time. - name: Test env: FILTERS: ${{ matrix.filters }} VITEST_WORKERS: ${{ matrix.vitest-workers }} + FAST_CHECK_SEED: "1" run: | [ -z "$VITEST_WORKERS" ] || export VITEST_MAX_WORKERS="$VITEST_WORKERS" node scripts/vp/run.ts --cache --fail-if-no-match $FILTERS test diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index f3cd08f1ff..8129a22aca 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -40,6 +40,24 @@ vendor this one as a submodule: - **A test that redeploys the Workshop starts a harness of its own.** A redeploy restarts the Workers under every session they have, which would break the tests sharing the file's harness. +## Property-based suites + +`__tests__/workshop-properties.test.ts` uses [fast-check](https://fast-check.dev/) to generate +sequences of the operations a real client performs and replays each against the Workshop, checking +the result against a small model: decisions on staged writes (with and without an auto-approval +rule), chats with shared and same-named attachments created and deleted, and two editors +submitting stale and retried code changes before an accept. Each run takes a fresh account, so +runs and shrink attempts are independent. + +Every run is real RPC, so a property costs hundreds of milliseconds per run and `numRuns` is kept +small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside the test timeout +(`src/property-runs.ts`). + +`FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so +a pull request's result depends only on its code. Without it, each run draws a fresh seed. A +failure prints `{ seed, path }`: rerun with `FAST_CHECK_SEED=` to reproduce it, or pass both +to that property's `fc.assert()` options to replay the shrunk counterexample directly. + ## The fixture gatekeeper `fixtures/gatekeeper-test/` is a real Worker speaking the real gatekeeper protocol, whose verification diff --git a/packages/integration-tests/__tests__/workshop-properties.test.ts b/packages/integration-tests/__tests__/workshop-properties.test.ts new file mode 100644 index 0000000000..c59b3e7f02 --- /dev/null +++ b/packages/integration-tests/__tests__/workshop-properties.test.ts @@ -0,0 +1,436 @@ +// Property-based end-to-end tests: fast-check generates sequences of the operations a real client +// performs, and each run replays one against the real Workshop over RPC, checked against a small +// model. Seeds and the shrink time limit come from `src/property-runs.ts`; a failure prints the +// seed and the shrunk counterexample's path, which replay it when passed to that fc.assert(). +// +// Every run allocates a fresh account (and so a fresh workspace and fixture-gatekeeper label), +// so runs, and shrink attempts, never see each other's state. + +import { setTimeout as sleep } from "node:timers/promises"; +import type { RpcStub } from "capnweb"; +import * as fc from "fast-check"; +import { afterAll, beforeAll, expect, it } from "vitest"; +import type { + AiChatAuthorInfo, AiChatMessage, AiChatMetadata, AiChatStreamEvent, AiChatSubscriber, + ActionLogEntry, Overseer, WorkpieceId, +} from "@gadgets/workshop-shared/api"; +import { applyCodeChange, diffFiles, type CodeChange, type CodeContent } from "@gadgets/workshop-shared/code-change"; +import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.js"; +import { loadAllChatHistory } from "../src/agent-session.js"; +import { + startTestGatekeeperHarness, TEST_VENDOR_ID, testActionState, type Harness, +} from "../src/harness.js"; +import { NetworkInterceptor } from "../src/network-interceptor.js"; +import { PROPERTY_RUN_OPTIONS } from "../src/property-runs.js"; +import { + accountLabel, connect, listConnectedAccounts, nextUsernames, RpcTarget, signUp, stubFor, waitFor, + WorkpieceRecorder, +} from "../src/rpc-client.js"; + +let harness: Harness; +const network = new NetworkInterceptor(); + +beforeAll(async () => { + network.install(); + harness = await startTestGatekeeperHarness(); +}); + +afterAll(async () => { + try { + await harness?.server.close(); + expect(network.getUnmockedCalls()).toEqual([]); + } finally { + network.uninstall(); + } +}); + +/** A fresh account's new workspace, released with the returned stack. */ +async function freshWorkspace(stack: DisposableStack, prefix: string) { + const [username] = nextUsernames(prefix); + const api = stack.use(await signUp(stack.use(connect(harness.url)), username)); + const ws = stack.use(await api.newWorkspace()); + return { api, ws }; +} + +// --------------------------------------------------------------------------------------------- +// Approvals: writes a gadget stages, decided by the user in any order, with and without an +// auto-approval rule. The fixture gatekeeper numbers its actions independently of the Workshop's +// ledger, so every decision also checks the ledger-to-gatekeeper id mapping: approving the wrong +// entry applies the wrong value, or throws in the fixture. + +type Decision = "approve" | "reject" | "leave"; + +class ApprovalModel { + readonly states: ("pending" | "approved" | "rejected")[]; + applied: number[] = []; + + constructor(readonly values: number[], readonly eligible: boolean[]) { + this.states = values.map(() => "pending"); + this.drain(); + } + + decide(index: number, decision: "approve" | "reject") { + this.states[index] = decision === "approve" ? "approved" : "rejected"; + if (decision === "approve") this.applied.push(this.values[index]!); + this.drain(); + } + + // The drain: eligible pending writes apply in submission order, stopping at the first pending + // write that needs a human (auto-approval.ts). + drain() { + for (const [i, state] of this.states.entries()) { + if (state !== "pending") continue; + if (!this.eligible[i]) return; + this.states[i] = "approved"; + this.applied.push(this.values[i]!); + } + } +} + +const approvalScenario = fc.uniqueArray(fc.integer({ min: -1_000_000, max: 1_000_000 }), + { minLength: 1, maxLength: 6 }).chain(values => fc.record({ + values: fc.constant(values), + ruleEnabled: fc.boolean(), + autoApprovable: fc.array(fc.boolean(), { minLength: values.length, maxLength: values.length }), + decisions: fc.array(fc.constantFrom("approve", "reject", "leave"), + { minLength: values.length, maxLength: values.length }), + order: fc.shuffledSubarray(values.map((_, i) => i), + { minLength: values.length, maxLength: values.length }), + concurrent: fc.boolean(), +})); + +async function writesOf(ws: RpcStub, gatekeeperId: number) { + const writes: (ActionLogEntry & { type: "action" })[] = []; + let beforeId: number | undefined; + do { + const page = await ws.listActions({ filter: "action", beforeId }); + for (const entry of page.entries) { + if (entry.type === "action" && entry.gatekeeperId === gatekeeperId) writes.push(entry); + } + beforeId = page.nextBeforeId; + } while (beforeId !== undefined); + return writes; +} + +const valueOf = (entry: ActionLogEntry) => Number(/^Set the test value to (-?\d+)$/ + .exec(entry.description.title)?.[1]); + +it.concurrent("decided writes apply exactly once, to the write the user decided", async () => { + await fc.assert(fc.asyncProperty(approvalScenario, async scenario => { + using stack = new DisposableStack(); + const { api, ws } = await freshWorkspace(stack, "approvals"); + await api.provisionAmbientAccount(TEST_VENDOR_ID); + const account = await waitFor("the test account", async () => + (await listConnectedAccounts(api)).find(a => a.vendorId === TEST_VENDOR_ID) ?? null); + const label = accountLabel(account); + const gatekeeper = await ws.newGatekeeper(account.id, "https://gadgets-test.example/things/approvals"); + if (!gatekeeper) throw new Error("Failed to create the test connection"); + const gatekeeperId = await stack.use(gatekeeper).getId(); + const session = stack.use(await gatekeeper.openSession() as RpcStub); + if (scenario.ruleEnabled) { + await ws.setAutoApprovedActionKind(gatekeeperId, { tag: "set-value", label: "Set value" }); + } + + const { values } = scenario; + const eligible = values.map((_, i) => scenario.ruleEnabled && scenario.autoApprovable[i]!); + const write = (i: number) => + session.writeValue(values[i]!, { autoApprovable: scenario.autoApprovable[i] }); + // A gadget may fire its writes concurrently, but the drain then races submission, so the + // model's "nothing applies past a pending gate" order holds only for sequential submission. + if (scenario.concurrent && !scenario.ruleEnabled) { + await Promise.all(values.map((_, i) => write(i))); + } else { + for (const i of values.keys()) await write(i); + } + + const model = new ApprovalModel(values, eligible); + const idOf = new Map((await writesOf(ws, gatekeeperId)).map(entry => [valueOf(entry), entry.id])); + expect([...idOf.keys()].toSorted()).toEqual(values.toSorted()); + + const matchesModel = async () => { + const writes = await writesOf(ws, gatekeeperId); + const states = new Map(writes.map(entry => [valueOf(entry), entry.state])); + return values.every((value, i) => states.get(value) === model.states[i]) ? writes : null; + }; + const expectFixtureMatches = async () => { + const pending = values.filter((_, i) => model.states[i] === "pending"); + const state = await testActionState(harness, label); + expect(state.pending.map(action => action.value).toSorted()).toEqual(pending.toSorted()); + expect(state.applyCount).toBe(model.applied.length); + expect(state.value).toBe(model.applied.at(-1)); + }; + + await waitFor("the initial drain", matchesModel, 10_000); + for (const i of scenario.order) { + const decision = scenario.decisions[i]!; + if (eligible[i] || decision === "leave" || model.states[i] !== "pending") continue; + const id = idOf.get(values[i]!)!; + await (decision === "approve" ? ws.approveAction(id) : ws.rejectAction(id)); + model.decide(i, decision); + await waitFor(`the ledger after ${decision} of ${values[i]}`, matchesModel, 10_000); + await expectFixtureMatches(); + } + + // Give a drain that wrongly skips past a pending gate time to show itself. It runs out of + // process (ctx.waitUntil), so only a real delay can show its absence. + await sleep(300); + const writes = await matchesModel(); + expect(writes, "ledger states after settling").not.toBeNull(); + for (const entry of writes!) { + const i = values.indexOf(valueOf(entry)); + if (model.states[i] === "approved") expect(!!entry.autoApproved).toBe(eligible[i]); + } + await expectFixtureMatches(); + + // A second tab deciding an already decided write must not apply it again. + const decided = values.findIndex((_, i) => model.states[i] !== "pending"); + if (decided !== -1) { + const id = idOf.get(values[decided]!)!; + await expect(ws.approveAction(id)).rejects.toThrow(`Action is not pending: ${id}`); + await expect(ws.rejectAction(id)).rejects.toThrow(`Action is not pending: ${id}`); + await expectFixtureMatches(); + } + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15 }); +}); + +// --------------------------------------------------------------------------------------------- +// Chats and their attachments: create, reply and delete chats, attaching uploaded files (some +// shared across messages and chats, some sharing a filename). Every attachment id resolves, to its +// file's exact bytes, in exactly the chat that holds it, including after its chat is deleted and +// another chat takes its place. + +type File = { bytes: Uint8Array; mimeType: string; name: string }; + +const PNG_SIGNATURE = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; +const file: fc.Arbitrary = fc.oneof( + fc.record({ + bytes: fc.string({ unit: "grapheme", maxLength: 200 }).map(text => new TextEncoder().encode(text)), + mimeType: fc.constant("text/plain"), + name: fc.constantFrom("notes.txt", "todo.txt"), + }), + fc.record({ + bytes: fc.uint8Array({ maxLength: 2048 }), + mimeType: fc.constant("application/octet-stream"), + name: fc.constantFrom("export.bin", "data.bin"), + }), + fc.record({ + bytes: fc.uint8Array({ maxLength: 1024 }).map(tail => new Uint8Array([...PNG_SIGNATURE, ...tail])), + mimeType: fc.constant("image/png"), + name: fc.constantFrom("screenshot.png", "photo.png"), + }), +); + +const attach = fc.uniqueArray(fc.nat(), { maxLength: 3 }); +const message = fc.constantFrom("Here you go", "See attached", "Thoughts?", "Ünïcödé 👋"); +const chatOp = fc.oneof( + fc.record({ op: fc.constant("new" as const), text: message, attach }), + fc.record({ op: fc.constant("send" as const), chat: fc.nat(), text: message, attach }), + fc.record({ op: fc.constant("delete" as const), chat: fc.nat() }), +); + +// A session opens with a chat carrying a file; the generated ops may then reply, delete, or start +// more. Without it, a run whose first chat carries nothing checks no attachment at all. +const opening = fc.record({ + op: fc.constant("new" as const), text: message, attach: fc.uniqueArray(fc.nat(), { minLength: 1, maxLength: 3 }), +}); + +type ModelMessage = { text: string; attachments: { id: string; file: number }[] }; + +it.concurrent("attachment ids resolve to their bytes in exactly the chat that holds them", async () => { + await fc.assert(fc.asyncProperty( + fc.array(file, { minLength: 1, maxLength: 4 }), + fc.tuple(opening, fc.array(chatOp, { maxLength: 7 })).map(([first, rest]) => [first, ...rest]), + async (files, ops) => { + using stack = new DisposableStack(); + const { ws } = await freshWorkspace(stack, "attachments"); + const oids: string[] = []; + for (const { bytes } of files) { + const oid = await ws.uploadBlob(bytes); + expect(await ws.uploadBlob(bytes.slice())).toBe(oid); + oids.push(oid); + } + + const live = new Map(); + const deleted = new Map(); + // Send `text` with the files `picks` selects, as a new chat or into `chatId`, and record the + // attachment ids the chat's history assigned them. + const send = async (chatId: number | undefined, text: string, picks: number[]) => { + const picked = [...new Set(picks.map(p => p % files.length))]; + const sent = picked.map(f => ({ oid: oids[f]!, mimeType: files[f]!.mimeType, name: files[f]!.name })); + if (chatId === undefined) { + chatId = await ws.newChat(text, null, undefined, sent); + expect(live.has(chatId)).toBe(false); + live.set(chatId, []); + } else { + await ws.sendChatMessage(chatId, text, null, undefined, sent); + } + const last = (await ws.getChatHistory(chatId)).messages.findLast(m => m.type === "message"); + if (last?.type !== "message") throw new Error(`Chat ${chatId} has no message`); + expect(last.message).toBe(text); + const attachments = last.attachments ?? []; + expect(attachments.map(({ oid, mimeType, name, size }) => ({ oid, mimeType, name, size }))) + .toEqual(sent.map((handle, i) => ({ ...handle, size: files[picked[i]!]!.bytes.byteLength }))); + live.get(chatId)!.push({ text, attachments: attachments.map((a, i) => ({ id: a.id, file: picked[i]! })) }); + }; + + for (const op of ops) { + const chats = [...live.keys()]; + if (op.op === "new") { + await send(undefined, op.text, op.attach); + } else if (chats.length > 0) { + const chatId = chats[op.chat % chats.length]!; + if (op.op === "send") { + await send(chatId, op.text, op.attach); + } else { + await ws.deleteChat(chatId); + deleted.set(chatId, live.get(chatId)!.flatMap(m => m.attachments.map(a => a.id))); + live.delete(chatId); + } + } + } + + expect((await ws.listChats()).map(chat => chat.id).toSorted()).toEqual([...live.keys()].toSorted()); + const owner = new Map(); + for (const [chatId, messages] of live) { + const history = await loadAllChatHistory(before => ws.getChatHistory(chatId, before)); + expect(history.flatMap(m => m.type === "message" ? [m.message] : [])) + .toEqual(messages.map(m => m.text)); + for (const { id, file: f } of messages.flatMap(m => m.attachments)) { + owner.set(id, chatId); + // capnweb hands Node a Buffer; compare the bytes, not the class. + expect(new Uint8Array(await ws.getChatAttachmentContent(chatId, id))).toEqual(files[f]!.bytes); + } + } + // A chat the user starts now holds none of them: every id, live or deleted, must miss there. + // Each live id is also tried in one other live chat, and each deleted chat's ids in its old id + // (which a later chat may now hold) and every live chat. + const chatIds = [...live.keys()]; + const fresh = await ws.newChat("Starting over", null); + for (const [id, chatId] of owner) { + await expect(ws.getChatAttachmentContent(fresh, id)).rejects.toThrow("Chat attachment not found."); + const other = chatIds.find(c => c !== chatId); + if (other !== undefined) { + await expect(ws.getChatAttachmentContent(other, id)).rejects.toThrow("Chat attachment not found."); + } + } + for (const [chatId, ids] of deleted) { + if (!live.has(chatId)) expect((await ws.getChatHistory(chatId)).messages).toEqual([]); + for (const id of ids) { + for (const c of new Set([chatId, fresh, ...chatIds])) { + await expect(ws.getChatAttachmentContent(c, id)).rejects.toThrow("Chat attachment not found."); + } + } + } + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15 }); +}); + +// --------------------------------------------------------------------------------------------- +// Code-change submission: two editors type into one chat's draft, each from whatever revision of +// the change stream it had seen (up to a few behind), and retry a submission as a dropped +// connection would. However stale the base, every insertion lands exactly once, the original lines +// keep their order, a retry is acknowledged without being applied again, and accepting the draft +// commits exactly the text the change stream describes. + +class ChangeRecorder extends RpcTarget implements AiChatSubscriber { + readonly applied: { chatId: number; revision: number; change: CodeChange }[] = []; + streamGeneration(_generation: number): void {} + metadata(_chat: AiChatMetadata): void {} + deleted(_chatId: number): void {} + message(_message: AiChatMessage): void {} + changeApplied(chatId: number, _generation: number, revision: number, _author: AiChatAuthorInfo, + change: CodeChange): void { + this.applied.push({ chatId, revision, change }); + } + stream(_chatId: number, _event: AiChatStreamEvent): void {} +} + +const PATH = "app.txt"; +const SEED_LINES = ["alpha", "beta", "gamma", "delta"]; +const linesOf = (text: string) => text.split("\n").slice(0, -1); +const textOf = (lines: string[]) => lines.map(line => `${line}\n`).join(""); +const files = (gadgetId: number, text?: string): CodeContent => + new Map([[gadgetId, new Map(text === undefined ? [] : [[PATH, text]])]]); + +const headOf = (workpieces: WorkpieceRecorder, gadgetId: WorkpieceId, after?: string) => + waitFor(`a new head for gadget ${gadgetId}`, async () => { + const summary = workpieces.summaries.get(gadgetId); + return summary?.type === "gadget" && summary.commitId !== undefined && + summary.commitId !== after ? summary.commitId : null; + }); + +const editorOp = fc.record({ + client: fc.constantFrom("left", "right"), + lag: fc.nat({ max: 3 }), + at: fc.nat(), + retry: fc.boolean(), +}); + +it.concurrent("stale and retried edits converge, and accepting commits the converged text", async () => { + await fc.assert(fc.asyncProperty( + fc.integer({ min: 1, max: SEED_LINES.length }), + fc.array(editorOp, { minLength: 1, maxLength: 8 }), + async (seedCount, ops) => { + using stack = new DisposableStack(); + const { ws } = await freshWorkspace(stack, "editors"); + const workpieces = new WorkpieceRecorder(); + const recorder = new ChangeRecorder(); + stack.use(await ws.subscribeToWorkpieces(stack.use(stubFor(workpieces)))); + stack.use(await ws.subscribeToChat(stack.use(stubFor(recorder)))); + await workpieces.loaded; + + const seedLines = SEED_LINES.slice(0, seedCount); + const seed = textOf(seedLines); + const gadget = stack.use(ws.createGadget("App", undefined, "APP")); + const gadgetId = await gadget.getId(); + const empty = await headOf(workpieces, gadgetId); + const seedChat = await ws.newChat("Seed", null); + await ws.submitCodeChange(seedChat, { + generation: 0, revision: 0, clientId: "seed", seq: 1, + pins: [{ gadgetId, baseCommit: empty }], change: diffFiles(files(gadgetId), files(gadgetId, seed)), + }); + expect(await ws.mergeChanges(seedChat)).toEqual({ outcome: "merged" }); + const head = await headOf(workpieces, gadgetId, empty); + + const chatId = await ws.newChat("Edit together", null); + const pins = [{ gadgetId, baseCommit: head }]; + const seqs = { left: 0, right: 0 }; + const stream = (count: number) => waitFor(`${count} changes to chat ${chatId}`, async () => { + const changes = recorder.applied.filter(c => c.chatId === chatId); + return changes.length >= count ? changes.toSorted((a, b) => a.revision - b.revision) : null; + }); + const textAt = (changes: { change: CodeChange }[]) => { + const text = changes.reduce((content, { change }) => applyCodeChange(content, change), + files(gadgetId, seed)).get(gadgetId)?.get(PATH); + if (typeof text !== "string") throw new Error(`${PATH} is not text`); + return text; + }; + + const markers: string[] = []; + let revision = 0; + for (const [i, op] of ops.entries()) { + const base = Math.max(0, revision - op.lag); + const before = textAt((await stream(revision)).slice(0, base)); + const lines = linesOf(before); + const marker = `${op.client} ${i}`; + lines.splice(op.at % (lines.length + 1), 0, marker); + markers.push(marker); + const submission = { + generation: 0, revision: base, clientId: op.client, seq: ++seqs[op.client], pins, + change: diffFiles(files(gadgetId, before), files(gadgetId, textOf(lines))), + }; + const ack = await ws.submitCodeChange(chatId, submission); + expect(ack).toEqual({ generation: 0, revision: ++revision }); + if (op.retry) expect(await ws.submitCodeChange(chatId, submission)).toEqual(ack); + } + + const converged = textAt(await stream(revision)); + const lines = linesOf(converged); + expect(lines.filter(line => !markers.includes(line))).toEqual(seedLines); + expect(lines.filter(line => markers.includes(line)).toSorted()).toEqual(markers.toSorted()); + + expect(await ws.mergeChanges(chatId)).toEqual({ outcome: "merged" }); + const merged = await headOf(workpieces, gadgetId, head); + expect(await ws.readFilesAtCommit(merged, [PATH])).toEqual([[PATH, { kind: "text", text: converged }]]); + expect(recorder.applied.filter(c => c.chatId === chatId)).toHaveLength(revision); + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15 }); +}); diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index f560163360..4c4fc22663 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -28,6 +28,7 @@ "@gadgets/scripts": "workspace:*", "@types/node": "^26.1.0", "capnweb-validate": "catalog:", + "fast-check": "^4.10.2", "typescript": "catalog:", "vitest": "catalog:", "wrangler": "catalog:" diff --git a/packages/integration-tests/src/property-runs.ts b/packages/integration-tests/src/property-runs.ts new file mode 100644 index 0000000000..50197791dc --- /dev/null +++ b/packages/integration-tests/src/property-runs.ts @@ -0,0 +1,27 @@ +// fast-check options shared by the property-based suites. + +/** + * `FAST_CHECK_SEED` pins every property's seed, so a run generates the same cases each time; CI + * sets it. Unset, fast-check draws a fresh seed per run and prints it with any failure, along with + * the `path` of the shrunk counterexample, so a local failure can be replayed. + */ +function seedFromEnv(): { seed?: number } { + const raw = process.env.FAST_CHECK_SEED; + if (!raw) return {}; + const seed = Number(raw); + if (!Number.isSafeInteger(seed)) throw new Error(`FAST_CHECK_SEED must be an integer, got "${raw}"`); + return { seed }; +} + +/** + * Options for every `fc.assert()` over the Workshop. + * + * Each run is real RPC, and so is every shrink attempt. The time limit keeps a failing property's + * shrink inside the test timeout; it reports the smallest counterexample found by then. An + * interrupt before any failure still fails, rather than silently passing on fewer runs. + */ +export const PROPERTY_RUN_OPTIONS = { + interruptAfterTimeLimit: 90_000, + markInterruptAsFailure: true, + ...seedFromEnv(), +} as const; diff --git a/packages/integration-tests/vite.config.ts b/packages/integration-tests/vite.config.ts index 486d71d623..cfa1b04a07 100644 --- a/packages/integration-tests/vite.config.ts +++ b/packages/integration-tests/vite.config.ts @@ -5,6 +5,11 @@ import { WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, } from '@gadgets/scripts/vitest-task' +const testTask = vitestTaskWithExclusions('vitest run', [ + ...VITEST_TOOL_SCRATCH_EXCLUSIONS, + ...WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, +]) + export default { run: { tasks: { @@ -46,10 +51,10 @@ export default { * does write: `tmp/` (per-boot bundles) and `state/` (local DO/KV storage). */ test: { - ...vitestTaskWithExclusions('vitest run', [ - ...VITEST_TOOL_SCRATCH_EXCLUSIONS, - ...WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, - ]), + ...testTask, + // The property suites' seed (src/property-runs.ts) decides which cases run, so it is + // passed and fingerprinted: a different seed is a cache miss, not a replayed result. + cache: { ...testTask.cache, env: [...testTask.cache.env, 'FAST_CHECK_SEED'] }, // Real gatekeepers boot from prebuilt validated trees, tracked as inputs like the fixture, // plus the release the Google upgrade suite starts from, and `cli-admins-only.test.ts` runs // `cf-os api`, whose command table is generated. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6f4f6ad094..e5a6a8798a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1208,6 +1208,9 @@ importers: capnweb-validate: specifier: 'catalog:' version: 0.3.0(capnweb@0.12.0)(esbuild@0.28.2)(rollup@4.62.3)(vite@7.3.6(@types/node@26.1.0)(jiti@2.7.0)(lightningcss@1.33.0)(terser@5.51.2)(yaml@2.9.1)) + fast-check: + specifier: ^4.10.2 + version: 4.10.2 typescript: specifier: 'catalog:' version: 7.0.2 @@ -4141,6 +4144,10 @@ packages: engines: {node: '>= 10.17.0'} hasBin: true + fast-check@4.10.2: + resolution: {integrity: sha512-iK2f+YrcmoeGqk6fA0ea2bptcu/itMIm4NfEozq6N25+aG6h7s5HZbB/k1aV7b5w5sFLMCbbtRUsTVR+BgC3xw==} + engines: {node: '>=12.17.0'} + fast-diff@1.3.0: resolution: {integrity: sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw==} @@ -5063,6 +5070,9 @@ packages: resolution: {integrity: sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==} engines: {node: '>=6'} + pure-rand@8.4.2: + resolution: {integrity: sha512-vvuOGgcuPJAirlHvuQw1TrOiw7ptaIXXmIbNuiNOY6lNGJJH49PQ1Kj4nd783nPdQhQdicgOjVI2yI/9BD6/Ng==} + react-day-picker@9.14.0: resolution: {integrity: sha512-tBaoDWjPwe0M5pGrum4H0SR6Lyk+BO9oHnp9JbKpGKW2mlraNPgP9BMfsg5pWpwrssARmeqk7YBl2oXutZTaHA==} engines: {node: '>=18'} @@ -8159,6 +8169,10 @@ snapshots: transitivePeerDependencies: - supports-color + fast-check@4.10.2: + dependencies: + pure-rand: 8.4.2 + fast-diff@1.3.0: {} fast-fifo@1.3.2: {} @@ -9274,6 +9288,8 @@ snapshots: punycode@2.3.1: {} + pure-rand@8.4.2: {} + react-day-picker@9.14.0(react@19.3.1): dependencies: '@date-fns/tz': 1.5.0 diff --git a/scripts/env-passthrough.test.ts b/scripts/env-passthrough.test.ts index bde2fce48a..d295e3f1fd 100644 --- a/scripts/env-passthrough.test.ts +++ b/scripts/env-passthrough.test.ts @@ -69,7 +69,9 @@ const EXPECTED: Record = { // `src/worker-inputs.ts` resolves BUNDLED_BLUEPRINTS_DIR only to name a directory for the watcher // and `forceRerunTriggers`. The `test` task is cached and strips it; `vitest run` never reads // those exports, and `pnpm test:watch` is a script, so it keeps the ambient value. + // `src/property-runs.ts` reads FAST_CHECK_SEED, which picks the property suites' cases. "packages/integration-tests": { + forwarded: ["FAST_CHECK_SEED"], injected: ["WORKSHOP_INTEGRATION_PREBUILT"], watch: ["BUNDLED_BLUEPRINTS_DIR"], }, From 4f7fe32a322d71446174d42ebfb99ecb021c978f Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 26 Oct 2026 16:33:40 -0500 Subject: [PATCH 02/12] Close gaps the adversarial review found in the Workshop property tests - Code changes: resend is its own op, so a resend can arrive after the other editor's change and must get its original acknowledgement back; the broadcast stream must number its changes 1..n; editors delete lines as well as insert them, as exact spans the way CodeEditor submits its transactions (a minimal diff can straddle lines); every session opens with an edit, and a draft that nets to no change keeps the head. - Approvals: the generator is a uniqueArray of writes (value, flag, decision, rank), which shrinks to minimal cases, and two pinned examples cover approving or rejecting a manual write ahead of auto-approvable ones, which random cases reached only on some seeds. - The 300ms settle sleep runs only while an eligible write is held behind a gate, and waits inside the properties use a 5s timeout, so failing shrinks finish in about 30s instead of the 90s cap. - Attachments: drop a guard for chat-id reuse, which never happens. - Docs no longer claim an unset seed explores on a cached run. --- .github/workflows/ci.yml | 2 +- packages/integration-tests/README.md | 13 +- .../__tests__/workshop-properties.test.ts | 211 +++++++++++------- .../integration-tests/src/property-runs.ts | 5 +- 4 files changed, 148 insertions(+), 83 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 810eeae4c9..198110abc1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,7 @@ jobs: # Sets `VITEST_MAX_WORKERS` only when the leg asks: an empty value would count as explicit # and leave every suite at vitest's own default. # `FAST_CHECK_SEED` pins the property suites' generated cases, so a PR's result depends only on - # its code; a local run without it explores a fresh seed each time. + # its code. - name: Test env: FILTERS: ${{ matrix.filters }} diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index 8129a22aca..f6eec9990b 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -46,17 +46,20 @@ vendor this one as a submodule: sequences of the operations a real client performs and replays each against the Workshop, checking the result against a small model: decisions on staged writes (with and without an auto-approval rule), chats with shared and same-named attachments created and deleted, and two editors -submitting stale and retried code changes before an accept. Each run takes a fresh account, so -runs and shrink attempts are independent. +inserting, deleting and resending stale code changes before an accept. Each run takes a fresh +account, so runs and shrink attempts are independent. Scenarios a fixed seed might miss are +pinned as fast-check `examples`, which run on every seed. Every run is real RPC, so a property costs hundreds of milliseconds per run and `numRuns` is kept small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside the test timeout (`src/property-runs.ts`). `FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so -a pull request's result depends only on its code. Without it, each run draws a fresh seed. A -failure prints `{ seed, path }`: rerun with `FAST_CHECK_SEED=` to reproduce it, or pass both -to that property's `fc.assert()` options to replay the shrunk counterexample directly. +a pull request's result depends only on its code. Without it, each run draws a fresh seed, but a +cached `pnpm test` replays its last result: run vitest directly (`test:run`) or +`vp run --no-cache` to explore a new one. A failure prints `{ seed, path }`: rerun with +`FAST_CHECK_SEED=` to reproduce it, or pass both to that property's `fc.assert()` options to +replay the shrunk counterexample directly. ## The fixture gatekeeper diff --git a/packages/integration-tests/__tests__/workshop-properties.test.ts b/packages/integration-tests/__tests__/workshop-properties.test.ts index c59b3e7f02..a0e2cc360b 100644 --- a/packages/integration-tests/__tests__/workshop-properties.test.ts +++ b/packages/integration-tests/__tests__/workshop-properties.test.ts @@ -2,6 +2,7 @@ // performs, and each run replays one against the real Workshop over RPC, checked against a small // model. Seeds and the shrink time limit come from `src/property-runs.ts`; a failure prints the // seed and the shrunk counterexample's path, which replay it when passed to that fc.assert(). +// Scenarios a fixed seed might not reach are pinned as `examples`, which run on every seed. // // Every run allocates a fresh account (and so a fresh workspace and fixture-gatekeeper label), // so runs, and shrink attempts, never see each other's state. @@ -12,9 +13,11 @@ import * as fc from "fast-check"; import { afterAll, beforeAll, expect, it } from "vitest"; import type { AiChatAuthorInfo, AiChatMessage, AiChatMetadata, AiChatStreamEvent, AiChatSubscriber, - ActionLogEntry, Overseer, WorkpieceId, + ActionLogEntry, CodeChangeSubmission, Overseer, WorkpieceId, } from "@gadgets/workshop-shared/api"; -import { applyCodeChange, diffFiles, type CodeChange, type CodeContent } from "@gadgets/workshop-shared/code-change"; +import { + applyCodeChange, diffFiles, replaceSpanChange, type CodeChange, type CodeContent, +} from "@gadgets/workshop-shared/code-change"; import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.js"; import { loadAllChatHistory } from "../src/agent-session.js"; import { @@ -44,6 +47,10 @@ afterAll(async () => { } }); +// How long to wait for an effect that takes milliseconds when it happens at all. Far above that, +// yet short enough that a failing property's shrink attempts each give up quickly. +const SETTLE_MS = 5_000; + /** A fresh account's new workspace, released with the returned stack. */ async function freshWorkspace(stack: DisposableStack, prefix: string) { const [username] = nextUsernames(prefix); @@ -59,6 +66,9 @@ async function freshWorkspace(stack: DisposableStack, prefix: string) { // entry applies the wrong value, or throws in the fixture. type Decision = "approve" | "reject" | "leave"; +/** One staged write, and what the user does with it when they reach it (in `rank` order). */ +type Write = { value: number; autoApprovable: boolean; decision: Decision; rank: number }; +type ApprovalScenario = { writes: Write[]; ruleEnabled: boolean; concurrent: boolean }; class ApprovalModel { readonly states: ("pending" | "approved" | "rejected")[]; @@ -87,17 +97,30 @@ class ApprovalModel { } } -const approvalScenario = fc.uniqueArray(fc.integer({ min: -1_000_000, max: 1_000_000 }), - { minLength: 1, maxLength: 6 }).chain(values => fc.record({ - values: fc.constant(values), +const approvalScenario: fc.Arbitrary = fc.record({ + writes: fc.uniqueArray(fc.record({ + value: fc.integer({ min: -1_000_000, max: 1_000_000 }), + autoApprovable: fc.boolean(), + decision: fc.constantFrom("approve", "reject", "leave"), + rank: fc.nat(), + }), { minLength: 1, maxLength: 6, selector: write => write.value }), ruleEnabled: fc.boolean(), - autoApprovable: fc.array(fc.boolean(), { minLength: values.length, maxLength: values.length }), - decisions: fc.array(fc.constantFrom("approve", "reject", "leave"), - { minLength: values.length, maxLength: values.length }), - order: fc.shuffledSubarray(values.map((_, i) => i), - { minLength: values.length, maxLength: values.length }), concurrent: fc.boolean(), -})); +}); + +/** + * Deciding a manual write must restart the drain for the auto-approvable writes queued behind it + * (the turn-resumption fix, 1200259, lives on this path). Random cases reach it only sometimes. + */ +const cascade = (decision: "approve" | "reject"): [ApprovalScenario] => [{ + writes: [ + { value: 1, autoApprovable: false, decision, rank: 0 }, + { value: 2, autoApprovable: true, decision: "leave", rank: 0 }, + { value: 3, autoApprovable: true, decision: "leave", rank: 0 }, + ], + ruleEnabled: true, + concurrent: false, +}]; async function writesOf(ws: RpcStub, gatekeeperId: number) { const writes: (ActionLogEntry & { type: "action" })[] = []; @@ -116,7 +139,7 @@ const valueOf = (entry: ActionLogEntry) => Number(/^Set the test value to (-?\d+ .exec(entry.description.title)?.[1]); it.concurrent("decided writes apply exactly once, to the write the user decided", async () => { - await fc.assert(fc.asyncProperty(approvalScenario, async scenario => { + await fc.assert(fc.asyncProperty(approvalScenario, async ({ writes, ruleEnabled, concurrent }) => { using stack = new DisposableStack(); const { api, ws } = await freshWorkspace(stack, "approvals"); await api.provisionAmbientAccount(TEST_VENDOR_ID); @@ -127,20 +150,19 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" if (!gatekeeper) throw new Error("Failed to create the test connection"); const gatekeeperId = await stack.use(gatekeeper).getId(); const session = stack.use(await gatekeeper.openSession() as RpcStub); - if (scenario.ruleEnabled) { + if (ruleEnabled) { await ws.setAutoApprovedActionKind(gatekeeperId, { tag: "set-value", label: "Set value" }); } - const { values } = scenario; - const eligible = values.map((_, i) => scenario.ruleEnabled && scenario.autoApprovable[i]!); - const write = (i: number) => - session.writeValue(values[i]!, { autoApprovable: scenario.autoApprovable[i] }); + const values = writes.map(write => write.value); + const eligible = writes.map(write => ruleEnabled && write.autoApprovable); + const submit = ({ value, autoApprovable }: Write) => session.writeValue(value, { autoApprovable }); // A gadget may fire its writes concurrently, but the drain then races submission, so the // model's "nothing applies past a pending gate" order holds only for sequential submission. - if (scenario.concurrent && !scenario.ruleEnabled) { - await Promise.all(values.map((_, i) => write(i))); + if (concurrent && !ruleEnabled) { + await Promise.all(writes.map(submit)); } else { - for (const i of values.keys()) await write(i); + for (const write of writes) await submit(write); } const model = new ApprovalModel(values, eligible); @@ -148,9 +170,9 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" expect([...idOf.keys()].toSorted()).toEqual(values.toSorted()); const matchesModel = async () => { - const writes = await writesOf(ws, gatekeeperId); - const states = new Map(writes.map(entry => [valueOf(entry), entry.state])); - return values.every((value, i) => states.get(value) === model.states[i]) ? writes : null; + const entries = await writesOf(ws, gatekeeperId); + const states = new Map(entries.map(entry => [valueOf(entry), entry.state])); + return values.every((value, i) => states.get(value) === model.states[i]) ? entries : null; }; const expectFixtureMatches = async () => { const pending = values.filter((_, i) => model.states[i] === "pending"); @@ -160,25 +182,27 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" expect(state.value).toBe(model.applied.at(-1)); }; - await waitFor("the initial drain", matchesModel, 10_000); - for (const i of scenario.order) { - const decision = scenario.decisions[i]!; + await waitFor("the initial drain", matchesModel, SETTLE_MS); + for (const i of [...writes.keys()].toSorted((a, b) => writes[a]!.rank - writes[b]!.rank)) { + const { value, decision } = writes[i]!; if (eligible[i] || decision === "leave" || model.states[i] !== "pending") continue; - const id = idOf.get(values[i]!)!; + const id = idOf.get(value)!; await (decision === "approve" ? ws.approveAction(id) : ws.rejectAction(id)); model.decide(i, decision); - await waitFor(`the ledger after ${decision} of ${values[i]}`, matchesModel, 10_000); + await waitFor(`the ledger after ${decision} of ${value}`, matchesModel, SETTLE_MS); await expectFixtureMatches(); } - // Give a drain that wrongly skips past a pending gate time to show itself. It runs out of - // process (ctx.waitUntil), so only a real delay can show its absence. - await sleep(300); - const writes = await matchesModel(); - expect(writes, "ledger states after settling").not.toBeNull(); - for (const entry of writes!) { + // A drain that wrongly skips past a pending gate runs out of process (ctx.waitUntil), so only + // a real delay can show it, and only while an eligible write is still held behind a gate. + if (eligible.some((isEligible, i) => isEligible && model.states[i] === "pending")) await sleep(300); + const entries = await matchesModel(); + expect(entries, "ledger states after settling").not.toBeNull(); + for (const entry of entries!) { const i = values.indexOf(valueOf(entry)); - if (model.states[i] === "approved") expect(!!entry.autoApproved).toBe(eligible[i]); + if (model.states[i] === "approved") { + expect(!!entry.autoApproved, `autoApproved of write ${values[i]}`).toBe(eligible[i]); + } } await expectFixtureMatches(); @@ -190,14 +214,13 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" await expect(ws.rejectAction(id)).rejects.toThrow(`Action is not pending: ${id}`); await expectFixtureMatches(); } - }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15 }); + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15, examples: [cascade("reject"), cascade("approve")] }); }); // --------------------------------------------------------------------------------------------- // Chats and their attachments: create, reply and delete chats, attaching uploaded files (some // shared across messages and chats, some sharing a filename). Every attachment id resolves, to its -// file's exact bytes, in exactly the chat that holds it, including after its chat is deleted and -// another chat takes its place. +// file's exact bytes, in exactly the chat that holds it, and nowhere once its chat is deleted. type File = { bytes: Uint8Array; mimeType: string; name: string }; @@ -302,8 +325,8 @@ it.concurrent("attachment ids resolve to their bytes in exactly the chat that ho } } // A chat the user starts now holds none of them: every id, live or deleted, must miss there. - // Each live id is also tried in one other live chat, and each deleted chat's ids in its old id - // (which a later chat may now hold) and every live chat. + // Each live id is also tried in one other live chat, and each deleted chat's ids in that chat + // and every live one. const chatIds = [...live.keys()]; const fresh = await ws.newChat("Starting over", null); for (const [id, chatId] of owner) { @@ -314,9 +337,9 @@ it.concurrent("attachment ids resolve to their bytes in exactly the chat that ho } } for (const [chatId, ids] of deleted) { - if (!live.has(chatId)) expect((await ws.getChatHistory(chatId)).messages).toEqual([]); + expect((await ws.getChatHistory(chatId)).messages).toEqual([]); for (const id of ids) { - for (const c of new Set([chatId, fresh, ...chatIds])) { + for (const c of [chatId, fresh, ...chatIds]) { await expect(ws.getChatAttachmentContent(c, id)).rejects.toThrow("Chat attachment not found."); } } @@ -325,11 +348,12 @@ it.concurrent("attachment ids resolve to their bytes in exactly the chat that ho }); // --------------------------------------------------------------------------------------------- -// Code-change submission: two editors type into one chat's draft, each from whatever revision of -// the change stream it had seen (up to a few behind), and retry a submission as a dropped -// connection would. However stale the base, every insertion lands exactly once, the original lines -// keep their order, a retry is acknowledged without being applied again, and accepting the draft -// commits exactly the text the change stream describes. +// Code-change submission: two editors insert and delete lines in one chat's draft, each from +// whatever revision of the change stream it had seen (up to a few behind), and resend their last +// submission later, as a dropped connection does. However stale the base, the stream numbers its +// changes 1..n, every surviving line appears exactly once and the original lines keep their order, +// a resend is acknowledged where it first landed without being applied again, and accepting the +// draft commits exactly the text the change stream describes. class ChangeRecorder extends RpcTarget implements AiChatSubscriber { readonly applied: { chatId: number; revision: number; change: CodeChange }[] = []; @@ -356,19 +380,31 @@ const headOf = (workpieces: WorkpieceRecorder, gadgetId: WorkpieceId, after?: st const summary = workpieces.summaries.get(gadgetId); return summary?.type === "gadget" && summary.commitId !== undefined && summary.commitId !== after ? summary.commitId : null; - }); + }, SETTLE_MS); -const editorOp = fc.record({ - client: fc.constantFrom("left", "right"), - lag: fc.nat({ max: 3 }), - at: fc.nat(), - retry: fc.boolean(), -}); +type Client = "left" | "right"; +type EditorOp = { kind: "insert" | "delete" | "resend"; client: Client; lag: number; at: number }; + +const editorOp = (kind: fc.Arbitrary): fc.Arbitrary => + fc.record({ kind, client: fc.constantFrom("left", "right"), lag: fc.nat({ max: 3 }), at: fc.nat() }); -it.concurrent("stale and retried edits converge, and accepting commits the converged text", async () => { +// A session opens with an edit, so every run has a draft to accept. +const editorSession = fc.tuple( + editorOp(fc.constant("insert")), + fc.array(editorOp(fc.constantFrom("insert", "delete", "resend")), { maxLength: 7 }), +).map(([first, rest]) => [first, ...rest]); + +/** A resend arriving after the other editor's change landed: random cases reach it only sometimes. */ +const resendAfterOther: [number, EditorOp[]] = [2, [ + { kind: "insert", client: "left", lag: 0, at: 0 }, + { kind: "insert", client: "right", lag: 0, at: 0 }, + { kind: "resend", client: "left", lag: 0, at: 0 }, +]]; + +it.concurrent("stale and resent edits converge, and accepting commits the converged text", async () => { await fc.assert(fc.asyncProperty( fc.integer({ min: 1, max: SEED_LINES.length }), - fc.array(editorOp, { minLength: 1, maxLength: 8 }), + editorSession, async (seedCount, ops) => { using stack = new DisposableStack(); const { ws } = await freshWorkspace(stack, "editors"); @@ -393,11 +429,15 @@ it.concurrent("stale and retried edits converge, and accepting commits the conve const chatId = await ws.newChat("Edit together", null); const pins = [{ gadgetId, baseCommit: head }]; - const seqs = { left: 0, right: 0 }; - const stream = (count: number) => waitFor(`${count} changes to chat ${chatId}`, async () => { - const changes = recorder.applied.filter(c => c.chatId === chatId); - return changes.length >= count ? changes.toSorted((a, b) => a.revision - b.revision) : null; - }); + // The changes broadcast to this chat, in arrival order, which must number them 1..n. + const stream = async (count: number) => { + const changes = await waitFor(`${count} changes to chat ${chatId}`, async () => { + const all = recorder.applied.filter(c => c.chatId === chatId); + return all.length >= count ? all : null; + }, SETTLE_MS); + expect(changes.map(c => c.revision)).toEqual(changes.map((_, i) => i + 1)); + return changes; + }; const textAt = (changes: { change: CodeChange }[]) => { const text = changes.reduce((content, { change }) => applyCodeChange(content, change), files(gadgetId, seed)).get(gadgetId)?.get(PATH); @@ -405,32 +445,53 @@ it.concurrent("stale and retried edits converge, and accepting commits the conve return text; }; - const markers: string[] = []; + const last = new Map(); + const inserted: string[] = []; + const deleted = new Set(); let revision = 0; - for (const [i, op] of ops.entries()) { - const base = Math.max(0, revision - op.lag); + for (const [i, { kind, client, lag, at }] of ops.entries()) { + if (kind === "resend") { + const prior = last.get(client); + if (prior) expect(await ws.submitCodeChange(chatId, prior.submission)).toEqual(prior.ack); + continue; + } + const base = Math.max(0, revision - lag); const before = textAt((await stream(revision)).slice(0, base)); const lines = linesOf(before); - const marker = `${op.client} ${i}`; - lines.splice(op.at % (lines.length + 1), 0, marker); - markers.push(marker); - const submission = { - generation: 0, revision: base, clientId: op.client, seq: ++seqs[op.client], pins, - change: diffFiles(files(gadgetId, before), files(gadgetId, textOf(lines))), + // The editor submits its transaction's exact range (CodeEditor.tsx), so edits here are + // spans too: a minimal diff could straddle lines and attribute a deletion to its neighbours. + let span: { line: number; replaced: string; insert: string }; + if (kind === "insert") { + inserted.push(`${client} ${i}`); + span = { line: at % (lines.length + 1), replaced: "", insert: `${inserted.at(-1)}\n` }; + } else if (lines.length > 0) { + const line = at % lines.length; + deleted.add(lines[line]!); + span = { line, replaced: `${lines[line]}\n`, insert: "" }; + } else { + continue; + } + const edit = replaceSpanChange( + before.length, textOf(lines.slice(0, span.line)).length, span.replaced, span.insert); + const submission: CodeChangeSubmission = { + generation: 0, revision: base, clientId: client, seq: (last.get(client)?.submission.seq ?? 0) + 1, + pins, change: { [gadgetId]: [[PATH, { edit }]] }, }; const ack = await ws.submitCodeChange(chatId, submission); expect(ack).toEqual({ generation: 0, revision: ++revision }); - if (op.retry) expect(await ws.submitCodeChange(chatId, submission)).toEqual(ack); + last.set(client, { submission, ack }); } const converged = textAt(await stream(revision)); const lines = linesOf(converged); - expect(lines.filter(line => !markers.includes(line))).toEqual(seedLines); - expect(lines.filter(line => markers.includes(line)).toSorted()).toEqual(markers.toSorted()); + const survivors = (all: string[]) => all.filter(line => !deleted.has(line)); + expect(lines.toSorted()).toEqual(survivors([...seedLines, ...inserted]).toSorted()); + expect(lines.filter(line => seedLines.includes(line))).toEqual(survivors(seedLines)); expect(await ws.mergeChanges(chatId)).toEqual({ outcome: "merged" }); - const merged = await headOf(workpieces, gadgetId, head); + // A draft that nets to no change (a line inserted, then deleted) leaves the head where it was. + const merged = converged === seed ? head : await headOf(workpieces, gadgetId, head); expect(await ws.readFilesAtCommit(merged, [PATH])).toEqual([[PATH, { kind: "text", text: converged }]]); expect(recorder.applied.filter(c => c.chatId === chatId)).toHaveLength(revision); - }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15 }); + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15, examples: [resendAfterOther] }); }); diff --git a/packages/integration-tests/src/property-runs.ts b/packages/integration-tests/src/property-runs.ts index 50197791dc..2b6ef6ba5c 100644 --- a/packages/integration-tests/src/property-runs.ts +++ b/packages/integration-tests/src/property-runs.ts @@ -2,8 +2,9 @@ /** * `FAST_CHECK_SEED` pins every property's seed, so a run generates the same cases each time; CI - * sets it. Unset, fast-check draws a fresh seed per run and prints it with any failure, along with - * the `path` of the shrunk counterexample, so a local failure can be replayed. + * sets it. Unset, fast-check draws a fresh seed whenever the suite actually runs (a cached task + * replays its last result instead) and prints it with any failure, along with the `path` of the + * shrunk counterexample, so a local failure can be replayed. */ function seedFromEnv(): { seed?: number } { const raw = process.env.FAST_CHECK_SEED; From 3b81faed4fee71b3dd7719dc423fa7e1273e23b3 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 2 Nov 2026 10:56:30 -0600 Subject: [PATCH 03/12] Share fast-check run options with workshop-backend's in-isolate tests - PROPERTY_RUN_OPTIONS moves from integration-tests to @gadgets/scripts/property-runs. - workshop-backend gains fast-check; its vitest config forwards FAST_CHECK_SEED into workerd, where nodejs_compat copies it into process.env, and its test task fingerprints it. - The env-passthrough guard accounts for the moved read. --- packages/integration-tests/README.md | 2 +- .../__tests__/workshop-properties.test.ts | 7 ++++--- packages/integration-tests/vite.config.ts | 4 ++-- packages/workshop-backend/package.json | 1 + packages/workshop-backend/vite.config.ts | 13 +++++++++---- packages/workshop-backend/vitest.config.ts | 7 ++++++- pnpm-lock.yaml | 3 +++ scripts/env-passthrough.test.ts | 8 ++++++-- scripts/package.json | 4 ++++ .../src => scripts}/property-runs.ts | 13 ++++++++----- 10 files changed, 44 insertions(+), 18 deletions(-) rename {packages/integration-tests/src => scripts}/property-runs.ts (56%) diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index f6eec9990b..c62e6c6bd1 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -52,7 +52,7 @@ pinned as fast-check `examples`, which run on every seed. Every run is real RPC, so a property costs hundreds of milliseconds per run and `numRuns` is kept small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside the test timeout -(`src/property-runs.ts`). +(`@gadgets/scripts/property-runs`). `FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so a pull request's result depends only on its code. Without it, each run draws a fresh seed, but a diff --git a/packages/integration-tests/__tests__/workshop-properties.test.ts b/packages/integration-tests/__tests__/workshop-properties.test.ts index a0e2cc360b..55a0c620d0 100644 --- a/packages/integration-tests/__tests__/workshop-properties.test.ts +++ b/packages/integration-tests/__tests__/workshop-properties.test.ts @@ -1,7 +1,8 @@ // Property-based end-to-end tests: fast-check generates sequences of the operations a real client // performs, and each run replays one against the real Workshop over RPC, checked against a small -// model. Seeds and the shrink time limit come from `src/property-runs.ts`; a failure prints the -// seed and the shrunk counterexample's path, which replay it when passed to that fc.assert(). +// model. Seeds and the shrink time limit come from `@gadgets/scripts/property-runs`; a failure +// prints the seed and the shrunk counterexample's path, which replay it when passed to that +// fc.assert(). // Scenarios a fixed seed might not reach are pinned as `examples`, which run on every seed. // // Every run allocates a fresh account (and so a fresh workspace and fixture-gatekeeper label), @@ -18,13 +19,13 @@ import type { import { applyCodeChange, diffFiles, replaceSpanChange, type CodeChange, type CodeContent, } from "@gadgets/workshop-shared/code-change"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.js"; import { loadAllChatHistory } from "../src/agent-session.js"; import { startTestGatekeeperHarness, TEST_VENDOR_ID, testActionState, type Harness, } from "../src/harness.js"; import { NetworkInterceptor } from "../src/network-interceptor.js"; -import { PROPERTY_RUN_OPTIONS } from "../src/property-runs.js"; import { accountLabel, connect, listConnectedAccounts, nextUsernames, RpcTarget, signUp, stubFor, waitFor, WorkpieceRecorder, diff --git a/packages/integration-tests/vite.config.ts b/packages/integration-tests/vite.config.ts index cfa1b04a07..ca9d218a83 100644 --- a/packages/integration-tests/vite.config.ts +++ b/packages/integration-tests/vite.config.ts @@ -52,8 +52,8 @@ export default { */ test: { ...testTask, - // The property suites' seed (src/property-runs.ts) decides which cases run, so it is - // passed and fingerprinted: a different seed is a cache miss, not a replayed result. + // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it + // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. cache: { ...testTask.cache, env: [...testTask.cache.env, 'FAST_CHECK_SEED'] }, // Real gatekeepers boot from prebuilt validated trees, tracked as inputs like the fixture, // plus the release the Google upgrade suite starts from, and `cli-admins-only.test.ts` runs diff --git a/packages/workshop-backend/package.json b/packages/workshop-backend/package.json index 0731b43f79..873ef26b08 100644 --- a/packages/workshop-backend/package.json +++ b/packages/workshop-backend/package.json @@ -36,6 +36,7 @@ "@gadgets/scripts": "workspace:*", "dompurify": "^3.4.17", "esbuild": "^0.28.2", + "fast-check": "^4.10.2", "typescript": "catalog:", "vitest": "catalog:", "wrangler": "catalog:" diff --git a/packages/workshop-backend/vite.config.ts b/packages/workshop-backend/vite.config.ts index b3ef5899fc..f32f4c2ff4 100644 --- a/packages/workshop-backend/vite.config.ts +++ b/packages/workshop-backend/vite.config.ts @@ -4,6 +4,11 @@ import { TESTS_WITH_TIMEOUT_ENV, vitestTask, withTestTimeout, } from '@gadgets/scripts/vitest-task' +const unitTask = vitestTask([ + { command: 'vitest run', idleSeconds: 120 }, + 'vitest run --config vitest.integration.config.ts', +]) + /** * Codegen steps stay separate commands rather than one `&&` string so each caches on its own. */ @@ -81,10 +86,10 @@ export default { * Import-heavy unit tests need a longer idle allowance; integration uses the shared defaults. */ test: { - ...vitestTask([ - { command: 'vitest run', idleSeconds: 120 }, - 'vitest run --config vitest.integration.config.ts', - ]), + ...unitTask, + // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it + // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. + cache: { ...unitTask.cache, env: [...unitTask.cache.env, 'FAST_CHECK_SEED'] }, dependsOn: [ '@gadgets/typed-storage#build', 'build:bundled-blueprints', 'build:browser-runtime', ], diff --git a/packages/workshop-backend/vitest.config.ts b/packages/workshop-backend/vitest.config.ts index 9f6afa0616..61c666fa17 100644 --- a/packages/workshop-backend/vitest.config.ts +++ b/packages/workshop-backend/vitest.config.ts @@ -85,7 +85,12 @@ export default defineConfig({ compatibilityDate: COMPATIBILITY_DATE, compatibilityFlags: ['experimental', 'streaming_tail_worker'], }], - bindings: { PUBLIC_BASE_URL: 'https://workshop.example/' }, + // FAST_CHECK_SEED reaches the property suites' `process.env` (see + // @gadgets/scripts/property-runs) through `nodejs_compat`, which copies string bindings there. + bindings: { + PUBLIC_BASE_URL: 'https://workshop.example/', + ...process.env.FAST_CHECK_SEED ? { FAST_CHECK_SEED: process.env.FAST_CHECK_SEED } : {}, + }, // The overseer loads gadget code through this, so a test can run a real gadget facet. workerLoaders: { LOADER: {} }, // Where a blueprint's content is published to and read back from. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e5a6a8798a..d298f9e1cc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1410,6 +1410,9 @@ importers: esbuild: specifier: ^0.28.2 version: 0.28.2 + fast-check: + specifier: ^4.10.2 + version: 4.10.2 typescript: specifier: 'catalog:' version: 7.0.2 diff --git a/scripts/env-passthrough.test.ts b/scripts/env-passthrough.test.ts index d295e3f1fd..86d04c3ffd 100644 --- a/scripts/env-passthrough.test.ts +++ b/scripts/env-passthrough.test.ts @@ -69,9 +69,7 @@ const EXPECTED: Record = { // `src/worker-inputs.ts` resolves BUNDLED_BLUEPRINTS_DIR only to name a directory for the watcher // and `forceRerunTriggers`. The `test` task is cached and strips it; `vitest run` never reads // those exports, and `pnpm test:watch` is a script, so it keeps the ambient value. - // `src/property-runs.ts` reads FAST_CHECK_SEED, which picks the property suites' cases. "packages/integration-tests": { - forwarded: ["FAST_CHECK_SEED"], injected: ["WORKSHOP_INTEGRATION_PREBUILT"], watch: ["BUNDLED_BLUEPRINTS_DIR"], }, @@ -88,7 +86,10 @@ const EXPECTED: Record = { ], injected: ["NODE_ENV"], }, + // `vitest.config.ts` forwards FAST_CHECK_SEED into workerd for the property suites; the `test` + // task declares it. "packages/workshop-backend": { + forwarded: ["FAST_CHECK_SEED"], uncached: ["BUNDLED_BLUEPRINTS_DIR"], }, // `build-gatekeeper-configurator.ts` is covered in detail by @@ -97,8 +98,11 @@ const EXPECTED: Record = { // never as vp tasks. // Every cached watchdogged task declares the timeout policy via TESTS_WITH_TIMEOUT_ENV; // vitest-task.test.ts pins the shared declaration. + // `property-runs.ts` reads FAST_CHECK_SEED inside the `test` tasks of integration-tests and + // workshop-backend, each of which declares it. scripts: { forwarded: [ + "FAST_CHECK_SEED", "TESTS_WITH_TIMEOUT_DISABLE", "TESTS_WITH_TIMEOUT_IDLE_SECONDS", "TESTS_WITH_TIMEOUT_MAX_SECONDS", "VITE_FRONTEND_ERROR_REPORTING", ], diff --git a/scripts/package.json b/scripts/package.json index 62adc87b54..c86c49954d 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -41,6 +41,10 @@ "types": "./worker-config.ts", "import": "./worker-config.ts" }, + "./property-runs": { + "types": "./property-runs.ts", + "import": "./property-runs.ts" + }, "./oxlint-plugin": "./oxlint-plugin.ts" }, "dependencies": { diff --git a/packages/integration-tests/src/property-runs.ts b/scripts/property-runs.ts similarity index 56% rename from packages/integration-tests/src/property-runs.ts rename to scripts/property-runs.ts index 2b6ef6ba5c..dedebb681a 100644 --- a/packages/integration-tests/src/property-runs.ts +++ b/scripts/property-runs.ts @@ -1,4 +1,6 @@ -// fast-check options shared by the property-based suites. +// fast-check options shared by the property-based suites: integration-tests' under Node, and +// workshop-backend's inside workerd, whose vitest config forwards `FAST_CHECK_SEED` as a binding +// that `nodejs_compat` copies into `process.env`. /** * `FAST_CHECK_SEED` pins every property's seed, so a run generates the same cases each time; CI @@ -15,11 +17,12 @@ function seedFromEnv(): { seed?: number } { } /** - * Options for every `fc.assert()` over the Workshop. + * Options for every property's `fc.assert()`. * - * Each run is real RPC, and so is every shrink attempt. The time limit keeps a failing property's - * shrink inside the test timeout; it reports the smallest counterexample found by then. An - * interrupt before any failure still fails, rather than silently passing on fewer runs. + * A run over real RPC costs hundreds of milliseconds, and so does every shrink attempt. The time + * limit keeps such a property's shrink inside its test timeout; it reports the smallest + * counterexample found by then. An interrupt before any failure still fails, rather than silently + * passing on fewer runs. */ export const PROPERTY_RUN_OPTIONS = { interruptAfterTimeLimit: 90_000, From c798ccb424bda0ea1fee39b6779af89f03452ca5 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 2 Nov 2026 10:56:30 -0600 Subject: [PATCH 04/12] Property-test failed applies and observer re-verification end to end - Approvals: the provider may fail one apply, from staging or from any user decision on. A write the drain failed stays pending and is never skipped past; a failed approval is retried or denied; every apply attempt reaches the provider exactly once. - Observer re-verification: a collaborator's 1-3 opens while connections lapse and are repaired, checked against ensureObserver's prompt, rollback, error and observer-id rules. --- packages/integration-tests/README.md | 14 +- .../__tests__/observer-reverification.test.ts | 121 ++++++++++++++ .../__tests__/workshop-properties.test.ts | 150 +++++++++++++++--- .../gatekeeper-test/src/test-gatekeeper.ts | 30 +++- 4 files changed, 278 insertions(+), 37 deletions(-) diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index c62e6c6bd1..1208edabd9 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -45,14 +45,18 @@ vendor this one as a submodule: `__tests__/workshop-properties.test.ts` uses [fast-check](https://fast-check.dev/) to generate sequences of the operations a real client performs and replays each against the Workshop, checking the result against a small model: decisions on staged writes (with and without an auto-approval -rule), chats with shared and same-named attachments created and deleted, and two editors -inserting, deleting and resending stale code changes before an accept. Each run takes a fresh -account, so runs and shrink attempts are independent. Scenarios a fixed seed might miss are -pinned as fast-check `examples`, which run on every seed. +rule, while the provider may fail one apply), chats with shared and same-named attachments created +and deleted, and two editors inserting, deleting and resending stale code changes before an +accept. `__tests__/observer-reverification.test.ts` adds one for a collaborator's opens while +their connections' credentials lapse and are repaired. Each run takes a fresh account, so runs +and shrink attempts are independent. Scenarios a fixed seed might miss are pinned as fast-check +`examples`, which run on every seed. Every run is real RPC, so a property costs hundreds of milliseconds per run and `numRuns` is kept small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside the test timeout -(`@gadgets/scripts/property-runs`). +(`@gadgets/scripts/property-runs`). workshop-backend's in-isolate properties (sharing, blueprint +releases, streamed tool input) share those options, and its `vitest.config.ts` forwards +`FAST_CHECK_SEED` into workerd. `FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so a pull request's result depends only on its code. Without it, each run draws a fresh seed, but a diff --git a/packages/integration-tests/__tests__/observer-reverification.test.ts b/packages/integration-tests/__tests__/observer-reverification.test.ts index 95b14357e6..b65e276500 100644 --- a/packages/integration-tests/__tests__/observer-reverification.test.ts +++ b/packages/integration-tests/__tests__/observer-reverification.test.ts @@ -14,7 +14,9 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import type { RpcStub } from "capnweb"; +import * as fc from "fast-check"; import type { AuthenticatedApi, Overseer, PublicApi } from "@gadgets/workshop-shared/api"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import { startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, testControl, type Harness, } from "../src/harness.js"; @@ -382,6 +384,125 @@ describe("observer re-verification", () => { }); }); +// --------------------------------------------------------------------------------------------- +// A collaborator's opens over time, while each connection's credential lapses or is repaired +// between opens and during the one re-prompt. Checked against ensureObserver()'s documented rules: +// a first open prompts for every binding; every pass verifies every binding; only failed bindings +// are re-prompted, once, with their failure; a terminal error names exactly the bindings still +// failing; a failed first-ever open removes everything it registered, under its own discarded +// observer id; a returning observer's failed open removes nothing (0d7793c); and a successful open +// persists the observer id every later open reuses. + +/** One open: per binding, whether its credential verifies on the first pass, then after a repair. */ +type ObserverOpen = { firstPass: boolean[]; afterReprompt: boolean[] }; + +// Credentials mostly work, so a collaborator also gets to be a returning observer. +const verifies = fc.oneof({ arbitrary: fc.constant(true), weight: 3 }, fc.constant(false)); +const observerOpens: fc.Arbitrary = fc.integer({ min: 1, max: 3 }) + .chain(bindings => fc.array(fc.record({ + firstPass: fc.array(verifies, { minLength: bindings, maxLength: bindings }), + afterReprompt: fc.array(verifies, { minLength: bindings, maxLength: bindings }), + }), { minLength: 1, maxLength: 3 })); + +/** An event as the model predicts it; `observer` is the open that minted the observer id. */ +type ExpectedEvent = { type: "add" | "remove"; observer: number }; + +describe("observer re-verification over a collaborator's opens", () => { + it.concurrent("registers, re-prompts, rolls back and reports exactly as the bindings verify", + async () => { + await fc.assert(fc.asyncProperty(observerOpens, async opens => withSession(async publicApi => { + const [prefix] = nextUsernames("obsprop"); + const names = opens[0]!.firstPass.map((_, i) => `${prefix}-${i}`); + const urls = names.map(thingUrl); + const shared = await shareGadgetWithBob(publicApi, names); + const bob = shared.bobAccount.id; + const setOutcomes = (outcomes: boolean[]) => Promise.all(outcomes.map((allow, i) => + setVerifyOutcome(shared.bobLabel, + allow ? { allow: true } : { allow: false, reason: EXPIRED_REASON }, urls[i]))); + + // The open whose observer id is persisted; undefined until one succeeds. + let persisted: number | undefined; + const expected: ExpectedEvent[][] = urls.map(() => []); + + for (const [k, { firstPass, afterReprompt }] of opens.entries()) { + const returning = persisted !== undefined; + const observer = persisted ?? k; + await setOutcomes(firstPass); + const recorder = new ObserverConfigRecorder(); + if (!returning) recorder.alwaysChoose(bob, 1); + recorder.respondWith(async needs => { + await setOutcomes(afterReprompt); + return needs.map(need => ({ gatekeeperId: need.gatekeeperId, accountId: bob })); + }); + const opened = await bobOpens(shared, recorder).then( + overseer => { overseer[Symbol.dispose](); return null; }, + (error: unknown) => error as Error); + + const passes = firstPass.every(Boolean) ? [firstPass] : [firstPass, afterReprompt]; + for (const pass of passes) { + pass.forEach((ok, i) => { if (ok) expected[i]!.push({ type: "add", observer }); }); + } + const failing = names.filter((_, i) => !passes.at(-1)![i]); + + const prompts = recorder.calls.map(needs => needs + .filter(need => urls.includes(need.resourceUrl!)) + .map(need => ({ name: names[urls.indexOf(need.resourceUrl!)], failure: need.failure }))); + const expectedPrompts = [ + ...returning ? [] : [names.map(name => ({ name, failure: undefined }))], + ...passes.length === 1 ? [] : [names.filter((_, i) => !firstPass[i]).map(name => ({ + name, + failure: { accountId: bob, reason: expect.stringContaining(EXPIRED_REASON) }, + }))], + ]; + expect(prompts, `prompts of open ${k}`).toEqual(expectedPrompts); + + if (failing.length === 0) { + expect(opened, `open ${k}`).toBeNull(); + persisted = observer; + } else { + expect(opened?.message, `open ${k}`).toMatch(/could not confirm/i); + const named = opened!.message.split("\n") + .filter(line => line.includes(shared.bobLabel)) + .map(line => /^Test Thing (\S+) \(/.exec(line)?.[1]); + expect(named.toSorted(), `bindings named by open ${k}`).toEqual(failing.toSorted()); + if (!returning) { + for (const events of expected) events.push({ type: "remove", observer }); + } + } + } + + // Every event names an observer id: the same id for the same open's observer, and a + // different one for each other. A binding that registered on one pass and failed on the next + // is removed once for each (`newlyAdded` and `invalidated` both hold it); removal is + // idempotent, so a repeat is collapsed rather than pinned. + const idOf = new Map(); + for (const [i, url] of urls.entries()) { + const events = (await observerEvents(url)).filter((event, j, all) => + event.type !== "remove" || all[j - 1]?.type !== "remove" || all[j - 1]?.id !== event.id); + expect(events.map(({ type }) => type), `events at ${names[i]}`) + .toEqual(expected[i]!.map(({ type }) => type)); + for (const [j, { id }] of events.entries()) { + const { observer } = expected[i]![j]!; + expect(idOf.get(observer) ?? id, `observer id of open ${observer}`).toBe(id); + idOf.set(observer, id); + } + } + expect(new Set(idOf.values()).size, "distinct observer ids").toBe(idOf.size); + })), { + ...PROPERTY_RUN_OPTIONS, + numRuns: 8, + examples: [ + // The 0d7793c reproduction: a lapses and is repaired as b lapses for good. + [[{ firstPass: [true, true], afterReprompt: [true, true] }, + { firstPass: [false, true], afterReprompt: [true, false] }]], + // A first-ever open fails terminally and rolls back; a later one starts afresh. + [[{ firstPass: [false, true], afterReprompt: [false, true] }, + { firstPass: [true, true], afterReprompt: [true, true] }]], + ], + }); + }); +}); + describe("harness", () => { it.concurrent("serves the Workshop worker", async () => { const res = await harness.server.fetch("/"); diff --git a/packages/integration-tests/__tests__/workshop-properties.test.ts b/packages/integration-tests/__tests__/workshop-properties.test.ts index 55a0c620d0..43e210d007 100644 --- a/packages/integration-tests/__tests__/workshop-properties.test.ts +++ b/packages/integration-tests/__tests__/workshop-properties.test.ts @@ -23,7 +23,7 @@ import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.js"; import { loadAllChatHistory } from "../src/agent-session.js"; import { - startTestGatekeeperHarness, TEST_VENDOR_ID, testActionState, type Harness, + startTestGatekeeperHarness, TEST_VENDOR_ID, testActionState, testControl, type Harness, } from "../src/harness.js"; import { NetworkInterceptor } from "../src/network-interceptor.js"; import { @@ -62,38 +62,67 @@ async function freshWorkspace(stack: DisposableStack, prefix: string) { // --------------------------------------------------------------------------------------------- // Approvals: writes a gadget stages, decided by the user in any order, with and without an -// auto-approval rule. The fixture gatekeeper numbers its actions independently of the Workshop's -// ledger, so every decision also checks the ledger-to-gatekeeper id mapping: approving the wrong -// entry applies the wrong value, or throws in the fixture. +// auto-approval rule, while the provider may fail one apply (a timeout, a 5xx). The fixture +// gatekeeper numbers its actions independently of the Workshop's ledger, so every decision also +// checks the ledger-to-gatekeeper id mapping: approving the wrong entry applies the wrong value, or +// throws in the fixture. type Decision = "approve" | "reject" | "leave"; /** One staged write, and what the user does with it when they reach it (in `rank` order). */ type Write = { value: number; autoApprovable: boolean; decision: Decision; rank: number }; -type ApprovalScenario = { writes: Write[]; ruleEnabled: boolean; concurrent: boolean }; +type ApprovalScenario = { + writes: Write[]; + ruleEnabled: boolean; + concurrent: boolean; + /** + * When the provider fails one apply attempt, whichever write and path makes it: the first + * attempt from staging the writes on (0), or from the user's nth decision on (n). + */ + failBefore: number | null; + /** What the user does when their own approval fails: approve again, or give up on the write. */ + afterFailure: "retry" | "reject"; +}; class ApprovalModel { readonly states: ("pending" | "approved" | "rejected")[]; + /** Whether each approved write was applied by the drain rather than by the user. */ + readonly auto: boolean[]; applied: number[] = []; + attempts = 0; + /** The provider fails the next apply attempt. */ + failNext = false; constructor(readonly values: number[], readonly eligible: boolean[]) { this.states = values.map(() => "pending"); - this.drain(); + this.auto = values.map(() => false); + } + + /** One apply attempt; false when it is the attempt the provider fails. */ + apply(index: number, auto: boolean): boolean { + this.attempts++; + if (this.failNext) { + this.failNext = false; + return false; + } + this.states[index] = "approved"; + this.auto[index] = auto; + this.applied.push(this.values[index]!); + return true; } - decide(index: number, decision: "approve" | "reject") { - this.states[index] = decision === "approve" ? "approved" : "rejected"; - if (decision === "approve") this.applied.push(this.values[index]!); + reject(index: number) { + this.states[index] = "rejected"; this.drain(); } - // The drain: eligible pending writes apply in submission order, stopping at the first pending - // write that needs a human (auto-approval.ts). + // The drain, which runs after each auto-approvable write is staged and after each decision: + // eligible pending writes apply in submission order, stopping at the first that needs a human or + // fails to apply (auto-approval.ts). A write the drain failed stays pending until the user + // decides it or a later drain retries it. drain() { for (const [i, state] of this.states.entries()) { if (state !== "pending") continue; - if (!this.eligible[i]) return; - this.states[i] = "approved"; - this.applied.push(this.values[i]!); + if (!this.eligible[i] || !this.apply(i, true)) return; } } } @@ -107,13 +136,15 @@ const approvalScenario: fc.Arbitrary = fc.record({ }), { minLength: 1, maxLength: 6, selector: write => write.value }), ruleEnabled: fc.boolean(), concurrent: fc.boolean(), + failBefore: fc.option(fc.nat({ max: 4 }), { freq: 2 }), + afterFailure: fc.constantFrom("retry" as const, "reject" as const), }); /** * Deciding a manual write must restart the drain for the auto-approvable writes queued behind it * (the turn-resumption fix, 1200259, lives on this path). Random cases reach it only sometimes. */ -const cascade = (decision: "approve" | "reject"): [ApprovalScenario] => [{ +const cascade = (decision: "approve" | "reject", failBefore: number | null = null): ApprovalScenario => ({ writes: [ { value: 1, autoApprovable: false, decision, rank: 0 }, { value: 2, autoApprovable: true, decision: "leave", rank: 0 }, @@ -121,7 +152,27 @@ const cascade = (decision: "approve" | "reject"): [ApprovalScenario] => [{ ], ruleEnabled: true, concurrent: false, -}]; + failBefore, + afterFailure: "reject", +}); + +const failureExamples: ApprovalScenario[] = [ + // The user's approval fails and they deny the write instead, which must withdraw it (835de9e) + // and still restart the drain. + cascade("approve", 1), + // The restarted drain fails the first write behind the gate: it stops there, never skipping + // ahead to the next. + cascade("reject", 1), + // The drain fails the only write and nothing retries it: it stays pending for the user to + // decide, not dropped or recorded as applied, and approving it is then a manual approval. + ...(["leave", "approve"] as const).map(decision => ({ + writes: [{ value: 1, autoApprovable: true, decision, rank: 0 }], + ruleEnabled: true, + concurrent: false, + failBefore: 0, + afterFailure: "retry" as const, + })), +]; async function writesOf(ws: RpcStub, gatekeeperId: number) { const writes: (ActionLogEntry & { type: "action" })[] = []; @@ -139,8 +190,11 @@ async function writesOf(ws: RpcStub, gatekeeperId: number) { const valueOf = (entry: ActionLogEntry) => Number(/^Set the test value to (-?\d+)$/ .exec(entry.description.title)?.[1]); +const PROVIDER_FAILURE = "The provider is temporarily unavailable."; + it.concurrent("decided writes apply exactly once, to the write the user decided", async () => { - await fc.assert(fc.asyncProperty(approvalScenario, async ({ writes, ruleEnabled, concurrent }) => { + await fc.assert(fc.asyncProperty(approvalScenario, async scenario => { + const { writes, ruleEnabled, concurrent, failBefore, afterFailure } = scenario; using stack = new DisposableStack(); const { api, ws } = await freshWorkspace(stack, "approvals"); await api.provisionAmbientAccount(TEST_VENDOR_ID); @@ -154,23 +208,38 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" if (ruleEnabled) { await ws.setAutoApprovedActionKind(gatekeeperId, { tag: "set-value", label: "Set value" }); } + const failNextApply = async () => { + await testControl(harness, "fail-next-apply", { label, reason: PROVIDER_FAILURE }); + model.failNext = true; + }; const values = writes.map(write => write.value); const eligible = writes.map(write => ruleEnabled && write.autoApprovable); + const model = new ApprovalModel(values, eligible); + if (failBefore === 0) await failNextApply(); const submit = ({ value, autoApprovable }: Write) => session.writeValue(value, { autoApprovable }); // A gadget may fire its writes concurrently, but the drain then races submission, so the // model's "nothing applies past a pending gate" order holds only for sequential submission. if (concurrent && !ruleEnabled) { await Promise.all(writes.map(submit)); } else { - for (const write of writes) await submit(write); + for (const [i, write] of writes.entries()) { + await submit(write); + if (eligible[i]) model.drain(); + } } - const model = new ApprovalModel(values, eligible); const idOf = new Map((await writesOf(ws, gatekeeperId)).map(entry => [valueOf(entry), entry.id])); expect([...idOf.keys()].toSorted()).toEqual(values.toSorted()); + // Apply attempts settle too, and must have finished, not just started: a pending write whose + // attempt is still in flight looks the same in the ledger as one the drain already failed, and + // deciding it then would race that attempt for the one-shot failure. Every attempt reaching the + // provider exactly once is itself part of the contract. const matchesModel = async () => { + const { attempts, settled } = await testControl<{ attempts: number; settled: number }>( + harness, "apply-attempts", { label }); + if (attempts !== model.attempts || settled !== attempts) return null; const entries = await writesOf(ws, gatekeeperId); const states = new Map(entries.map(entry => [valueOf(entry), entry.state])); return values.every((value, i) => states.get(value) === model.states[i]) ? entries : null; @@ -184,13 +253,37 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" }; await waitFor("the initial drain", matchesModel, SETTLE_MS); + // The user works through the pending list, including auto-approvable writes still held behind + // a gate or failed by the drain. + let decisions = 0; for (const i of [...writes.keys()].toSorted((a, b) => writes[a]!.rank - writes[b]!.rank)) { const { value, decision } = writes[i]!; - if (eligible[i] || decision === "leave" || model.states[i] !== "pending") continue; + if (decision === "leave" || model.states[i] !== "pending") continue; + if (++decisions === failBefore) await failNextApply(); const id = idOf.get(value)!; - await (decision === "approve" ? ws.approveAction(id) : ws.rejectAction(id)); - model.decide(i, decision); - await waitFor(`the ledger after ${decision} of ${value}`, matchesModel, SETTLE_MS); + let step: string = decision; + if (decision === "reject") { + await ws.rejectAction(id); + model.reject(i); + } else if (model.apply(i, false)) { + await ws.approveAction(id); + model.drain(); + } else { + const failure = await ws.approveAction(id).then(() => "approved", (error: unknown) => `${error}`); + expect(failure, `the provider's failure to apply ${value}`).toContain(PROVIDER_FAILURE); + await waitFor(`the ledger after the failed approval of ${value}`, matchesModel, SETTLE_MS); + await expectFixtureMatches(); + step = `${afterFailure} after a failed approval`; + if (afterFailure === "retry") { + await ws.approveAction(id); + model.apply(i, false); + model.drain(); + } else { + await ws.rejectAction(id); + model.reject(i); + } + } + await waitFor(`the ledger after ${step} of ${value}`, matchesModel, SETTLE_MS); await expectFixtureMatches(); } @@ -198,11 +291,11 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" // a real delay can show it, and only while an eligible write is still held behind a gate. if (eligible.some((isEligible, i) => isEligible && model.states[i] === "pending")) await sleep(300); const entries = await matchesModel(); - expect(entries, "ledger states after settling").not.toBeNull(); + expect(entries, "ledger states and apply attempts after settling").not.toBeNull(); for (const entry of entries!) { const i = values.indexOf(valueOf(entry)); if (model.states[i] === "approved") { - expect(!!entry.autoApproved, `autoApproved of write ${values[i]}`).toBe(eligible[i]); + expect(!!entry.autoApproved, `autoApproved of write ${values[i]}`).toBe(model.auto[i]); } } await expectFixtureMatches(); @@ -215,7 +308,12 @@ it.concurrent("decided writes apply exactly once, to the write the user decided" await expect(ws.rejectAction(id)).rejects.toThrow(`Action is not pending: ${id}`); await expectFixtureMatches(); } - }), { ...PROPERTY_RUN_OPTIONS, numRuns: 15, examples: [cascade("reject"), cascade("approve")] }); + }), { + ...PROPERTY_RUN_OPTIONS, + numRuns: 15, + examples: [cascade("reject"), cascade("approve"), ...failureExamples] + .map(example => [example] as [ApprovalScenario]), + }); }); // --------------------------------------------------------------------------------------------- diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index fd2407ae36..fd71b87b2c 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -251,6 +251,18 @@ export class TestControl extends DurableObject { return this.ctx.storage.kv.get(`apply-attempts:${label}`) ?? 0; } + recordApplySettled(label: string): void { + this.ctx.storage.kv.put(`apply-settled:${label}`, this.getApplySettled(label) + 1); + } + + /** + * applyAction() calls that have finished, applied or failed. Below getApplyAttempts(), an attempt + * is still in flight, and its outcome (a one-shot failure included) is not yet decided. + */ + getApplySettled(label: string): number { + return this.ctx.storage.kv.get(`apply-settled:${label}`) ?? 0; + } + /** Parks the next applyAction() for `label` until releaseApply(), so a test can race another. */ holdNextApply(label: string): void { this.ctx.storage.kv.put(`hold-next-apply:${label}`, true); @@ -847,10 +859,14 @@ export class TestGatekeeper const { label } = this.ctx.props; const held = await state.takeNextApplyHold(label); await state.recordApplyAttempt(label); - if (held) await waitForApplyRelease(state, label); - const failure = await state.takeApplyFailure(label); - if (failure !== null) throw new Error(failure); - await state.applyAction(label, action); + try { + if (held) await waitForApplyRelease(state, label); + const failure = await state.takeApplyFailure(label); + if (failure !== null) throw new Error(failure); + await state.applyAction(label, action); + } finally { + await state.recordApplySettled(label); + } } async rejectAction(action: number): Promise { @@ -1052,11 +1068,13 @@ export default { return Response.json({ configurations: await control(ctx.exports).getCreations(label) }); } - // Body: {"label": "..."} -> {"attempts": number} + // Body: {"label": "..."} -> {"attempts": number, "settled": number} + // `settled` is read first, so equal counts mean nothing was in flight in between. if (url.pathname === "/control/apply-attempts" && req.method === "POST") { const { label } = body as Record; if (!isNonEmptyString(label)) return badRequest("`label` must be a non-empty string"); - return Response.json({ attempts: await control(ctx.exports).getApplyAttempts(label) }); + const settled = await control(ctx.exports).getApplySettled(label); + return Response.json({ attempts: await control(ctx.exports).getApplyAttempts(label), settled }); } // One-shot: the next applyAction() for `label` waits for /control/release-apply. From 3d83bad12aa0f00003ea8c8223d5e663eb3d78ec Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 2 Nov 2026 10:56:30 -0600 Subject: [PATCH 05/12] Property-test sharing, blueprint releases and streamed tool input in-isolate - Sharing: Share-dialog action sequences against a reachability model, with remove/revoke previews checked against what they actually change. - Blueprint releases: generated gadget trees round-trip through pack and install, match GitStore's tree, and get an id that depends on the files alone. - Streamed tool input: arbitrary delta splits of real tool-call JSON keep the parser and the code-preview managers in step with the decodable prefix. --- .../__tests__/blueprint-release.test.ts | 94 +++++ .../__tests__/code-preview.test.ts | 86 ++++ .../__tests__/sharing.test.ts | 367 +++++++++++++++++- .../__tests__/streaming-json-parser.test.ts | 58 +++ .../__tests__/tool-input-streams.ts | 117 ++++++ 5 files changed, 720 insertions(+), 2 deletions(-) create mode 100644 packages/workshop-backend/__tests__/tool-input-streams.ts diff --git a/packages/workshop-backend/__tests__/blueprint-release.test.ts b/packages/workshop-backend/__tests__/blueprint-release.test.ts index 781ca6ed0b..0b2202050f 100644 --- a/packages/workshop-backend/__tests__/blueprint-release.test.ts +++ b/packages/workshop-backend/__tests__/blueprint-release.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; +import * as fc from "fast-check"; import { deflateSync } from "node:zlib"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import { MAX_FILE_PATH_LENGTH } from "@gadgets/workshop-shared/code-change"; import { MAX_RELEASE_COMMITS, @@ -377,6 +379,98 @@ describe("release packs", () => { }); }); +// Property: publishing a gadget's files and installing them elsewhere. fast-check generates the +// small file trees real gadgets have -- source under nested directories, a directory beside a +// file of the same stem (`utils/` and `utils.ts`, which git sorts unlike a plain string compare), +// identical and empty files, unicode and CRLF text -- and checks, for each, that: +// - the snapshot release's tree is the one the workspace's git cache writes for the same files, so +// a workspace's own commit and the release agree on every tree id; +// - building its pack, reading it back and listing it gives exactly those files; +// - its commit id is a function of the files alone: the same in whatever order they are given, +// and different once any one file's text changes. +// Malformed packs are the examples' business ("release pack validation"), not this property's. +describe("release round trip, for generated gadget trees", () => { + type Entry = [path: string, text: string]; + + // Directory names and file stems overlap, as `utils/` and `utils.ts` do in real projects. + const directory = fc.constantFrom( + "src", "lib", "public", "components", "utils", "api", "assets", "styles", "server", + "docs", "na\u00efve"); + const fileName = fc.oneof( + fc.tuple( + fc.constantFrom( + "index", "app", "main", "utils", "api", "api-client", "server", "lib", "components", + "styles", "README", "caf\u00e9"), + fc.constantFrom(".ts", ".tsx", ".js", ".html", ".css", ".json", ".md", "")) + .map(([stem, extension]) => stem + extension), + fc.constantFrom(".gitignore", ".env.example", "LICENSE")); + const path = fc.tuple(fc.array(directory, { maxLength: 3 }), fileName) + .map(([directories, name]) => [...directories, name].join("/")); + // Lines of printable text in either line ending, or one of a few texts that real gadgets + // repeat across files, so that one blob can sit under several paths. + const text = fc.oneof( + fc.tuple( + fc.array(fc.string({ unit: "grapheme", maxLength: 40 }), { maxLength: 8 }), + fc.constantFrom("\n", "\r\n"), fc.boolean()) + .map(([lines, ending, final]) => lines.join(ending) + (final ? ending : "")), + fc.constantFrom("", "export {};\n", "MIT License\n")); + // A path cannot be both a file and a directory; a gadget holds whichever came deeper. + const files = fc.uniqueArray(fc.tuple(path, text), { + minLength: 1, maxLength: 15, selector: ([filePath]) => filePath, + }).map(entries => entries.filter(([filePath]) => + !entries.some(([other]) => other.startsWith(`${filePath}/`)))); + const scenario = files.chain(entries => fc.record({ + entries: fc.constant(entries), + reordered: fc.shuffledSubarray(entries, { + minLength: entries.length, maxLength: entries.length, + }), + edit: fc.record({ index: fc.nat({ max: entries.length - 1 }), text }) + .filter(edit => edit.text !== entries[edit.index][1]), + })); + type Scenario = { entries: Entry[], reordered: Entry[], edit: { index: number, text: string } }; + + // Each sort trap at once, beneath a nested directory, with shared and empty files: what a + // random seed reaches only now and then. + const sortTraps: Entry[] = [ + ["src/utils.ts", "export * from \"./utils/format\";\n"], + ["src/utils/format.ts", "export const format = (s: string) => s.trim();\n"], + ["src/utils-v2.ts", "export {};\n"], + ["src/utils0.ts", "export {};\n"], + ["src/components/na\u00efve/caf\u00e9.tsx", "\ufeffexport default \"\u2615\";\r\n"], + ["public/index.html", "\n\u{1F600}\n"], + [".gitignore", ""], + ]; + const examples: [Scenario][] = [[{ + entries: sortTraps, + reordered: sortTraps.toReversed(), + edit: { index: 1, text: "export const format = (s: string) => s;\n" }, + }]]; + + it("installs exactly the published files, under an id they alone determine", async () => { + await fc.assert(fc.asyncProperty(scenario, async ({ entries, reordered, edit }) => { + let published = new Map(entries); + let { commitId, objects } = await buildSnapshotRelease(published); + + let store = makeLocalGitCache(); + let viaStore = await store.writeFilesAsCommit( + published, { parents: [], author: ALICE, message: "files", timestamp: new Date(0) }); + expect(parseGitCommitRefs(objects.get(commitId)!.payload).tree, + "the release's tree is the one the git cache writes").toBe(store.commitTree(viaStore)); + + let pack = await buildReleasePack(oid => objects.get(oid), commitId); + let installed = listReleaseFiles(await readReleasePack(pack, commitId), commitId); + expect(installed, "the installed files are the published ones").toStrictEqual(published); + + expect((await buildSnapshotRelease(new Map(reordered))).commitId, + "the same files in another order make the same release").toBe(commitId); + + let edited = new Map(published).set(entries[edit.index][0], edit.text); + expect((await buildSnapshotRelease(edited)).commitId, + `editing ${entries[edit.index][0]} makes another release`).not.toBe(commitId); + }), { ...PROPERTY_RUN_OPTIONS, numRuns: 100, examples }); + }); +}); + describe("release pack validation", () => { it("refuses a pack over the size cap", async () => { await expect(readReleasePack(new Uint8Array(MAX_RELEASE_PACK_BYTES + 1), ABSENT)) diff --git a/packages/workshop-backend/__tests__/code-preview.test.ts b/packages/workshop-backend/__tests__/code-preview.test.ts index 62082814b7..1b708303ba 100644 --- a/packages/workshop-backend/__tests__/code-preview.test.ts +++ b/packages/workshop-backend/__tests__/code-preview.test.ts @@ -1,6 +1,12 @@ import { describe, it, expect } from 'vitest'; import { CodePreviewManager, ExecuteCodeStreamManager } from '../src/code-preview'; import type { AiChatStreamEvent } from '@gadgets/workshop-shared/api'; +import * as fc from 'fast-check'; +import { PROPERTY_RUN_OPTIONS } from '@gadgets/scripts/property-runs'; +import { + cutDeltas, decodableValue, deltaSizes, previewedToolCall, serialize, STREAMED_FIELD, + type PreviewedToolCall, +} from './tool-input-streams'; // =========================================================================== // Helpers @@ -214,3 +220,83 @@ describe('ExecuteCodeStreamManager', () => { expect(deltas.map(ev => ev.delta).join("")).toBe("run('😀')"); }); }); + +// =========================================================================== +// Property: preview deltas of a streamed tool call +// =========================================================================== + +// The client rebuilds a streamed tool call's preview by concatenating its deltas (editPreviewDelta +// for writeFile / editFile, toolCodeDelta for executeCode), which reach it UTF-8 encoded. For the +// calls agents make, cut into provider deltas anywhere (see tool-input-streams.ts), every delta must +// be well-formed UTF-16 -- a lone surrogate would arrive as U+FFFD -- the text must keep up with +// the input, holding back only a high surrogate whose low half is still to come, and the whole +// must be exactly the field's value. +describe('code preview property', () => { + it('streams well-formed deltas that keep up with the input and add up to it', () => { + let surrogateCall: PreviewedToolCall = { + toolName: "writeFile", + input: {workpiece: "app", filename: "app.ts", content: "a😀b\\\"😀"}, + asciiOnly: true, + spaced: true, + }; + fc.assert(fc.property(previewedToolCall, deltaSizes, (call, sizes) => { + let field = STREAMED_FIELD[call.toolName]; + let value = call.input[field]; + let {json, valueStart} = serialize(call); + let events: AiChatStreamEvent[] = []; + let edits = new CodePreviewManager(ev => events.push(ev), () => ({workpieceId: 7})); + let code = new ExecuteCodeStreamManager(ev => events.push(ev)); + + edits.startToolCall(CALL, call.toolName); + code.startToolCall(CALL, call.toolName); + let received = 0; + let checked = 0; + let text = ""; + for (let delta of cutDeltas(json, sizes)) { + edits.appendInput(CALL, delta); + code.appendInput(CALL, delta); + received += delta.length; + let at = `after ${received} of ${json.length} chars of ${json}`; + for (let ev of events.slice(checked)) { + if (ev.type !== "editPreviewDelta" && ev.type !== "toolCodeDelta") continue; + expect(ev.delta.isWellFormed(), `${at}: delta ${JSON.stringify(ev.delta)}`).toBe(true); + text += ev.delta; + } + checked = events.length; + let expected = decodableValue(json, valueStart, received); + if (received < json.length - 1 && /[\ud800-\udbff]$/.test(expected)) { + expected = expected.slice(0, -1); + } + // Compared JSON-encoded: a lone surrogate in a failure message is invalid UTF-8 to the + // vitest pool's WebSocket, which then drops the report. + expect(JSON.stringify(text), at).toBe(JSON.stringify(expected)); + } + edits.finishToolCall(CALL, true); + code.finishToolCall(CALL); + + let finalText = events.map(ev => + ev.type === "editPreviewDelta" || ev.type === "toolCodeDelta" ? ev.delta : "").join(""); + expect(finalText).toBe(value); + expect(ofType(events, "editPreviewClear")).toEqual([]); + if (call.toolName !== "executeCode") { + let filename = call.input.filename; + expect(ofType(events, "editPreviewStart")).toEqual([{ + type: "editPreviewStart", + toolCallId: CALL, + file: {workpieceId: 7, filename}, + ...call.toolName === "editFile" ? {textToReplace: call.input.textToReplace} : {}, + }]); + } + }), { + ...PROPERTY_RUN_OPTIONS, + numRuns: 200, + examples: [ + // One UTF-16 code unit per delta, so both a raw pair and a `\ud83d\ude00` pair split. + [surrogateCall, [1]], + [{...surrogateCall, asciiOnly: false}, [1]], + [{toolName: "executeCode", input: {code: "run('😀')"}, asciiOnly: true, spaced: false}, + [1]], + ], + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/sharing.test.ts b/packages/workshop-backend/__tests__/sharing.test.ts index 21eb072a90..9b0f30948d 100644 --- a/packages/workshop-backend/__tests__/sharing.test.ts +++ b/packages/workshop-backend/__tests__/sharing.test.ts @@ -1,10 +1,12 @@ import { describe, it, expect } from "vitest"; +import * as fc from "fast-check"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import { createTypedStorage, collection } from "@gadgets/typed-storage"; import { SharingManager, SharingStorage, accessIncludesGadget } from "../src/sharing.js"; import type { CollaboratorRecord, ShareKeyRecord } from "../src/storage-schema/overseer-storage.js"; import { - AiChatAuthorInfo, PermissionEdge, CollaboratorRole, CollaboratorAccess, WorkpieceId, - getOpenWorkspaceErrorCode, OPEN_WORKSPACE_ERROR_CODES, + AffectedCollaborator, AiChatAuthorInfo, PermissionEdge, CollaboratorRole, CollaboratorAccess, + CollaboratorGrant, WorkpieceId, getOpenWorkspaceErrorCode, OPEN_WORKSPACE_ERROR_CODES, } from "@gadgets/workshop-shared/api"; import { makeMockStorage } from "./mock-storage.js"; @@ -1214,3 +1216,364 @@ describe("ownerInvitesOnly", () => { expect(mgr.getEffectiveAccess("b")).toEqual(only(1)); }); }); + +// --------------------------------------------------------------------------------------------- +// Property: the sharing graph under the actions the Share dialog offers. fast-check generates a +// sequence of steps by the owner and four other people -- invite someone by name, create a share +// link, copy it, open a link, revoke a link, remove someone -- and replays it against a +// SharingManager. Each invite and link grants the workspace or one gadget. Removal and revocation +// go through the dialog's flow: preview, then confirm with some of the previewed people kept. +// After every step each person's access is checked against a model. +// +// The model keeps only the grants: what each person granted whom (all of it, since a repeat +// invite from the same person never narrows what they granted), which links each person opened, +// and which links are revoked. Access follows the rule the module documents: a grant counts only +// while its sharer (a link's creator) holds "build", and the owner holds "build". So "build" is +// reachability from the owner over grants of the workspace, and anyone else may use each gadget a +// "build" holder granted them. A link's grants come from its creator. Removal and revocation never +// touch other people's grants; downstream people just stop being reachable, and come back when a +// path does. Keeping someone gives them fresh grants from the actor of what they lost. +// +// Steps only take actions the Workshop offers the acting person: only someone with "build" can +// share (a "use" session is refused every sharing call), so someone who lost it cannot act at +// all; a collaborator removes only people they invited and revokes or copies only links they +// created; and the dialog lists only people who currently have access and live links. Anyone may +// open any link they were sent, live or revoked, with or without access. +// Scenarios a seed might miss are pinned as `examples`. + +const PEOPLE = [OWNER, "ana", "ben", "cy", "dot"]; +// The gadgets the steps share; makeManager's workspace has them all. +const SHARED_GADGETS: WorkpieceId[] = [1, 2]; + +// `person` and `target` are indices into PEOPLE (0 is the owner). Every other number picks, modulo +// the count, from what is on offer when the step runs: `actor` among the people who can share +// (the owner, then the rest in PEOPLE order); `choice` among the dialog's (actor, link) or (actor, +// person) pairs, by actor and then by link creation or PEOPLE order; `link` and `key` among every +// link created so far and that link's keys. `keep` is a bitmask over PEOPLE indices choosing which +// previewed people to keep. +type Step = + | { kind: "invite"; actor: number; target: number; grant: CollaboratorGrant } + | { kind: "createLink"; actor: number; grant: CollaboratorGrant } + | { kind: "copyLink"; choice: number } + | { kind: "open"; person: number; link: number; key: number } + | { kind: "revoke"; choice: number; keep: number } + | { kind: "remove"; choice: number; keep: number }; + +// One grant in the model: "build" of the workspace, or use of one gadget. +type ModelGrant = "build" | WorkpieceId; + +function modelGrant(grant: CollaboratorGrant): ModelGrant { + return grant === "workspace" ? "build" : grant.gadgetId; +} + +function describeGrant(grant: CollaboratorGrant): string { + return grant === "workspace" ? "the workspace" : `gadget ${grant.gadgetId}`; +} + +// Whether going from `before` to `after` (undefined: no access) narrows what someone may do. +function narrows(before: CollaboratorAccess, after: CollaboratorAccess | undefined): boolean { + if (!after) return true; + if (before.role === "build") return after.role !== "build"; + return SHARED_GADGETS.some(id => accessIncludesGadget(before, id) && !accessIncludesGadget(after, id)); +} + +// The grants restoring what someone with `prior` access lost on dropping to `now`. +function lostGrants(prior: CollaboratorAccess, now: CollaboratorAccess | undefined): ModelGrant[] { + if (prior.role === "build") return ["build"]; + return SHARED_GADGETS.filter(id => accessIncludesGadget(prior, id) && !(now && accessIncludesGadget(now, id))); +} + +type ModelLink = { id: string; creator: string; grant: ModelGrant; revoked: boolean; keys: string[] }; + +class SharingModel { + /** target -> inviter -> everything that inviter granted them. */ + invites = new Map>>(); + /** person -> ids of the links they opened. */ + opened = new Map>(); + links: ModelLink[] = []; + + invite(target: string, inviter: string, grant: ModelGrant) { + let inviters = this.invites.get(target) ?? new Map>(); + inviters.set(inviter, (inviters.get(inviter) ?? new Set()).add(grant)); + this.invites.set(target, inviters); + } + + open(person: string, linkId: string) { + this.opened.set(person, (this.opened.get(person) ?? new Set()).add(linkId)); + } + + /** Everyone but the owner who has access, with that access. */ + accesses(): Map { + let edges: { from: string; to: string; grant: ModelGrant }[] = []; + for (let [to, inviters] of this.invites) { + for (let [from, grants] of inviters) { + for (let grant of grants) edges.push({ from, to, grant }); + } + } + for (let [to, linkIds] of this.opened) { + for (let shareLink of this.links) { + if (linkIds.has(shareLink.id) && !shareLink.revoked) { + edges.push({ from: shareLink.creator, to, grant: shareLink.grant }); + } + } + } + let builders = new Set([OWNER]); + for (let grew = true; grew;) { + grew = false; + for (let edge of edges) { + if (edge.grant === "build" && builders.has(edge.from) && !builders.has(edge.to)) { + builders.add(edge.to); + grew = true; + } + } + } + let users = new Map>(); + for (let edge of edges) { + if (edge.grant !== "build" && builders.has(edge.from) && !builders.has(edge.to)) { + users.set(edge.to, (users.get(edge.to) ?? new Set()).add(edge.grant)); + } + } + builders.delete(OWNER); + let result = new Map(); + for (let id of builders) result.set(id, BUILD); + for (let [id, gadgetIds] of users) result.set(id, only(...[...gadgetIds].toSorted((a, b) => a - b))); + return result; + } + + access(id: string): CollaboratorAccess | undefined { + return id === OWNER ? BUILD : this.accesses().get(id); + } +} + +type Change = { id: string; oldAccess: CollaboratorAccess; newAccess: CollaboratorAccess | null }; + +// The people whose access narrowed going from `before` to `after`. (None of these steps widens +// anyone's.) +function changes(before: Map, after: Map): Change[] { + let result: Change[] = []; + for (let [id, oldAccess] of before) { + let newAccess = after.get(id); + if (narrows(oldAccess, newAccess)) result.push({ id, oldAccess, newAccess: newAccess ?? null }); + } + return result.toSorted((a, b) => a.id.localeCompare(b.id)); +} + +function reported(list: AffectedCollaborator[]): Change[] { + return list.map(x => ({ id: x.profile.id, oldAccess: x.oldAccess, newAccess: x.newAccess })) + .toSorted((a, b) => a.id.localeCompare(b.id)); +} + +function caller(id: string) { + return { profileId: id, isOwner: id === OWNER }; +} + +function pick(list: T[], index: number): T | undefined { + return list.length === 0 ? undefined : list[index % list.length]; +} + +async function replaySharing(steps: Step[]) { + let { mgr } = makeManager(); + let model = new SharingModel(); + let sharers = () => PEOPLE.filter(id => model.access(id)?.role === "build"); + // The live links each sharer may revoke or copy: any, for the owner; their own, for anyone else. + let linkActions = () => sharers().flatMap(actor => model.links + .filter(shareLink => !shareLink.revoked && (actor === OWNER || shareLink.creator === actor)) + .map(shareLink => ({ actor, shareLink }))); + // The people each sharer may remove: anyone with access, for the owner; people they invited who + // still have access, for anyone else. + let removals = () => { + let accesses = model.accesses(); + return sharers().flatMap(actor => PEOPLE + .filter(target => accesses.has(target) && target !== actor + && (actor === OWNER || model.invites.get(target)?.has(actor))) + .map(target => ({ actor, target }))); + }; + + // Confirm a previewed removal or revocation, keeping the previewed people `keep` selects (never + // the person being removed, whom the dialog does not offer). The preview must name exactly who + // the model says loses access or some of it; the confirmation, the same minus whoever the kept + // people's fresh grants restore. + let confirm = (opts: { + label: string; actor: string; keep: number; before: Map; + preview: Change[]; target?: string; commit: (keepUsers: string[]) => Change[]; + }) => { + let afterSever = model.accesses(); + expect(opts.preview, `${opts.label}: preview`).toEqual(changes(opts.before, afterSever)); + let keepUsers = opts.preview.map(x => x.id) + .filter(id => id !== opts.target && (opts.keep & (1 << PEOPLE.indexOf(id))) !== 0); + let actual = opts.commit(keepUsers); + for (let id of keepUsers) { + for (let grant of lostGrants(opts.before.get(id)!, afterSever.get(id))) { + model.invite(id, opts.actor, grant); + } + } + expect(actual, `${opts.label} keeping [${keepUsers.join(", ")}]: people reported as losing access`) + .toEqual(changes(opts.before, model.accesses())); + }; + + for (let [index, step] of steps.entries()) { + let label = `step ${index}`; + switch (step.kind) { + case "invite": { + let actor = pick(sharers(), step.actor)!, target = PEOPLE[step.target]; + if (actor === target) break; + label += `: ${actor} invites ${target} to ${describeGrant(step.grant)}`; + let info = mgr.addCollaborator({ caller: caller(actor), profile: profile(target), grant: step.grant }); + model.invite(target, actor, modelGrant(step.grant)); + expect(info.access, `${label}: access reported for ${target}`).toEqual(model.access(target)); + break; + } + case "createLink": { + let actor = pick(sharers(), step.actor)!; + label += `: ${actor} creates link ${model.links.length} to ${describeGrant(step.grant)}`; + let { key, linkId } = await mgr.createShareLink({ caller: caller(actor), grant: step.grant }); + model.links.push({ id: linkId, creator: actor, grant: modelGrant(step.grant), revoked: false, keys: [key] }); + break; + } + case "copyLink": { + let chosen = pick(linkActions(), step.choice); + if (!chosen) break; + let { actor, shareLink } = chosen; + label += `: ${actor} copies link ${model.links.indexOf(shareLink)}`; + let { key } = await mgr.newShareLinkKey({ caller: caller(actor), linkId: shareLink.id }); + shareLink.keys.push(key); + break; + } + case "open": { + let person = PEOPLE[step.person], shareLink = pick(model.links, step.link); + if (!shareLink) break; + let keyIndex = step.key % shareLink.keys.length; + label += `: ${person} opens link ${model.links.indexOf(shareLink)} with key ${keyIndex}`; + await mgr.redeemShareKey({ + rawKey: shareLink.keys[keyIndex], + profileId: person, + fetchProfile: async () => profile(person), + }); + if (!shareLink.revoked) model.open(person, shareLink.id); + break; + } + case "revoke": { + let chosen = pick(linkActions(), step.choice); + if (!chosen) break; + let { actor, shareLink } = chosen; + label += `: ${actor} revokes link ${model.links.indexOf(shareLink)}`; + let before = model.accesses(); + let preview = reported(mgr.previewRevokeShareLink(caller(actor), shareLink.id)); + shareLink.revoked = true; + confirm({ + label, actor, keep: step.keep, before, preview, + commit: keepUsers => reported(mgr.revokeShareLink(caller(actor), shareLink.id, keepUsers)), + }); + break; + } + case "remove": { + let chosen = pick(removals(), step.choice); + if (!chosen) break; + let { actor, target } = chosen; + label += `: ${actor} removes ${target}`; + let before = model.accesses(); + let preview = reported(mgr.previewRemoveCollaborator(caller(actor), target)); + // The owner cuts every grant the target holds; anyone else, only their own invites. + if (actor === OWNER) { + model.invites.delete(target); + model.opened.delete(target); + } else { + model.invites.get(target)!.delete(actor); + } + confirm({ + label, actor, keep: step.keep, before, preview, target, + commit: keepUsers => reported(mgr.removeCollaborator(caller(actor), target, keepUsers)), + }); + break; + } + } + + let expected = model.accesses(); + for (let id of PEOPLE.slice(1)) { + expect(mgr.getEffectiveAccess(id), `${label}: ${id}'s access`).toEqual(expected.get(id)); + } + expect(Object.fromEntries(mgr.listCollaborators().map(c => [c.profile.id, c.access])), + `${label}: people with access`).toEqual(Object.fromEntries(expected)); + expect(mgr.listShareLinkRecords().map(l => l.id).toSorted(), `${label}: live links`) + .toEqual(model.links.filter(l => !l.revoked).map(l => l.id).toSorted()); + } +} + +describe("sharing graph property", () => { + const grant = fc.oneof( + fc.constant("workspace"), + fc.constantFrom(...SHARED_GADGETS).map((gadgetId): CollaboratorGrant => ({ gadgetId }))); + const person = fc.integer({ min: 1, max: 4 }); + const choice = fc.nat({ max: 15 }); + const keep = fc.nat({ max: (1 << PEOPLE.length) - 1 }); + // Grants outweigh removals, so people often hold several paths when one is cut. + const step: fc.Arbitrary = fc.oneof( + { arbitrary: fc.record({ kind: fc.constant("invite" as const), actor: choice, target: person, grant }), weight: 4 }, + { arbitrary: fc.record({ kind: fc.constant("createLink" as const), actor: choice, grant }), weight: 2 }, + { arbitrary: fc.record({ kind: fc.constant("copyLink" as const), choice }), weight: 1 }, + { arbitrary: fc.record({ kind: fc.constant("open" as const), person, link: choice, key: choice }), weight: 4 }, + { arbitrary: fc.record({ kind: fc.constant("revoke" as const), choice, keep }), weight: 1 }, + { arbitrary: fc.record({ kind: fc.constant("remove" as const), choice, keep }), weight: 1 }, + ); + + // Indices into PEOPLE. The comments resolve each pick. + const [ANA, BEN, CY] = [1, 2, 3]; + const examples: [Step[]][] = [ + // Removing a link's creator cuts off whoever joined through it. Re-adding them with one gadget + // leaves those people cut off, since only "build" supports grants; re-adding them to the + // workspace restores them. + [[ + { kind: "invite", actor: 0, target: ANA, grant: "workspace" }, // owner + { kind: "createLink", actor: 1, grant: "workspace" }, // ana + { kind: "open", person: BEN, link: 0, key: 0 }, + { kind: "remove", choice: 0, keep: 0 }, // owner removes ana + { kind: "invite", actor: 0, target: ANA, grant: { gadgetId: 1 } }, + { kind: "invite", actor: 0, target: ANA, grant: "workspace" }, + ]], + // A collaborator withdraws their own invite: ana keeps the owner's grant of gadget 1, and cy, + // who joined through ana's link, loses access, since ana no longer holds "build". + [[ + { kind: "invite", actor: 0, target: ANA, grant: { gadgetId: 1 } }, + { kind: "invite", actor: 0, target: BEN, grant: "workspace" }, + { kind: "invite", actor: 1, target: ANA, grant: "workspace" }, // ben + { kind: "createLink", actor: 1, grant: "workspace" }, // ana + { kind: "open", person: CY, link: 0, key: 0 }, + { kind: "remove", choice: 3, keep: 0 }, // ben removes ana + ]], + // Revoking a link while keeping ben, who joined through it, also keeps cy, whom ben invited. + [[ + { kind: "invite", actor: 0, target: ANA, grant: "workspace" }, + { kind: "createLink", actor: 1, grant: "workspace" }, // ana + { kind: "open", person: BEN, link: 0, key: 0 }, + { kind: "invite", actor: 2, target: CY, grant: { gadgetId: 1 } }, // ben + { kind: "revoke", choice: 0, keep: 1 << BEN }, // owner revokes link 0 + ]], + // Neither a copied key nor the original lets anyone in once the link is revoked. + [[ + { kind: "createLink", actor: 0, grant: { gadgetId: 1 } }, + { kind: "copyLink", choice: 0 }, + { kind: "open", person: ANA, link: 0, key: 1 }, + { kind: "revoke", choice: 0, keep: 0 }, + { kind: "open", person: BEN, link: 0, key: 1 }, + { kind: "open", person: ANA, link: 0, key: 0 }, + ]], + // Keeping someone who lost one of two gadgets restores only that one, from the actor: when + // the other gadget's path is cut later, they keep the restored one, and re-adding its sharer + // brings the other back. + [[ + { kind: "invite", actor: 0, target: ANA, grant: "workspace" }, + { kind: "invite", actor: 0, target: CY, grant: "workspace" }, + { kind: "invite", actor: 1, target: BEN, grant: { gadgetId: 2 } }, // ana + { kind: "invite", actor: 2, target: BEN, grant: { gadgetId: 1 } }, // cy + { kind: "remove", choice: 0, keep: 1 << BEN }, // owner removes ana, keeping ben + { kind: "remove", choice: 1, keep: 0 }, // owner removes cy + { kind: "invite", actor: 0, target: CY, grant: "workspace" }, + ]], + ]; + + it("gives each person the access the grants still reachable from the owner allow", async () => { + await fc.assert( + fc.asyncProperty(fc.array(step, { minLength: 1, maxLength: 30, size: "medium" }), replaySharing), + { ...PROPERTY_RUN_OPTIONS, numRuns: 200, examples }); + }); +}); diff --git a/packages/workshop-backend/__tests__/streaming-json-parser.test.ts b/packages/workshop-backend/__tests__/streaming-json-parser.test.ts index c0edfacc0b..47fb7c504f 100644 --- a/packages/workshop-backend/__tests__/streaming-json-parser.test.ts +++ b/packages/workshop-backend/__tests__/streaming-json-parser.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from 'vitest'; +import * as fc from 'fast-check'; +import { PROPERTY_RUN_OPTIONS } from '@gadgets/scripts/property-runs'; import { StreamingToolInputParser } from '../src/streaming-json-parser.js'; +import { + cutDeltas, decodableValue, deltaSizes, previewedToolCall, serialize, STREAMED_FIELD, + type PreviewedToolCall, +} from './tool-input-streams.js'; // Helper: feed a complete JSON string in one shot. function parseAll(streamingField: string, json: string) { @@ -383,3 +389,55 @@ describe('StreamingToolInputParser', () => { }); }); }); + +// =========================================================================== +// Property: the live preview of a streamed tool call +// =========================================================================== + +// While an agent streams a writeFile / editFile / executeCode call, the preview shows the +// streamed field as far as it has arrived. fast-check generates the calls agents make (source +// text with quotes, backslashes, CRLFs, accented text and emoji), serialized compactly or with +// spaces, with raw or `\uXXXX`-escaped non-ASCII, and cut into provider deltas anywhere -- inside +// an escape, or between a surrogate pair's halves. After every delta the parser must hold exactly +// what that much input determines: no prefix fields until the streamed value opens, then all of +// them; the streamed value decoded up to any escape still cut off, so it only grows and never +// lags; and, at the end, the same input JSON.parse reads. +describe('StreamingToolInputParser property', () => { + it('tracks the decodable prefix of a streamed tool input after every delta', () => { + let surrogateCall: PreviewedToolCall = { + toolName: "writeFile", + input: {filename: "app.ts", content: "label: '😀',\n\\\"end\\\""}, + asciiOnly: true, + spaced: true, + }; + fc.assert(fc.property(previewedToolCall, deltaSizes, (call, sizes) => { + let field = STREAMED_FIELD[call.toolName]; + let {json, valueStart} = serialize(call); + let {[field]: value, ...prefix} = JSON.parse(json); + let parser = new StreamingToolInputParser(field); + let received = 0; + for (let delta of cutDeltas(json, sizes)) { + parser.append(delta); + received += delta.length; + let at = `after ${received} of ${json.length} chars of ${json}`; + expect(parser.hasError, at).toBe(false); + expect(parser.prefixFields, at).toEqual(received >= valueStart ? prefix : null); + // Compared JSON-encoded: the value may end in a lone high surrogate, which in a failure + // message is invalid UTF-8 to the vitest pool's WebSocket, which then drops the report. + expect(JSON.stringify(parser.streamingValue), at) + .toBe(JSON.stringify(decodableValue(json, valueStart, received))); + expect(parser.streamComplete, at).toBe(received >= json.length - 1); + } + expect(parser.prefixFields).toEqual(prefix); + expect(parser.streamingValue).toBe(value); + }), { + ...PROPERTY_RUN_OPTIONS, + numRuns: 200, + examples: [ + // One UTF-16 code unit per delta: every escape and `\ud83d\ude00` pair split everywhere. + [surrogateCall, [1]], + [{...surrogateCall, asciiOnly: false}, [1]], + ], + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/tool-input-streams.ts b/packages/workshop-backend/__tests__/tool-input-streams.ts new file mode 100644 index 0000000000..4f36c5e5dc --- /dev/null +++ b/packages/workshop-backend/__tests__/tool-input-streams.ts @@ -0,0 +1,117 @@ +// Generators and a reference decoder for the property tests of the live code preview +// (streaming-json-parser.test.ts, code-preview.test.ts): the writeFile / editFile / executeCode +// tool inputs an agent streams, serialized as the model providers emit them, and cut into the +// deltas the provider delivers. + +import * as fc from "fast-check"; + +/** A tool call whose input the code preview streams, with the field it streams. */ +export type PreviewedToolCall = { + toolName: "writeFile" | "editFile" | "executeCode"; + /** The tool's arguments, in schema order (the order models emit them); the streamed one is last. */ + input: Record; + /** Escape every non-ASCII code unit as `\uXXXX` (as e.g. Python's `json.dumps` does). */ + asciiOnly: boolean; + /** `", "` and `": "` separators rather than JSON.stringify's compact ones. */ + spaced: boolean; +}; + +/** The input field each tool's preview streams. */ +export const STREAMED_FIELD = {writeFile: "content", editFile: "replacement", executeCode: "code"}; + +// Lines of the source files agents write: quotes, backslashes (escaped quotes, regexes, Windows +// paths, a shell line continuation), a literal `\u` escape in source text, accented and CJK text, +// and astral emoji (surrogate pairs, one with a skin-tone modifier). +const SOURCE_LINES = [ + "export function greet(name) {", + " return `Hello, ${name}!`;", + "}", + 'const quote = "She said \\"hi\\"";', + "const re = /^\\d+(\\.\\d+)?$/;", + "const dir = 'C:\\\\Users\\\\me\\\\';", + "npm run build && \\", + "console.log('caf\\u00e9');", + '

Café ☕ — naïve résumé

', + "// 日本語のコメント", + "label: '😀 done 🎉',", + "👍🏽", + '{"items": [1, 2, 3]}', + "", +]; +const LINE_BREAKS = ["\n", "\n ", "\r\n", "\t", " "]; + +const sourceText = (minLines: number) => + fc.array(fc.tuple(fc.constantFrom(...SOURCE_LINES), fc.constantFrom(...LINE_BREAKS)), + {minLength: minLines, maxLength: 10}) + .map(lines => lines.map(([line, br]) => line + br).join("")); + +const filename = fc.constantFrom("index.html", "app.ts", "src/components/Chart.tsx", + "styles.css", "data/notes 2024.json", "docs/résumé.md"); +// The optional chat binding name of the target workpiece. +const workpiece = fc.constantFrom("app", "todoList", "weather_widget"); + +/** A previewed tool call as an agent issues it. */ +export const previewedToolCall: fc.Arbitrary = fc.record({ + call: fc.oneof( + fc.record({workpiece, filename, content: sourceText(0)}, + {requiredKeys: ["filename", "content"]}) + .map(input => ({toolName: "writeFile" as const, input})), + fc.record({workpiece, filename, textToReplace: sourceText(1), replacement: sourceText(0)}, + {requiredKeys: ["filename", "textToReplace", "replacement"]}) + .map(input => ({toolName: "editFile" as const, input})), + sourceText(1).map(code => ({toolName: "executeCode" as const, input: {code}}))), + asciiOnly: fc.boolean(), + spaced: fc.boolean(), +}).map(({call, asciiOnly, spaced}) => ({...call, asciiOnly, spaced})); + +/** Delta sizes, in UTF-16 code units, cycled over the serialized input. */ +export const deltaSizes = fc.array(fc.integer({min: 1, max: 12}), {minLength: 1, maxLength: 16}); + +/** + * Serializes `call`'s input, returning the JSON text and the index of the first character of the + * streamed string value's body (just past its opening quote). + */ +export function serialize(call: PreviewedToolCall): {json: string, valueStart: number} { + let encode = (s: string) => { + let json = JSON.stringify(s); + return call.asciiOnly + ? json.replace(/[\u0080-\uffff]/g, c => "\\u" + c.charCodeAt(0).toString(16).padStart(4, "0")) + : json; + }; + let entries = Object.entries(call.input) + .map(([key, value]) => encode(key) + (call.spaced ? ": " : ":") + encode(value)); + let json = "{" + entries.join(call.spaced ? ", " : ",") + "}"; + // The streamed value is last: `""}`. + let valueStart = json.length - encode(call.input[STREAMED_FIELD[call.toolName]]).length; + return {json, valueStart}; +} + +/** Cuts `json` into consecutive deltas of the given sizes, cycled. */ +export function cutDeltas(json: string, sizes: number[]): string[] { + let deltas: string[] = []; + for (let pos = 0, i = 0; pos < json.length; i++) { + let size = sizes[i % sizes.length]; + deltas.push(json.slice(pos, pos + size)); + pos += size; + } + return deltas; +} + +/** + * The streamed value decodable from the first `received` characters of `json`: its body so far, + * less an escape sequence still cut off at the end. (A `\uXXXX` half of a surrogate pair decodes + * on its own, so this may end in a lone high surrogate.) + */ +export function decodableValue(json: string, valueStart: number, received: number): string { + let body = json.slice(valueStart, Math.min(received, json.length - 2)); + for (let i = 0; i < body.length; i++) { + if (body[i] !== "\\") continue; + let length = body[i + 1] === "u" ? 6 : 2; + if (i + length > body.length) { + body = body.slice(0, i); + break; + } + i += length - 1; + } + return JSON.parse('"' + body + '"'); +} From ddc8686d6e73edc79511cdda9fefe8c63873b490 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sun, 8 Nov 2026 11:40:26 -0600 Subject: [PATCH 06/12] Property-test turn resume, two-chat changes, scope restarts and unread counts in-isolate - Turn resume: real agent turns stage awaited writes; approvals, rejections, the "always approve" rule, failed applies and workspace restarts are replayed over the session's RPC interface. A suspended turn makes exactly one more model request, when the last write it waits on is approved, and none after a rejection. - Two chats about one gadget: edit, save, discard, ask, accept, update and revert in any order. Each chat's cache and every fold of its log read its own changes on what it last merged, an accept is stale only when the chat changed something and is behind, and it commits exactly the chat's content. - Scope-widening restarts: what an owner does while collaborators have the workspace open, through the owner's client interface. A restart is scheduled exactly when an open session's scope widens (per docs/observers.md), what it gained is blocked until then, and each gadget lists the model's bindings. - Unread counts: two users and the agent in three chats while handoffs are lost, repeated and delayed, and objects reset. Stored rows, queries, summaries and delta-folding subscribers match docs/user-unread-state.md after every step. --- packages/integration-tests/README.md | 5 +- .../__tests__/chat-reroot.test.ts | 319 +++++++++++++-- .../__tests__/durable-decision-resume.test.ts | 271 ++++++++++++- .../__tests__/observer-scope-restart.test.ts | 372 +++++++++++++++++- .../__tests__/unread-state.test.ts | 292 +++++++++++++- 5 files changed, 1213 insertions(+), 46 deletions(-) diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index 1208edabd9..acd466aeb2 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -55,8 +55,9 @@ and shrink attempts are independent. Scenarios a fixed seed might miss are pinne Every run is real RPC, so a property costs hundreds of milliseconds per run and `numRuns` is kept small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside the test timeout (`@gadgets/scripts/property-runs`). workshop-backend's in-isolate properties (sharing, blueprint -releases, streamed tool input) share those options, and its `vitest.config.ts` forwards -`FAST_CHECK_SEED` into workerd. +releases, streamed tool input, decisions on a turn's staged writes, two chats' changes to one +gadget, scope-widening restarts, unread counts) share those options, and its `vitest.config.ts` +forwards `FAST_CHECK_SEED` into workerd. `FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so a pull request's result depends only on its code. Without it, each run draws a fresh seed, but a diff --git a/packages/workshop-backend/__tests__/chat-reroot.test.ts b/packages/workshop-backend/__tests__/chat-reroot.test.ts index 20b04f37b9..f2e81a4bdc 100644 --- a/packages/workshop-backend/__tests__/chat-reroot.test.ts +++ b/packages/workshop-backend/__tests__/chat-reroot.test.ts @@ -7,13 +7,15 @@ // real OverseerImpl, and for the agent's fold the real runAgent with pi's faux provider standing // in for the model. -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import * as fc from "fast-check"; import { env } from "cloudflare:workers"; import { runInDurableObject } from "cloudflare:test"; import { createFauxCore, fauxAssistantMessage, fauxText, fauxToolCall, type Context, type TranscriptContext, } from "@earendil-works/pi-ai"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import { keyString } from "@gadgets/typed-storage"; import type { AiChatAuthorInfo, AiChatMessage, AiChatMessageBody, ChatGadgetPinState, @@ -64,26 +66,26 @@ function head(impl: any): string { return impl.storage.gadgets.get(GADGET).commitId; } -function chatMessages(impl: any): AiChatMessage[] { - return [...impl.storage.chats.list({ prefix: `${keyString(CHAT)}.` })]; +function chatMessages(impl: any, chatId = CHAT): AiChatMessage[] { + return [...impl.storage.chats.list({ prefix: `${keyString(chatId)}.` })]; } -function pin(impl: any): ChatGadgetPinState | undefined { - return impl.storage.chatMeta.get(CHAT).codeBase?.pins +function pin(impl: any, chatId = CHAT): ChatGadgetPinState | undefined { + return impl.storage.chatMeta.get(chatId).codeBase?.pins .find((p: ChatGadgetPinState) => p.gadgetId === GADGET); } -function putMessage(impl: any, body: AiChatMessageBody): number { - let sequence = impl.nextChatSequence(CHAT); +function putMessage(impl: any, body: AiChatMessageBody, chatId = CHAT): number { + let sequence = impl.nextChatSequence(chatId); impl.storage.chats.put( - { chatId: CHAT, sequence, timestamp: impl.getChatTimestamp(), author: USER, ...body }); + { chatId, sequence, timestamp: impl.getChatTimestamp(), author: USER, ...body }); return sequence; } // The gadget's files as the chat holds them, rebuilt from the log rather than from the cache. -async function chatFiles(impl: any): Promise> { - impl.invalidateChatContent(CHAT); - let content = await impl.getCurrentChatContent(CHAT, impl.storage.chatMeta.get(CHAT)); +async function chatFiles(impl: any, chatId = CHAT): Promise> { + impl.invalidateChatContent(chatId); + let content = await impl.getCurrentChatContent(chatId, impl.storage.chatMeta.get(chatId)); return Object.fromEntries(content.get(GADGET) ?? new Map()); } @@ -92,18 +94,29 @@ async function commitTree(impl: any, commitId: string): Promise { } // The user edits the chat's copy of the gadget from `before` to `after`, declaring the pin if -// the gadget has none yet, and the edit is materialized into a message. -async function edit(impl: any, before: Record, after: Record) - : Promise { - let codeBase = impl.storage.chatMeta.get(CHAT).codeBase ?? { generation: 0, revision: 0 }; +// the gadget has none yet. A `late` edit was typed before the editor heard of the accept that +// closed its stream: it is rooted where that stream ended (see ChatCodeBase.prior) and declares +// nothing, the gadget having been pinned there. +async function submitEdit( + impl: any, before: Record, after: Record, chatId = CHAT, + late = false): Promise { + let codeBase = impl.storage.chatMeta.get(chatId).codeBase ?? { generation: 0, revision: 0 }; let content = (files: Record): CodeContent => new Map([[GADGET, new Map(Object.entries(files))]]); - await impl.submitCodeChange(CHAT, { - generation: codeBase.generation, revision: codeBase.revision, + await impl.submitCodeChange(chatId, { + generation: late ? codeBase.prior.generation : codeBase.generation, + revision: late ? codeBase.prior.finalRevision : codeBase.revision, clientId: `cli-${codeBase.generation}`, seq: codeBase.revision + 1, - ...(pin(impl) === undefined ? { pins: [{ gadgetId: GADGET, baseCommit: head(impl) }] } : {}), + ...(!late && pin(impl, chatId) === undefined + ? { pins: [{ gadgetId: GADGET, baseCommit: head(impl) }] } : {}), change: diffFiles(content(before), content(after)), }, USER, "alice-user-do"); +} + +// The same, with the edit materialized into a message. +async function edit(impl: any, before: Record, after: Record) + : Promise { + await submitEdit(impl, before, after); return impl.materializeChatChanges(CHAT)!.sequence; } @@ -167,9 +180,9 @@ function compact(impl: any, compactedTo: number) { // Runs one agent turn, prompted by a new user message, in which the model makes `calls` and // then stops. Returns the text of each call's result. -async function agentTurn(impl: any, calls: ReturnType[]) +async function agentTurn(impl: any, calls: ReturnType[], chatId = CHAT) : Promise { - putMessage(impl, { type: "message", message: "Go." }); + putMessage(impl, { type: "message", message: "Go." }, chatId); let faux = createFauxCore({ models: [{ id: "faux-model" }] }); let contexts: Context[] = []; faux.setResponses([ @@ -179,7 +192,7 @@ async function agentTurn(impl: any, calls: ReturnType[]) contexts.push({ messages: structuredClone(context.messages) }); return step; })); - await runAgent(impl, { model: faux.getModel(), stream: faux.stream }, CHAT, + await runAgent(impl, { model: faux.getModel(), stream: faux.stream }, chatId, { type: "agent", id: "faux-model", name: "Faux" }, new AbortController().signal, USER, { provider: "cloudflare", model: "faux-model", apiToken: "" } as any); return contexts[1].messages.flatMap(message => message.role === "toolResult" @@ -188,41 +201,45 @@ async function agentTurn(impl: any, calls: ReturnType[]) } // The gadget's files as the agent's replay of the log reads them. -async function agentReads(impl: any, files: Record) +async function agentReads(impl: any, files: Record, chatId = CHAT) : Promise> { let names = Object.keys(files); let texts = await agentTurn(impl, names.map(filename => - fauxToolCall("readFile", { workpiece: "APP", filename }))); + fauxToolCall("readFile", { workpiece: "APP", filename })), chatId); return Object.fromEntries(names.map((name, i) => [name, texts[i]])); } -// Every fold of the chat's log, and its content cache, agree that the gadget holds `expected`. -async function expectEveryFold(impl: any, expected: Record): Promise { - expect(Object.fromEntries((await impl.buildChatContent(CHAT)).get(GADGET))).toEqual(expected); - expect(await chatFiles(impl)).toEqual(expected); +// Every fold of the chat's log agrees that the gadget holds `expected`. +async function expectLogFolds(impl: any, expected: Record, chatId = CHAT) + : Promise { + expect(Object.fromEntries((await impl.buildChatContent(chatId)).get(GADGET))).toEqual(expected); // The epoch's composed change, as a client folds it, over the live pin's tree. - let messages = chatMessages(impl); - let epoch = impl.storage.chatMeta.get(CHAT).codeBase.epoch ?? -1; + let messages = chatMessages(impl, chatId); + let epoch = impl.storage.chatMeta.get(chatId).codeBase.epoch ?? -1; let reverted = new Set(messages.filter(msg => msg.type === "revert") .flatMap(revert => messages.filter(msg => msg.sequence >= (revert as any).revertFrom && msg.sequence < revert.sequence)) .map(msg => msg.sequence)); let composed = composeEpochChanges(messages.filter(msg => msg.type === "changes" && msg.sequence >= epoch && !reverted.has(msg.sequence)) as any); - let base = await commitTree(impl, pin(impl)!.baseCommit); + let base = await commitTree(impl, pin(impl, chatId)!.baseCommit); let folded = composed === undefined ? base : applyCodeChange(base, composed); expect(Object.fromEntries(folded.get(GADGET)!)).toEqual(expected); // A checkpoint of the whole log replays to it: its pins' trees, then its change. let state = buildCompactionState(messages, messages.at(-1)!.sequence + 1, [], undefined); let checkpointPin = state.pins!.find(p => p.gadgetId === GADGET)!; - expect(checkpointPin.baseCommit).toBe(pin(impl)!.baseCommit); + expect(checkpointPin.baseCommit).toBe(pin(impl, chatId)!.baseCommit); let replayed = await commitTree(impl, checkpointPin.baseCommit); if (state.proposedChange) replayed = applyCodeChange(replayed, state.proposedChange); expect(Object.fromEntries(replayed.get(GADGET)!)).toEqual(expected); +} - // And the agent's replay reads it. +// So do its content cache and the agent's replay. +async function expectEveryFold(impl: any, expected: Record): Promise { + await expectLogFolds(impl, expected); + expect(await chatFiles(impl)).toEqual(expected); expect(await agentReads(impl, expected)).toEqual(expected); } @@ -653,3 +670,241 @@ describe("what the agent is told of an update from mainline", () => { .rejects.toThrow(/update from mainline/); })); }); + +// --------------------------------------------------------------------------------------------- +// Property: two chats about one gadget. fast-check generates what their users do, in any order +// -- edit, save the draft or discard it, ask the agent, accept, take the update that a stale +// accept offers, revert to a saved batch or discard everything pending -- and replays it against +// the real OverseerImpl. After every step each chat's files are read from its content cache and +// from every fold of its log, and the gadget's head is read too; the agent's replay reads them +// whenever it is asked. An edit silently lost, or one a revert brought back, shows as a read that +// differs from the model. +// +// The model keeps mainline as the versions accepts made of it and, for a chat that has changed +// something, a copy of the files with the version that copy last merged, plus what each pending +// batch of changes replaced. Its rules are the documented ones. A chat with no copy reads the +// head. A revert puts back what its batch replaced, with whatever that had merged. An accept is +// stale when the copy changed something and the version it merged is no longer the head; +// otherwise it commits the copy, if that changed anything, and the chat has no copy again. An +// update keeps the chat's own changes on mainline's files: each chat edits only its own file, so +// what the merge gives needs no oracle. +// +// Steps are ones the Workshop offers: saving or discarding needs a draft, accepting or discarding +// everything needs pending changes, a revert names a batch still pending, and an update follows +// the stale accept that offers it. Scenarios a seed might miss are pinned as `examples`. + +type Files = Record; +// A chat's copy of the gadget, and the mainline version (an index) it last merged. +type Copy = { files: Files, merged: number }; +type ModelChat = { + id: number, + own: string, // the one file it edits + copy?: Copy, + // What each pending batch replaced, oldest first. A draft is last, with no sequence yet. + pending: { sequence?: number, replaced?: Copy }[], + batches: number, // the "changes" messages its log should hold +}; + +// `at` and `to` pick, modulo the count, a line of the chat's file and a revert target. A `late` +// edit that directly follows its chat's accept is sent as typed before the accept landed. +type Step = + | { kind: "edit", chat: number, at: number, remove: boolean, late: boolean } + | { kind: "save" | "discard" | "ask", chat: number } + | { kind: "accept", chat: number, update: boolean } + | { kind: "revert", chat: number, to: number }; + +function replayChats(steps: Step[]): Promise { + return withImpl(async impl => { + let mainline: { files: Files, commit: string }[] = + [{ files: H0_FILES, commit: await commitFiles(impl, H0_FILES) }]; + impl.storage.gadgets.put({ + type: "gadget", id: GADGET, title: "App", created: new Date(0), bindingName: "APP", + bindings: {}, commitId: mainline[0].commit, + }); + let chats = Object.keys(H0_FILES) + .map((own, i): ModelChat => ({ id: CHAT + i, own, pending: [], batches: 0 })); + for (let { id } of chats) { + impl.storage.chatMeta.put( + { id, title: `Chat ${id}`, started: new Date(0), lastActive: new Date(id) }); + } + let latest = () => mainline.length - 1; + let draft = (chat: ModelChat) => + chat.pending.at(-1)?.sequence === undefined ? chat.pending.at(-1) : undefined; + // The step just taken saved the chat's draft, if it had one, as its newest batch. + let saved = (chat: ModelChat) => { + let batch = draft(chat); + if (batch === undefined) return; + chat.batches++; + batch.sequence = + chatMessages(impl, chat.id).findLast(msg => msg.type === "changes")!.sequence; + }; + + // The chat whose accept, in the step before, closed its stream with the content carried over. + let carried: ModelChat | undefined; + for (let [index, step] of steps.entries()) { + let chat = chats[step.chat], label = `step ${index}, ${step.kind} in chat ${chat.id}`; + let handedOff = carried === chat; + carried = undefined; + switch (step.kind) { + case "edit": { + let files = chat.copy?.files ?? mainline[latest()].files; + let lines = files[chat.own].split("\n").slice(0, -1); + if (step.remove && lines.length > 1) lines.splice(step.at % lines.length, 1); + else lines.splice(step.at % (lines.length + 1), 0, `${chat.own} ${index}`); + let edited = { ...files, [chat.own]: lines.map(line => `${line}\n`).join("") }; + await submitEdit(impl, files, edited, chat.id, step.late && handedOff); + if (draft(chat) === undefined) chat.pending.push({ replaced: chat.copy }); + chat.copy = { files: edited, merged: chat.copy?.merged ?? latest() }; + break; + } + case "save": + if (draft(chat) === undefined) break; + impl.materializeChatChanges(chat.id); + saved(chat); + break; + case "discard": + if (draft(chat) === undefined) break; + impl.discardChatDraftChanges(chat.id); + chat.copy = chat.pending.pop()!.replaced; + break; + case "ask": { + // Sending a message saves the draft; the agent then reads each file. + impl.materializeChatChanges(chat.id); + saved(chat); + let files = chat.copy?.files ?? mainline[latest()].files; + expect(await agentReads(impl, files, chat.id), `${label}: the agent's reads`) + .toEqual(files); + break; + } + case "accept": { + if (chat.copy === undefined) break; + let { files, merged } = chat.copy; + let changed = files[chat.own] !== mainline[merged].files[chat.own]; + let stale = changed && merged !== latest(); + expect(await impl.mergeChanges(chat.id, USER_META, "alice-user-do"), label) + .toEqual({ outcome: stale ? "stale" : "merged" }); + saved(chat); + if (!stale) { + // A copy that changed nothing is dropped where it stood. If that was behind the + // head, the chat's content does not carry over, and its editors are told so. + let carries = changed || merged === latest(); + expect(impl.storage.chatMeta.get(chat.id).codeBase.prior.discontinuousGadgets, + `${label}: what did not carry over`).toEqual(carries ? [] : [GADGET]); + if (carries) carried = chat; + if (changed) { + let before = mainline[latest()].commit; + expect(head(impl) !== before && impl.gitCache.isAncestor(before, head(impl)), + `${label}: the head fast-forwards`).toBe(true); + mainline.push({ files, commit: head(impl) }); + } + chat.copy = undefined; + chat.pending = []; + } else if (step.update) { + expect(await impl.updateChatFromMainline(chat.id, USER), `${label}: the update`) + .toEqual({ conflictPaths: [] }); + chat.pending.push({ replaced: chat.copy }); + saved(chat); + chat.copy = { + files: { ...mainline[latest()].files, [chat.own]: files[chat.own] }, + merged: latest(), + }; + } + break; + } + case "revert": { + if (chat.pending.length === 0) break; + // Sequence 0 is the banner's "discard pending changes"; the rest name a saved batch. + let targets = [0, ...chat.pending.flatMap(batch => batch.sequence ?? [])]; + let to = step.to % targets.length; + await impl.revertChanges(chat.id, targets[to], USER); + let kept = Math.max(to - 1, 0); + chat.copy = chat.pending[kept].replaced; + chat.pending.length = kept; + break; + } + } + + expect(head(impl), `${label}: the head`).toBe(mainline[latest()].commit); + expect(Object.fromEntries(impl.gitCache.readCommitFiles(head(impl))), + `${label}: the head's files`).toEqual(mainline[latest()].files); + for (let each of chats) { + let where = `${label}: chat ${each.id}`; + expect(chatMessages(impl, each.id).filter(msg => msg.type === "changes").length, + `${where}'s batches`).toBe(each.batches); + expect(pin(impl, each.id)?.mergedCommit, `${where}'s merged commit`) + .toBe(each.copy && mainline[each.copy.merged].commit); + // With its draft: the content cache as the step left it, and as rebuilt from the log. + let cached = (await impl.getCurrentChatContent( + each.id, impl.storage.chatMeta.get(each.id))).get(GADGET); + expect(Object.fromEntries(cached ?? []), `${where}'s cached files`) + .toEqual(each.copy?.files ?? {}); + expect(await chatFiles(impl, each.id), `${where}'s files`) + .toEqual(each.copy?.files ?? {}); + // Without it: every fold of the log. + let logged = draft(each) ? draft(each)!.replaced : each.copy; + if (logged) await expectLogFolds(impl, logged.files, each.id); + else expect((await impl.buildChatContent(each.id)).has(GADGET), where).toBe(false); + } + } + }); +} + +describe("two chats about one gadget", () => { + const chat = fc.constantFrom(0, 1); + const pick = fc.nat({ max: 7 }); + const step: fc.Arbitrary = fc.oneof( + { arbitrary: fc.record({ kind: fc.constant("edit" as const), chat, at: pick, + remove: fc.boolean(), late: fc.boolean() }), weight: 6 }, + { arbitrary: fc.record({ kind: fc.constant("save" as const), chat }), weight: 3 }, + { arbitrary: fc.record({ kind: fc.constantFrom("discard" as const, "ask" as const), chat }), + weight: 2 }, + { arbitrary: fc.record({ kind: fc.constant("accept" as const), chat, update: fc.boolean() }), + weight: 3 }, + { arbitrary: fc.record({ kind: fc.constant("revert" as const), chat, to: pick }), weight: 1 }, + ); + + const edit = (chat: number, at = 0, remove = false, late = false): Step => + ({ kind: "edit", chat, at, remove, late }); + const save = (chat: number): Step => ({ kind: "save", chat }); + const accept = (chat: number, update = true): Step => ({ kind: "accept", chat, update }); + const revert = (chat: number, to: number): Step => ({ kind: "revert", chat, to }); + // Chat 1 saved an edit and chat 2's accept then moved the head: a seed takes a while to get + // here, so two runs in three open with it, and half of those with chat 1 updated as well. + const overtaken = [edit(0), save(0), edit(1), accept(1)]; + const opening = fc.constantFrom([], overtaken, [...overtaken, accept(0)]); + + const examples: [Step[]][] = [ + // Updated, chat 1 edits on and reverts the update: stale again, holding the edit from before + // it. It declines the next update, then takes it and accepts. + [[ + ...overtaken, accept(0), edit(0, 1), { kind: "ask", chat: 0 }, revert(0, 2), + accept(0, false), accept(0), accept(0), { kind: "ask", chat: 1 }, + ]], + // A chat that undid its only edit accepts though the head moved: nothing is committed, and + // it reads a head it never merged. Its next edit starts from there. + [[edit(0), edit(1), accept(1), edit(0, 0, true), accept(0), edit(0), accept(0)]], + // Typing through an accept: the edit lands on what was accepted, in the next epoch. The + // other chat, now two versions behind, is stale until it updates. + [[ + edit(1), edit(0), accept(0), edit(0, 1, false, true), accept(0), accept(1), accept(1), + edit(1, 0, false, true), { kind: "discard", chat: 1 }, + ]], + // A revert takes the draft with it, back past an update or to no copy at all, and the edit + // after that starts from the head as it then is. + [[ + ...overtaken, accept(0), edit(0, 1), save(0), edit(0, 2), revert(0, 2), accept(0, false), + edit(0), revert(0, 0), edit(0), accept(0), + ]], + ]; + + it("each chat reads its own changes on what it last merged, and an accept commits them", + async () => { + // An agent turn logs a screenful, and hundreds would bury a failure's report. + using _debug = vi.spyOn(console, "debug").mockImplementation(() => {}); + using _warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const steps = fc.tuple(opening, fc.array(step, { maxLength: 24, size: "max" })) + .map(([opened, rest]) => [...opened, ...rest]); + await fc.assert(fc.asyncProperty(steps, replayChats), + { ...PROPERTY_RUN_OPTIONS, numRuns: 100, examples }); + }, 120_000); // a failure's shrink may run to PROPERTY_RUN_OPTIONS' time limit +}); diff --git a/packages/workshop-backend/__tests__/durable-decision-resume.test.ts b/packages/workshop-backend/__tests__/durable-decision-resume.test.ts index d944c6f426..8d20d9f468 100644 --- a/packages/workshop-backend/__tests__/durable-decision-resume.test.ts +++ b/packages/workshop-backend/__tests__/durable-decision-resume.test.ts @@ -1,7 +1,9 @@ -import { env, type RpcStub } from "cloudflare:workers"; +import { env, RpcStub } from "cloudflare:workers"; import { evictDurableObject, runInDurableObject } from "cloudflare:test"; +import * as fc from "fast-check"; import { afterEach, describe, expect, it, vi } from "vitest"; -import type { AiChatAuthorInfo, UserNotification } from "@gadgets/workshop-shared/api"; +import type { ActionState, AiChatAuthorInfo, UserNotification } from "@gadgets/workshop-shared/api"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; import type { OverseerDurableObject } from "../src/overseer"; import { UserDurableObject } from "../src/user"; import { OVERSEER_STORAGE_VERSION } from "../src/storage-schema/overseer-migrations"; @@ -27,21 +29,26 @@ declare module "cloudflare:workers" { afterEach(() => { vi.restoreAllMocks(); vi.unstubAllGlobals(); }); -function inference(gate?: Promise) { +// Answers each model request with "Done.", or, while `stages()` holds, with an executeCode call. +function inference(gate?: Promise, stages = () => false) { let count = 0; const fetch = globalThis.fetch; vi.stubGlobal("fetch", (input: RequestInfo | URL, init?: RequestInit) => { const url = new URL(input instanceof Request ? input.url : input); if (url.origin !== MODEL.config.apiUrl) return fetch(input, init); count++; + const call = stages(); const events = [ {type: "message_start", message: {id: "resume", type: "message", role: "assistant", model: MODEL.profile.id, content: [], stop_reason: null, stop_sequence: null, usage: {input_tokens: 1, output_tokens: 1}}}, - {type: "content_block_start", index: 0, content_block: {type: "text", text: ""}}, - {type: "content_block_delta", index: 0, delta: {type: "text_delta", text: "Done."}}, + {type: "content_block_start", index: 0, content_block: call + ? {type: "tool_use", id: `stage-${count}`, name: "executeCode", input: {}} : {type: "text", text: ""}}, + {type: "content_block_delta", index: 0, delta: call + ? {type: "input_json_delta", partial_json: `{"code": "stage"}`} : {type: "text_delta", text: "Done."}}, {type: "content_block_stop", index: 0}, - {type: "message_delta", delta: {stop_reason: "end_turn", stop_sequence: null}, usage: {output_tokens: 1}}, + {type: "message_delta", delta: {stop_reason: call ? "tool_use" : "end_turn", stop_sequence: null}, + usage: {output_tokens: 1}}, {type: "message_stop"}, ]; const response = new Response(events.map(e => @@ -75,7 +82,7 @@ function decide(impl: Impl, kind: Kind) { } } -async function scenario(kind: Kind = "connection", active = false, initiator: AiChatAuthorInfo = USER) { +async function scenario(kind: Kind | null = "connection", active = false, initiator: AiChatAuthorInfo = USER) { const stub = env.TEST_OVERSEER.getByName(crypto.randomUUID()); const user = env.TEST_USER.getByName(crypto.randomUUID()); await runInDurableObject(user, instance => { @@ -95,6 +102,7 @@ async function scenario(kind: Kind = "connection", active = false, initiator: Ai impl.storage.chatMeta.put({id: CHAT, title: "Resume", started: timestamp, lastActive: timestamp, ...(active ? {activeAgent: MODEL.profile} : {})}); impl.storage.chatContext.put({chatId: CHAT, bindings: {}, alwaysAvailableCapsuleIds: []}); + if (!kind) return; // A new chat. impl.addChatMessages(CHAT, USER, [{type: "message", message: "Do the work"}]); request(impl, kind); impl.storage.activeAgents.put({chatId: CHAT, modelId: MODEL.profile.id, @@ -111,6 +119,14 @@ async function reset(stub: DurableObjectStub) { })).rejects.toThrow("Injected decision recovery reset"); } +// A gatekeeper class cannot be minted in a test: record connection 31 with one the workspace +// never instantiates, and answer what a decision asks of its facet. +function fakeGatekeeper(impl: Impl, applyAction: () => Promise = async () => {}) { + impl.storage.gatekeepers.put({id: 31, class: {} as any}); + vi.spyOn(impl, "getGatekeeperFacet").mockResolvedValue({applyAction, rejectAction: async () => {}} as + unknown as Awaited>); +} + function decisionClient(impl: Impl, profile = USER, overrides: object = {}) { const userId = profile.id === USER.id ? impl.ownerId : env.TEST_USER.idFromName(profile.id).toString(); return openFakeOverseer(impl.storage, {ownerId: impl.ownerId, userId, impl: { @@ -229,7 +245,7 @@ describe("decision handoff using the existing execution row", () => { impl.storage.activeAgents.delete(CHAT); modelLookup(impl, async () => ({profile: USER, aiModel: MODEL})); // Only the rule-setting path reads this record; applying the external effect is stubbed below. - impl.storage.gatekeepers.put({id: 31, class: {} as any}); + fakeGatekeeper(impl); const action = impl.storage.actions.get(7)!; if (action.type !== "action") throw new Error("Missing action"); const actionKind = {tag: "edit", label: "Edits"}; @@ -731,8 +747,7 @@ describe("decision handoff using the existing execution row", () => { const gate = Promise.withResolvers(); modelLookup(impl, async () => { await gate.promise; return {profile: USER, aiModel: MODEL}; }); const apply = vi.fn(async () => {}); - vi.spyOn(impl, "getGatekeeperFacet").mockResolvedValue({applyAction: apply} as unknown as - Awaited>); + fakeGatekeeper(impl, apply); const action = impl.storage.actions.get(7)!; if (action.type !== "action") throw new Error("Missing action"); await impl.applyPendingAction(action, USER, auto); @@ -746,3 +761,239 @@ describe("decision handoff using the existing execution row", () => { expect(requests()).toBe(1); }); }); + +// --------------------------------------------------------------------------------------------- +// Property: a turn that staged writes carries on exactly once. fast-check generates what a user +// does around a chat whose agent writes through one connection: prompt it (the turn stages one to +// three awaited writes, each auto-approvable or not), approve or reject a pending write, in any +// order, and switch the connection's "always approve" rule on or off, in sessions with a workspace +// restart between them, while the provider may fail an apply. Each step goes through the session's +// RPC interface. After every step the model requests made, the "approved" notes sent to the agent +// and every write's state are checked against a model: a hung chat is a request that never came, +// a double resume one too many. +// +// The model keeps the writes in submission order, the rule, and the writes the latest turn waits +// on. Writes apply as auto-approval.ts documents: an approval applies its own write; after it, and +// after a rejection, the rule being switched on, or an auto-approvable write being staged under +// it, the pending writes the rule covers apply in order, up to the first that needs a person or +// fails. A turn makes one more request once everything it staged is approved: within its step and +// without a note if that all auto-applies, whether before the step ends or after; else as a resume, +// when the last approval lands. A rejected write ends the wait for good, as does a newer prompt. +// +// Left out: concurrent decisions (pinned in integration-tests' workshop-agent-actions.test.ts), +// and an apply that fails while the step that staged its write still runs, whose turn carries on +// regardless (see #drainAutoApprovalsAndResume). Scenarios a seed might miss are pinned as +// `examples`. + +const EDITS = {tag: "edit", label: "Edits"}; + +// `writes` holds each staged write's auto-approval verdict. `late` lands the auto-applies after +// the step that staged them has ended, rather than while its tool still runs. `pick` chooses among +// the pending writes, oldest first, modulo their count. `fail` is which of the step's apply +// attempts the provider fails, from 1 (0: none). +type Step = + | {kind: "prompt"; writes: boolean[]; late: boolean} + | {kind: "approve" | "reject"; pick: number; fail: number} + | {kind: "rule"; on: boolean; fail: number}; + +class ResumeModel { + states: ActionState[] = []; + auto: boolean[] = []; + rule = false; + /** The writes the latest turn is suspended on. */ + waiting: number[] = []; + requests = 0; + nudges = 0; + fail = 0; + + #apply(i: number): boolean { + if (--this.fail === 0) return false; + this.states[i] = "approved"; + return true; + } + + #drain() { + for (const [i, state] of this.states.entries()) { + if (state === "pending" && !(this.rule && this.auto[i] && this.#apply(i))) return; + } + } + + #carryOn(resumed: boolean) { + if (this.waiting.some(i => this.states[i] === "rejected")) { + this.waiting = []; + } else if (this.waiting.length && this.waiting.every(i => this.states[i] === "approved")) { + this.waiting = []; + this.requests++; + if (resumed) this.nudges++; + } + } + + prompt(writes: boolean[]) { + this.requests++; // The step that stages the writes. + this.waiting = writes.map((_, i) => this.states.length + i); + for (const auto of writes) { + this.states.push("pending"); + this.auto.push(auto); + if (this.rule && auto) this.#drain(); + } + this.#carryOn(false); + } + + /** False when the provider failed the approval, which leaves the write pending. */ + approve(i: number): boolean { + const applied = this.#apply(i); + if (applied) this.#drain(); + this.#carryOn(true); + return applied; + } + + reject(i: number) { + this.states[i] = "rejected"; + this.#drain(); + this.#carryOn(true); + } + + setRule(on: boolean) { + this.rule = on; + if (on) this.#drain(); + this.#carryOn(true); + } +} + +// Replays a run, one session after another, on a fresh workspace and account. +async function replayDecisions(sessions: Step[][]) { + vi.unstubAllGlobals(); // The previous run's provider. + const s = await scenario(null), model = new ResumeModel(); + // The writes the next executeCode call stages. Until it has, the provider answers with that call. + let staged: boolean[] | undefined; + let landing: PromiseWithResolvers | undefined, fail = 0, at = 0; + const requests = inference(undefined, () => staged !== undefined); + // A decision finishes in the background (the drain, and the turn it resumes), so a check first + // waits for everything the workspace has started. + const background: Promise[] = []; + const check = (label: string) => s.visit(async impl => { + await impl.waitForAllAgentsToComplete(); // A late apply lands once its step is over. + landing?.resolve(); + landing = undefined; + do { + await Promise.allSettled(background.splice(0)); + await impl.waitForAllAgentsToComplete(); + } while (background.length); + expect({ + states: [...impl.storage.actions.list()].map(action => action.state), + requests: requests(), + nudges: [...impl.storage.chats.list()].filter(m => m.type === "agentNudge").length, + busy: impl.storage.chatMeta.get(CHAT)?.activeAgent !== undefined, + }, label).toEqual({states: model.states, requests: model.requests, nudges: model.nudges, busy: false}); + }); + + for (const [session, steps] of sessions.entries()) { + if (session) await reset(s.stub); + await s.visit(impl => { + const waitUntil = impl.ctx.waitUntil.bind(impl.ctx); + vi.spyOn(impl.ctx, "waitUntil").mockImplementation(task => { background.push(task); waitUntil(task); }); + fakeGatekeeper(impl, async () => { + await landing?.promise; + if (--fail === 0) throw new Error("The provider failed."); + }); + // Stands in for the sandbox and the connection's session, which stage through submitAction. + vi.spyOn(impl, "executeCodeMode").mockImplementation(async chatId => { + const writes = staged!.entries(), started = background.length; + staged = undefined; + for (const [action, autoApprovable] of writes) { + await impl.submitAction(31, action, {title: `Write ${action}`, description: "", + implementsRevert: false, awaitDecision: true, autoApprovable, actionKind: EDITS}, + {from: "agent", chatId}); + } + if (!landing) await Promise.allSettled(background.slice(started)); + return "Staged."; + }); + }); + using closed = new RpcStub(() => {}); + using client = await env.TEST_OVERSEER.get(s.stub.id).open(s.user.id.toString(), USER.id, closed); + await check(`session ${session} opens`); + for (const step of steps) { + const label = `step ${at++} ${JSON.stringify(step)}`; + const pending = model.states.flatMap((state, i) => state === "pending" ? [i] : []); + model.fail = fail = step.kind === "prompt" ? 0 : step.fail; + if (step.kind === "prompt") { + staged = step.writes; + if (step.late) landing = Promise.withResolvers(); + await client.sendChatMessage(CHAT, "Write it.", MODEL.profile.id); + model.prompt(step.writes); + } else if (step.kind === "rule") { + await (step.on ? client.setAutoApprovedActionKind(31, EDITS) + : client.removeAutoApprovedActionKind(31, EDITS.tag)); + model.setRule(step.on); + } else if (pending.length) { + const nth = step.pick % pending.length, write = pending[nth]; + // The ledger lists the newest first, under ids of its own. + const {id} = (await client.listActions({filter: "pending"})).entries.toReversed()[nth]; + if (step.kind === "approve") { + expect(await client.approveAction(id).then(() => true, () => false), `${label}: applied`) + .toBe(model.approve(write)); + } else { + await client.rejectAction(id); + model.reject(write); + } + } + await check(label); + } + await s.visit(impl => impl.ctx.storage.sync()); // What a restart must not lose. + } +} + +describe("decisions on a turn's staged writes, for generated sessions", () => { + const RULE_ON: Step = {kind: "rule", on: true, fail: 0}; + const fail = fc.constantFrom(0, 0, 0, 0, 1, 2); + const pick = fc.nat({max: 5}); + const prompt = fc.record({kind: fc.constant("prompt" as const), + writes: fc.array(fc.boolean(), {minLength: 1, maxLength: 3}), late: fc.boolean()}); + // Mostly approvals, so that waits often run to their resume. + const step: fc.Arbitrary = fc.oneof( + {weight: 8, arbitrary: fc.record({kind: fc.constant("approve" as const), pick, fail})}, + {weight: 1, arbitrary: fc.record({kind: fc.constant("reject" as const), pick, fail})}, + {weight: 3, arbitrary: fc.record({kind: fc.constant("rule" as const), + on: fc.constantFrom(true, true, false), fail})}, + {weight: 2, arbitrary: prompt}, + ); + // A run opens on a prompt, as often as not with the rule already on, so that turns whose writes + // all auto-apply are common. The workspace restarts between a run's sessions. + const sessions = fc.tuple(fc.boolean(), prompt, + fc.array(fc.array(step, {maxLength: 6}), {minLength: 1, maxLength: 3})) + .map(([ruled, first, [session, ...later]]): Step[][] => + [[...ruled ? [RULE_ON] : [], first, ...session], ...later]); + + const examples: [Step[][]][] = [ + // "Always approve" applies the first of two held writes and fails on the second, which the + // user approves after a restart: one resume, at the second. + [[ + [{kind: "prompt", writes: [true, true], late: false}, {kind: "rule", on: true, fail: 2}], + [{kind: "approve", pick: 0, fail: 0}], + ]], + // A newer turn's auto-approvable write queues behind an older turn's held one, so that turn + // suspends too. Rejecting the older write applies the newer and resumes its turn. + [[[ + {kind: "prompt", writes: [false], late: false}, RULE_ON, + {kind: "prompt", writes: [true], late: true}, {kind: "reject", pick: 0, fail: 0}, + ]]], + // A rejection ends the wait: after a restart, neither approving another of the turn's writes, + // at the second attempt, nor always-approving the last resumes it. + [[ + [{kind: "prompt", writes: [false, true, true], late: false}, {kind: "reject", pick: 0, fail: 0}], + [{kind: "approve", pick: 0, fail: 1}, {kind: "approve", pick: 0, fail: 0}, RULE_ON], + ]], + // Writes that all auto-apply after their step ended never suspended it. Of two later turns, + // approving the newer one's write resumes it, and then the older one's resumes nothing. + [[[ + RULE_ON, {kind: "prompt", writes: [true, true], late: true}, + {kind: "prompt", writes: [false], late: false}, {kind: "prompt", writes: [false], late: false}, + {kind: "approve", pick: 1, fail: 0}, {kind: "approve", pick: 0, fail: 0}, + ]]], + ]; + + it("resumes a suspended turn once, when the last write it waits on is approved", async () => { + await fc.assert(fc.asyncProperty(sessions, replayDecisions), + {...PROPERTY_RUN_OPTIONS, numRuns: 50, examples}); + }, 120_000); +}); diff --git a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts index 01df2aa186..58736567e5 100644 --- a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts +++ b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts @@ -10,9 +10,11 @@ // replaced with a recorder: a real ctx.abort() would kill the test DO. import { describe, expect, it } from "vitest"; +import * as fc from "fast-check"; import { env, RpcStub as NativeRpcStub } from "cloudflare:workers"; import { runInDurableObject } from "cloudflare:test"; -import type { AiChatAuthorInfo } from "@gadgets/workshop-shared/api"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; +import type { AiChatAuthorInfo, Overseer, WorkpieceId } from "@gadgets/workshop-shared/api"; import type { OverseerDurableObject } from "../src/overseer.js"; import type { SeedBindingInfo } from "../src/agent.js"; import { openFakeOverseer } from "./fixtures.js"; @@ -1414,3 +1416,371 @@ describe("ambient resources a chat gains", () => { expect(impl.getChatAgentContext(2).bindings).toEqual({}); })); }); + +// --------------------------------------------------------------------------------------------- +// Property: what an owner does to a workspace while collaborators have it open. fast-check +// generates a sequence of steps -- create a connection (one that needs an account, an AI model, or +// an agent spawner over existing workpieces), remove one, bind one into a gadget from the +// Connections tab (for good, or as the open chat's proposal), rename or remove a binding the tab +// lists, accept a chat's changes, a gadget binding a hook and the owner enabling, disabling or +// deleting it, a collaborator opening or closing the workspace ("build") or one gadget ("use") -- +// and replays it through the owner's real client interface. After every step: +// - each gadget lists the model's bindings, with no chat open and with each, and opening the +// workspace or a gadget demands the model's scope; +// - a restart was scheduled exactly if a session open across the step gained scope; +// - if one was, nothing a session gained is reachable through connections still usable; if none +// was, every connection is usable. +// +// The model keeps only what the steps did, and takes scope from docs/observers.md ("Restarting +// when verification scope widens") rather than from #gadgetReach. A "build" session answers for +// every connection that needs an account. A "use" session answers for what its gadget reaches: +// what the gadget binds, the connection of each enabled hook that wakes it, and whatever an agent +// spawner so reached binds in turn, other gadgets included. A chat's proposal reaches nothing +// until accepted. A widening that goes unnoticed leaves a collaborator holding access nobody +// verified them for; a restart for nothing disconnects everyone. +// +// A restart destroys the workspace object, and with it the sessions and the blocks on what they +// gained. The recorder cannot, so the replay carries on in a fresh workspace that the same steps +// rebuild with nobody connected. Scenarios a seed might miss are pinned as `examples`. + +const GADGETS: WorkpieceId[] = [100, 101]; +const CHATS = [1, 2]; +const MODEL_SPEC = { type: "aiModel", modelId: "m", provider: "anthropic", modelName: "claude" }; + +// `gadget` indexes GADGETS; `chat` is 0 with no chat open, else an index into CHATS plus one; +// `holds` is a gadget's index, or GADGETS.length for the workspace. Every other number picks, +// modulo the count, from what exists when the step runs, in creation order: `connection` among +// the connections (a hook's, among those that need an account), `env` among the gadgets and then +// the connections, `edge` among the bindings the tab lists, `hook` and `session` among those. +type Step = + | { kind: "connect"; as: "account" | "model" } + | { kind: "connect"; as: "spawner"; env: number[] } + | { kind: "remove"; connection: number } + | { kind: "bind"; gadget: number; connection: number; chat: number } + | { kind: "unbind" | "rename"; gadget: number; edge: number; chat: number } + | { kind: "accept"; chat: number } + | { kind: "hook"; connection: number; gadget: number } + | { kind: "enableHook" | "disableHook" | "deleteHook"; hook: number } + | { kind: "join"; holds: number } + | { kind: "leave"; session: number }; + +// One binding in the model; `chat` while it is still that chat's proposal. +type Edge = { name: string; target: WorkpieceId; chat?: number }; + +class ScopeModel { + connections = new Map(); + /** What each gadget and each agent spawner binds. */ + edges = new Map(GADGETS.map(id => [id, []])); + hooks = new Map(); + /** Open collaborator sessions, by what each holds: the workspace, or one gadget. */ + sessions: { holds: "build" | WorkpieceId; leave: () => void }[] = []; + + accounts(): WorkpieceId[] { + return [...this.connections].filter(([, kind]) => kind === "account").map(([id]) => id); + } + + /** The gadget's bindings as the Connections tab lists them with `chat` open. */ + listed(gadget: WorkpieceId, chat?: number): Edge[] { + return this.edges.get(gadget)!.filter(edge => edge.chat === undefined || edge.chat === chat); + } + + /** + * What a session holding `holds` answers for: connections that need an account and, for a + * gadget, the gadgets it reaches. `usable` leaves out connections that refuse every caller. + */ + scope(holds: "build" | WorkpieceId, usable = (_id: WorkpieceId) => true): Set { + if (holds === "build") return new Set(this.accounts().filter(usable)); + let reached = new Set(); + let reach = (id: WorkpieceId) => { + if (reached.has(id) || (this.connections.has(id) && !usable(id))) return; + reached.add(id); + for (let edge of this.edges.get(id) ?? []) if (edge.chat === undefined) reach(edge.target); + for (let hook of this.hooks.values()) { + if (hook.enabled && hook.gadget === id) reach(hook.connection); + } + }; + reach(holds); + return new Set([...reached].filter(id => GADGETS.includes(id) || this.accounts().includes(id))); + } +} + +function pick(list: T[], index: number): T | undefined { + return list.length === 0 ? undefined : list[index % list.length]; +} + +// Replays `steps`. Those before `from` rebuild what a restart left behind, with nobody connected. +async function replayOwnerSteps(steps: Step[], from = 0): Promise { + let restartedAt: number | undefined; + await withImpl(async (impl, restarts, instance) => { + // The owner's user DO, and the gatekeepers behind connections and hooks, faked. + let user = { + id: "owner-user", + whoami: async () => USER_META.profile, + getChatContext: async () => USER_META, + getGatekeeperClassFor: async () => + ({ class: {}, vendorId: "testvendor", typeUrlPattern: "https://*" }), + setWorkspaceLastActive: async () => {}, + messageArrived: async () => {}, + }; + impl.ownerId = user.id; + impl.users = { idFromString: (id: string) => id, get: () => user }; + impl.ensureAmbientCapsules = async () => {}; + impl.markOutputsDirty = () => {}; + impl.getGatekeeperFacet = () => ({ + describe: async () => + ({ title: "Test", url: "https://example.com/new", suggestedBindingName: "RES" }), + }); + // A live hook controller can't be persisted through the real storage, as above. + let controller = { enable: async () => {}, disable: async () => {} }; + let hookRecords = new Map(); + impl.storage.boundHooks = { + get: (id: number) => hookRecords.get(id), + put: (record: { id: number }) => hookRecords.set(record.id, record), + delete: (id: number) => hookRecords.delete(id), + list: () => [...hookRecords.values()], + }; + for (let id of GADGETS) seedGadget(impl, id); + for (let id of CHATS) { + impl.storage.chatMeta.put( + { id, title: "Chat", started: new Date(0), lastActive: new Date(id) }); + } + using notifyClosed = new NativeRpcStub<() => void>(() => {}); + // (The interface open() returns is disposable, which the Overseer type does not say.) + using owner = await instance.open(user.id, OWNER, notifyClosed) as Overseer & Disposable; + let gadgets = new Map>>(); + for (let id of GADGETS) gadgets.set(id, await owner.getGadget(id)); + // Each connection's capability, as its creation returned it. + let created = new Map }>(); + + let model = new ScopeModel(); + let usable = (id: WorkpieceId): boolean => impl.gatekeeperUsable(id); + let needs = (grant: "workspace" | { gadgetId: WorkpieceId }): WorkpieceId[] => + impl.listObserverRequirements(grant) + .map((need: { gatekeeperId: WorkpieceId }) => need.gatekeeperId) + .toSorted((a: WorkpieceId, b: WorkpieceId) => a - b); + let byName = (a: { name: string }, b: { name: string }) => a.name.localeCompare(b.name); + + for (let [index, step] of steps.entries()) { + let label = `step ${index} (${step.kind})`; + let before = new Map(model.sessions.map(session => [session, model.scope(session.holds)])); + let chat = "chat" in step ? CHATS[step.chat - 1] : undefined; + switch (step.kind) { + case "connect": { + let env = Object.fromEntries((step.as === "spawner" ? step.env : []).map((choice, i) => + [`E${i}`, pick([...GADGETS, ...model.connections.keys()], choice)!])); + let client = step.as === "account" + ? await owner.newGatekeeper(1, "https://example.com/doc") + : step.as === "spawner" + ? await owner.newAgentSpawnerGatekeeper({ displayName: "Spawner", modelId: null, env }) + // (The model gatekeeper's entrypoint is not among the test Worker's exports.) + : await impl.addGatekeeper({}, MODEL_SPEC, user.id); + let id = await client.getId(); + created.set(id, client); + model.connections.set(id, step.as); + model.edges.set(id, Object.entries(env).map(([name, target]) => ({ name, target }))); + break; + } + case "remove": { + let id = pick([...model.connections.keys()], step.connection); + if (id === undefined) break; + await created.get(id)!.remove(); + model.connections.delete(id); + model.edges.delete(id); + for (let [holder, edges] of model.edges) { + model.edges.set(holder, edges.filter(edge => edge.target !== id)); + } + for (let [hookId, hook] of model.hooks) { + if (hook.connection === id) model.hooks.delete(hookId); + } + break; + } + case "bind": { + let gadget = GADGETS[step.gadget], edges = model.edges.get(gadget)!; + let target = pick([...model.connections.keys()], step.connection); + if (target === undefined) break; + let name = await gadgets.get(gadget)!.bindWithSuggestedName(target, chat); + let bound = model.listed(gadget, chat).filter(edge => edge.target === target); + if (bound.length > 0) { + expect(bound.map(edge => edge.name), `${label}: its names here already`).toContain(name); + } else { + expect(edges.map(edge => edge.name), `${label}: names in use`).not.toContain(name); + edges.push({ name, target, ...chat === undefined ? {} : { chat } }); + } + break; + } + case "unbind": case "rename": { + let gadget = GADGETS[step.gadget], edges = model.edges.get(gadget)!; + let edge = pick(model.listed(gadget, chat), step.edge); + if (!edge) break; + if (step.kind === "unbind") { + await gadgets.get(gadget)!.unbind(edge.name); + edges.splice(edges.indexOf(edge), 1); + } else { + await gadgets.get(gadget)!.renameBinding(edge.name, edge.name = `B${index}`); + } + break; + } + case "accept": { + expect(await owner.mergeChanges(chat!), label).toEqual({ outcome: "merged" }); + for (let edges of model.edges.values()) { + for (let edge of edges) if (edge.chat === chat) delete edge.chat; + } + break; + } + case "hook": { + let gadget = GADGETS[step.gadget], connection = pick(model.accounts(), step.connection); + if (connection === undefined) break; + let id: number = impl.storage.nextHookId.get(); + await impl.bindHook(connection, controller, {}, + { title: "Hook", description: "Delivers events" }, { from: "gadget", gadgetId: gadget }); + model.hooks.set(id, { connection, gadget, enabled: false }); + break; + } + case "enableHook": case "disableHook": case "deleteHook": { + let id = pick([...model.hooks.keys()], step.hook); + if (id === undefined) break; + // (enableHook's loopback entrypoint is not among the test Worker's exports either; this + // is the record flip its gatekeeper round trip ends on.) + if (step.kind === "enableHook") impl.enableHookRecord(hookRecords.get(id)); + else await owner[step.kind](id); + if (step.kind === "deleteHook") model.hooks.delete(id); + else model.hooks.get(id)!.enabled = step.kind === "enableHook"; + break; + } + case "join": { + if (index < from) break; + let holds = step.holds < GADGETS.length ? GADGETS[step.holds] : "build" as const; + // A "use" collaborator also holds the workspace shell, which answers for nothing. + let leaves: (() => void)[] = holds === "build" + ? [joinSession(impl)] : [impl.joinSession("use"), joinSession(impl, holds)]; + model.sessions.push({ holds, leave: () => leaves.forEach(leave => leave()) }); + break; + } + case "leave": { + let session = pick(model.sessions, step.session); + if (!session) break; + session.leave(); + model.sessions.splice(model.sessions.indexOf(session), 1); + break; + } + } + + expect(needs("workspace"), `${label}: what opening the workspace demands`) + .toEqual(model.accounts()); + for (let [id, client] of gadgets) { + expect(needs({ gadgetId: id }), `${label}: what opening gadget ${id} demands`) + .toEqual(model.accounts().filter(account => model.scope(id).has(account))); + for (let open of [undefined, ...CHATS]) { + let listed = await client.listBindings(open); + expect(listed.map(({ name, target, chatId }) => ({ name, target, chat: chatId })) + .toSorted(byName), `${label}: gadget ${id}'s bindings with chat ${open} open`) + .toEqual(model.listed(id, open).toSorted(byName)); + } + } + + // What the sessions open across the step gained, among what `reachable` connections reach. + let gained = (reachable?: (id: WorkpieceId) => boolean) => [...before] + .filter(([session]) => model.sessions.includes(session)) + .flatMap(([session, scope]) => + [...model.scope(session.holds, reachable)].filter(id => !scope.has(id))); + let widened = gained().length > 0; + expect(restarts.length, `${label}: restarts scheduled`).toBe(widened ? 1 : 0); + expect(gained(usable), `${label}: gained and still reachable`).toEqual([]); + if (widened) { + restartedAt = index; + return; + } + expect([...model.connections.keys()].filter(id => !usable(id)), `${label}: unusable`) + .toEqual([]); + } + }); + if (restartedAt !== undefined) await replayOwnerSteps(steps, restartedAt + 1); +} + +describe("scope widening property", () => { + const BUILD = GADGETS.length; + const of = (...values: T[]) => fc.constantFrom(...values); + const choice = fc.nat({ max: 15 }); + const gadget = fc.nat({ max: GADGETS.length - 1 }); + // Most bindings are made with no chat open, and most collaborators hold a gadget. + const chat = fc.constantFrom(0, 0, 1, 2); + const holds = fc.constantFrom(0, 0, 1, 1, BUILD); + const step: fc.Arbitrary = fc.oneof( + { weight: 3, arbitrary: fc.record({ + kind: of("connect"), as: of("account", "account", "model") }) }, + { weight: 2, arbitrary: fc.record({ + kind: of("connect"), as: of("spawner"), env: fc.array(choice, { maxLength: 2 }) }) }, + { weight: 1, arbitrary: fc.record({ kind: of("remove"), connection: choice }) }, + { weight: 6, arbitrary: fc.record({ kind: of("bind"), gadget, connection: choice, chat }) }, + { weight: 2, arbitrary: fc.record({ kind: of("unbind", "rename"), gadget, edge: choice, chat }) }, + { weight: 3, arbitrary: fc.record({ kind: of("accept"), chat: fc.constantFrom(1, 2) }) }, + { weight: 2, arbitrary: fc.record({ kind: of("hook"), connection: choice, gadget }) }, + { weight: 3, arbitrary: fc.record({ + kind: of("enableHook", "enableHook", "disableHook", "deleteHook"), hook: choice }) }, + { weight: 4, arbitrary: fc.record({ kind: of("join"), holds }) }, + { weight: 1, arbitrary: fc.record({ kind: of("leave"), session: choice }) }, + ); + + // Each comment marks a step that severs whoever is connected. + const examples: [Step[]][] = [ + // A connection that needs an account severs a "build" collaborator when it is created, and a + // gadget's holder when it is bound there. An AI model severs nobody. + [[ + { kind: "join", holds: 0 }, { kind: "connect", as: "account" }, { kind: "join", holds: BUILD }, + { kind: "connect", as: "model" }, { kind: "bind", gadget: 0, connection: 1, chat: 0 }, + { kind: "connect", as: "account" }, // severs + { kind: "join", holds: BUILD }, { kind: "join", holds: 1 }, + { kind: "bind", gadget: 0, connection: 0, chat: 0 }, + { kind: "bind", gadget: 1, connection: 0, chat: 0 }, // severs + ]], + // A chat's proposal severs nobody until that chat is accepted. A second name for a connection + // the gadget reaches widens nothing; binding it again once both are gone does. + [[ + { kind: "connect", as: "account" }, { kind: "join", holds: 0 }, + { kind: "bind", gadget: 0, connection: 0, chat: 1 }, + { kind: "bind", gadget: 0, connection: 0, chat: 2 }, + { kind: "rename", gadget: 0, edge: 0, chat: 1 }, + { kind: "accept", chat: 1 }, // severs + { kind: "join", holds: 0 }, { kind: "accept", chat: 2 }, + { kind: "unbind", gadget: 0, edge: 0, chat: 0 }, { kind: "unbind", gadget: 0, edge: 0, chat: 0 }, + { kind: "bind", gadget: 0, connection: 0, chat: 0 }, // severs + ]], + // A hook widens the gadget it wakes whenever it is enabled, unless the gadget binds its + // connection already. + [[ + { kind: "connect", as: "account" }, { kind: "connect", as: "account" }, + { kind: "bind", gadget: 0, connection: 0, chat: 0 }, + { kind: "hook", connection: 0, gadget: 0 }, { kind: "hook", connection: 1, gadget: 0 }, + { kind: "join", holds: 0 }, { kind: "join", holds: 1 }, { kind: "enableHook", hook: 0 }, + { kind: "enableHook", hook: 1 }, // severs + { kind: "join", holds: 0 }, { kind: "disableHook", hook: 1 }, + { kind: "enableHook", hook: 1 }, // severs + ]], + // A spawner bound into gadget 0 hands its holders gadget 1. That alone severs them, though no + // connection is new, and from then on so does what gadget 1 gains: a binding, a hook and, + // once a second spawner hands gadget 1's holders gadget 0 in turn, what gadget 0 gains. + [[ + { kind: "connect", as: "spawner", env: [1] }, { kind: "connect", as: "account" }, + { kind: "connect", as: "account" }, { kind: "hook", connection: 1, gadget: 1 }, + { kind: "join", holds: 0 }, { kind: "join", holds: 1 }, + { kind: "bind", gadget: 0, connection: 0, chat: 0 }, // severs + { kind: "join", holds: 0 }, + { kind: "bind", gadget: 1, connection: 1, chat: 0 }, // severs + { kind: "join", holds: 0 }, + { kind: "enableHook", hook: 0 }, // severs + { kind: "connect", as: "spawner", env: [0] }, + { kind: "bind", gadget: 1, connection: 3, chat: 0 }, { kind: "connect", as: "account" }, + { kind: "join", holds: 1 }, + { kind: "bind", gadget: 0, connection: 4, chat: 0 }, // severs + ]], + ]; + + it("restarts exactly when an open session's scope widens, and blocks what it gained", async () => { + await fc.assert( + fc.asyncProperty( + fc.array(step, { minLength: 1, maxLength: 30, size: "medium" }), + steps => replayOwnerSteps(steps)), + { ...PROPERTY_RUN_OPTIONS, numRuns: 200, examples }); + // Longer than the options' time limit, so a failure reports its shrunk case, not a timeout. + }, 120_000); +}); diff --git a/packages/workshop-backend/__tests__/unread-state.test.ts b/packages/workshop-backend/__tests__/unread-state.test.ts index 09f999f74a..97691814e0 100644 --- a/packages/workshop-backend/__tests__/unread-state.test.ts +++ b/packages/workshop-backend/__tests__/unread-state.test.ts @@ -1,8 +1,10 @@ import { env, RpcStub as NativeRpcStub } from "cloudflare:workers"; import { evictDurableObject, runDurableObjectAlarm, runInDurableObject } from "cloudflare:test"; import { RpcStub, RpcTarget } from "capnweb"; +import * as fc from "fast-check"; import { describe, expect, it, vi } from "vitest"; -import type { AiChatAuthorInfo, UnreadStateSubscriber, UserUnreadState } from "@gadgets/workshop-shared/api"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; +import type { AiChatAuthorInfo, ChatReadState, UnreadStateSubscriber, UnreadSummaryUpdate, UserUnreadState } from "@gadgets/workshop-shared/api"; import { UserDurableObject, type ChatMessageProgress } from "../src/user.js"; import { OverseerDurableObject } from "../src/overseer.js"; import { makeUserStorage } from "../src/storage-schema/user-storage.js"; @@ -1309,3 +1311,291 @@ describe("source authorization and removal", () => { expect(await reader.getWorkspaceUnreadState(fixture.workspaceId)).toEqual([]); }); }); + +// --------------------------------------------------------------------------------------------- +// Property: unread counts while two people and the agent use three chats in two workspaces, and +// the platform loses, repeats and delays the workspaces' handoffs to the users. fast-check +// generates the steps (see `step`). After each one, for both users, every stored row has +// `unreadCount = latestVisibleSequence - readThroughVisibleSequence` with the model's values, the +// queries list those counts, the summaries are their sums, and a subscriber that folded the +// deltas holds what a snapshot would. A stuck or wrong badge is the first thing every user sees. +// +// The model is docs/user-unread-state.md. A workspace counts each chat's visible messages. A user +// has, per chat, the highest count a handoff or read command brought them and how many of those +// they read. A handoff carries the count rather than a difference, so a repeated or older one +// changes nothing, and the only message that reads itself is a caught-up user's own next one. A +// read command goes through the workspace, so it brings the current count; a bulk read knows only +// what the user already received. Once a user is told a chat is deleted, it stays deleted. +// Scenarios a seed might miss are pinned as `examples`. +describe("unread counts property", () => { + // What became of one handoff: acknowledged, lost on the way, delivered with its ACK lost, or + // held up past the handoff limit until a "land" step picks it. + type Fate = "ack" | "lost" | "noAck" | "late"; + const user = fc.nat({max: 1}), workspace = fc.nat({max: 1}), chat = fc.constantFrom(0, 0, 1, 2); + const fate = fc.constantFrom("ack", "ack", "ack", "ack", "lost", "noAck", "late"), fates = fc.tuple(fate, fate); + // `user`, `chat` and `workspace` index the lists in replay(); the first chat gets half the steps. + // A message is `by` a user or the agent, whose "tool" records are not visible. `at` and `pick` + // choose, modulo the count, among what is there when the step runs: for "read" any record on + // screen, for "unread" a message. Without `at` the command is the thread menu's, which names no + // record; "readAll" without `workspace` reads everything. "retry" is a workspace's alarm retrying + // its failed handoffs. Resets and evictions are the slow steps, so they are the rare ones. + const step = fc.oneof( + {weight: 8, arbitrary: fc.record({kind: fc.constant("post" as const), chat, fates, + by: fc.constantFrom(0, 1, "agent", "agent", "tool")})}, + {weight: 2, arbitrary: fc.record({kind: fc.constant("delete" as const), chat, fates})}, + {weight: 6, arbitrary: fc.record({kind: fc.constantFrom("read" as const, "unread" as const), user, chat, + at: fc.nat({max: 7})}, {requiredKeys: ["kind", "user", "chat"]})}, + {weight: 2, arbitrary: fc.record({kind: fc.constant("readAll" as const), user, workspace}, {requiredKeys: ["kind", "user"]})}, + {weight: 1, arbitrary: fc.record({kind: fc.constant("resetUser" as const), user})}, + {weight: 3, arbitrary: fc.record({kind: fc.constant("retry" as const), workspace})}, + {weight: 1, arbitrary: fc.record({kind: fc.constant("evictWorkspace" as const), workspace})}, + {weight: 3, arbitrary: fc.record({kind: fc.constant("land" as const), pick: fc.nat({max: 7})})}, + ); + type Step = typeof step extends fc.Arbitrary ? T : never; + // A handoff's content: the chat's visible count and which user, if any, wrote its latest visible + // message. Null once the chat is deleted. + type Packet = {head: number; author?: number} | null; + // `records` says which raw sequences are visible. A run numbers its records across chats, as + // `write` derives a workspace-unique timestamp from the sequence, so each chat has gaps. + type Chat = {workspace: number; chatId: number; records: boolean[]; author?: number; deleted?: true; failed: Set}; + type Row = {head: number; read: number; deleted?: true}; + + class Folded extends RpcTarget implements UnreadStateSubscriber { + state?: UserUnreadState; + first = Promise.withResolvers(); + async updated(state: UserUnreadState) { this.state = state; this.first.resolve(); } + async summaryUpdated({workspace, ...totals}: UnreadSummaryUpdate) { + const others = this.state!.workspaces.filter(row => row.workspaceId !== workspace.workspaceId); + this.state = {...this.state!, ...totals, workspaces: workspace.unreadChatCount ? [...others, workspace] : others}; + } + async chatUpdated(workspaceId: string, chatId: number, chat: ChatReadState | null, summary: UnreadSummaryUpdate) { + const others = this.state!.chats.filter(row => row.workspaceId !== workspaceId || row.chatId !== chatId); + this.state = {...this.state!, chats: chat ? [...others, chat] : others}; + await this.summaryUpdated(summary); + } + } + const unordered = (state: UserUnreadState) => ({...state, chats: new Set(state.chats), workspaces: new Set(state.workspaces)}); + const sum = (counts: number[]) => ({unreadChatCount: counts.filter(Boolean).length, + unreadMessageCount: counts.reduce((total, count) => total + count, 0)}); + + async function replay(steps: Step[]) { + const first = await setup(), {reader, owner, profile} = await addReader(first), second = await setup(first.user); + // A second chat in the first workspace, and the collaborator in the second workspace too. + await withImpl(first.workspace, async impl => { + impl.storage.chatMeta.put({id: 2, title: "Second", started: new Date(), lastActive: new Date()}); + await impl.deliverDirtyChats(); + }); + await withImpl(second.workspace, async impl => { + impl.storage.collaborators.put({profile, addedBy: [{type: "user", sharer: owner.id, role: "build", created: new Date()}]}); + }); + await reader.recordSharedWorkspaceOpen(second.workspaceId, "Workspace", owner, "build", await notificationBaselines(second)); + const workspaces = [first, second], users = [first.user, reader], authors = [owner, profile]; + // The owner watches the first workspace's chat list and the collaborator the inbox. + const scopes = [{workspaceId: first.workspaceId}, "unread"] as const; + const chats: Chat[] = [[0, 1], [0, 2], [1, 1]].map(([workspace, chatId]) => ({workspace, chatId, records: [], failed: new Set()})); + const rows = users.map(() => chats.map((): Row => ({head: 0, read: 0}))); + const late: {user: number; chat: number; packet: Packet; args: Parameters}[] = []; + let nextSequence = 0; + const key = (chat: Chat) => `${workspaces[chat.workspace].workspaceId}:${chat.chatId}`; + // How many of a chat's records through raw sequence `through` are visible. + const visible = (chat: Chat, through = chat.records.length - 1) => chat.records.slice(0, through + 1).filter(Boolean).length; + const packetFor = (chat: Chat): Packet => chat.deleted ? null : {head: visible(chat), author: chat.author}; + const receive = (user: number, chat: number, packet: Packet) => { + const row = rows[user][chat]; + if (row.deleted) return; + if (!packet) { row.deleted = true; return; } + const caughtUp = row.head === row.read; + row.head = Math.max(row.head, packet.head); + if (caughtUp && row.head === row.read + 1 && packet.author === user) row.read = row.head; + }; + + const subscribe = async (user: number) => { + const folded = new Folded(), callback = new RpcStub(folded); + const subscription = await users[user].subscribeToUnreadState(scopes[user], callback); + await folded.first.promise; + return {folded, [Symbol.dispose]() { subscription[Symbol.dispose](); callback[Symbol.dispose](); }}; + }; + const subscriptions = [await subscribe(0), await subscribe(1)]; + + // Make a change in a chat's workspace and fan it out, each user's handoff meeting its fate. + // As after any write, failures parked on the workspace's other chats wait for the alarm. + const fanOut = async (chat: Chat, fates: Fate[], change: (impl: Impl) => void) => { + const index = chats.indexOf(chat), packet = packetFor(chat); + await withImpl(workspaces[chat.workspace].workspace, async impl => { + const restore = interceptDelivery(impl, async (stub, ...args) => { + const user = users.findIndex(candidate => candidate.id.toString() === stub.id.toString()); + if (fates[user] === "late") late.push({user, chat: index, packet, args}); + else if (fates[user] !== "lost") await stub.messageArrived(...args); + if (fates[user] !== "ack") throw new Error("Handoff failed"); + }); + try { + change(impl); + await impl.deliverDirtyChats({retryFailures: false}) + .catch(error => { if (fates.every(fate => fate === "ack")) throw error; }); + } finally { restore(); } + }); + fates.forEach((fate, user) => { + if (fate === "ack" || fate === "noAck") receive(user, index, packet); + if (fate === "ack") chat.failed.delete(user); else chat.failed.add(user); + }); + }; + + const run = async (step: Step) => { + switch (step.kind) { + case "post": { + const chat = chats[step.chat], shown = step.by !== "tool", author = typeof step.by === "number" ? step.by : undefined; + if (chat.deleted) break; + const sequence = nextSequence++; + chat.records[sequence] = shown; + if (shown) chat.author = author; + await fanOut(chat, shown ? step.fates : [], impl => write(impl, sequence, shown ? "Hello" : "", chat.chatId, + author === undefined ? undefined : authors[author])); + break; + } + case "delete": { + const chat = chats[step.chat]; + if (chat.deleted) break; + chat.deleted = true; + await fanOut(chat, step.fates, impl => { impl.storage.chatMeta.delete(chat.chatId); }); + break; + } + case "read": case "unread": { + const chat = chats[step.chat], row = rows[step.user][step.chat], rewind = step.kind === "unread"; + const messages = chat.records.flatMap((shown, sequence) => shown ? [sequence] : []); + const offered = rewind ? messages : chat.records.flatMap((_, sequence) => [sequence]); + const at = step.at === undefined ? undefined : offered[step.at % offered.length]; + await setRead({...workspaces[chat.workspace], user: users[step.user]}, at, rewind, chat.chatId); + const boundary = at ?? messages.at(-1) ?? -1; + if (chat.deleted || (rewind && boundary < 0)) break; + row.read = rewind ? visible(chat, boundary - 1) : Math.max(row.read, visible(chat, boundary)); + row.head = Math.max(row.head, visible(chat)); + break; + } + case "readAll": + await (step.workspace === undefined ? users[step.user].markAllRead() + : users[step.user].markWorkspaceRead(workspaces[step.workspace].workspaceId)); + for (const [index, row] of rows[step.user].entries()) { + if (step.workspace === undefined || chats[index].workspace === step.workspace) row.read = row.head; + } + break; + // A subscribed client pins its user's object, so it goes away only by a reset. + case "resetUser": + subscriptions[step.user][Symbol.dispose](); + await expect(runInDurableObject(users[step.user], (_instance, ctx) => ctx.abort("Reset"))).rejects.toThrow("Reset"); + users[step.user] = env.TEST_USER.get(users[step.user].id); + subscriptions[step.user] = await subscribe(step.user); + break; + // An evicted workspace with failed handoffs is next woken by the alarm that retries them. + case "retry": case "evictWorkspace": { + if (step.kind === "evictWorkspace") await evictDurableObject(workspaces[step.workspace].workspace); + const pending = chats.filter(chat => chat.workspace === step.workspace && chat.failed.size); + const armed = await runDurableObjectAlarm(workspaces[step.workspace].workspace); + if (pending.length) expect(armed, "failed handoffs keep the workspace's alarm armed").toBe(true); + for (const chat of pending) { + for (const user of chat.failed) receive(user, chats.indexOf(chat), packetFor(chat)); + chat.failed.clear(); + } + break; + } + case "land": { + if (!late.length) break; + const [handoff] = late.splice(step.pick % late.length, 1); + await users[handoff.user].messageArrived(...handoff.args); + receive(handoff.user, handoff.chat, handoff.packet); + break; + } + } + }; + + const check = async (label: string) => { + for (const [user, stub] of users.entries()) { + const {stored, summary, listed} = await runInDurableObject(stub, async (instance, ctx) => ({ + stored: chats.map(chat => makeUserStorage(ctx.storage).chatReadCursors.get(key(chat))), + summary: await instance.getUnreadSummary(), + listed: (await Promise.all(workspaces.map(({workspaceId}) => instance.getWorkspaceUnreadState(workspaceId)))).flat(), + })); + const counts = rows[user].map(row => row.deleted ? 0 : row.head - row.read); + const rollups = workspaces.map(({workspaceId}, workspace) => + ({workspaceId, ...sum(counts.filter((_, index) => chats[index].workspace === workspace))})); + expect({ + // Per chat: latest, read, unread, and the visible count through the raw read position, + // which ties the divider the UI draws there to the count. + stored: stored.map((cursor, chat) => cursor?.deleted ? `deleted, ${cursor.unreadCount} unread` : + [cursor?.latestVisibleSequence ?? 0, cursor?.readThroughVisibleSequence ?? 0, cursor?.unreadCount ?? 0, + visible(chats[chat], cursor?.readThroughSequence ?? -1)]), + listed: Object.fromEntries(listed.map(state => [`${state.workspaceId}:${state.chatId}`, state.unreadMessageCount])), + summary: unordered({...summary, chats: []}), + }, `${label}: user ${user}`).toEqual({ + stored: rows[user].map((row, chat) => row.deleted ? "deleted, 0 unread" : [row.head, row.read, counts[chat], row.read]), + // The queries list every tracked chat that is not deleted, read ones included. + listed: Object.fromEntries(chats.flatMap((chat, index) => + stored[index] && !rows[user][index].deleted ? [[key(chat), counts[index]]] : [])), + summary: unordered({...sum(counts), workspaces: rollups.filter(rollup => rollup.unreadChatCount), chats: []}), + }); + const scope = scopes[user], inScope = (state: ChatReadState) => + scope === "unread" ? state.unreadMessageCount > 0 : state.workspaceId === scope.workspaceId; + await vi.waitFor(() => expect(unordered(subscriptions[user].folded.state!), `${label}: user ${user}'s folded deltas`) + .toEqual(unordered({...summary, chats: listed.filter(inScope)})), {interval: 2}); + } + }; + + try { + await check("start"); + for (const [index, step] of steps.entries()) { + await run(step); + await check(`step ${index} ${JSON.stringify(step)}`); + } + // Once the retries are through, everything still on its way is old news. + await run({kind: "retry", workspace: 0}); await run({kind: "retry", workspace: 1}); + while (late.length) await run({kind: "land", pick: 0}); + await check("settled"); + for (const [user, {folded}] of subscriptions.entries()) { + using fresh = await subscribe(user); + expect(unordered(folded.state!), `user ${user}'s folded deltas`).toEqual(unordered(fresh.folded.state!)); + } + } finally { for (const subscription of subscriptions) subscription[Symbol.dispose](); } + } + + const [OWNER, READER] = [0, 1], acked: [Fate, Fate] = ["ack", "ack"]; + const examples: [Step[]][] = [ + // Redelivery after mark-unread. The owner's message read itself when it arrived; they mark it + // unread, and the workspace, which missed the ACK, hands it over again. It stays unread. + [[ + {kind: "post", chat: 0, by: OWNER, fates: ["noAck", "ack"]}, + {kind: "unread", user: OWNER, chat: 0}, + {kind: "retry", workspace: 0}, + ]], + // A reset between a fan-out and a read, and an older handoff landing after a newer one. The + // owner's first handoff is held up, the second brings both messages, and they read through the + // tool record between the two: one message is left, whenever the first handoff lands. The + // collaborator reads everything; reading through an earlier record then takes nothing back. + [[ + {kind: "post", chat: 0, by: "agent", fates: ["late", "ack"]}, + {kind: "post", chat: 0, by: "tool", fates: acked}, + {kind: "post", chat: 0, by: READER, fates: acked}, + {kind: "resetUser", user: OWNER}, + {kind: "read", user: OWNER, chat: 0, at: 1}, + {kind: "land", pick: 0}, + {kind: "read", user: READER, chat: 0}, + {kind: "read", user: READER, chat: 0, at: 0}, + ]], + // A chat deleted during a pending retry. The collaborator is told at once, so their held-up + // handoff from before the deletion changes nothing, even after a reset. The owner is not, and + // goes on counting the message until the evicted workspace's retry brings the deletion. + [[ + {kind: "post", chat: 1, by: "agent", fates: ["noAck", "late"]}, + {kind: "post", chat: 0, by: "agent", fates: acked}, + {kind: "delete", chat: 1, fates: ["lost", "ack"]}, + {kind: "resetUser", user: READER}, + {kind: "land", pick: 0}, + {kind: "unread", user: OWNER, chat: 1}, + {kind: "evictWorkspace", workspace: 0}, + ]], + ]; + + it("keeps every user's counts, summaries and subscribers on what the workspaces handed over and the user read", async () => { + using _warnings = vi.spyOn(console, "warn").mockImplementation(() => {}); // One per failed handoff. + await fc.assert(fc.asyncProperty(fc.array(step, {minLength: 1, maxLength: 24, size: "max"}), replay), + {...PROPERTY_RUN_OPTIONS, numRuns: 60, examples}); + }, 120_000); +}); From ba836e83390e16caaea60aacfbb8e67a58173819 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sun, 8 Nov 2026 11:40:39 -0600 Subject: [PATCH 07/12] Property-test gatekeeper-kit's action set and credentials - Action set: submissions, approvals and denials in the SharePoint shape (a creation, items inside it, edits of them) while the provider succeeds, fails retryably, refuses or times out. Journal records, the pending projection, handler dispatches, release hooks and refused submissions match a model after every step, and no pending action waits on a reference nothing can still bind. - Credentials: two chats on one connection with single-use refresh tokens, under fast-check's scheduler, while the token expires, the user reconnects or disconnects and the provider revokes the grant. No operation runs under two connections or replays a write, the stored grant is the last one connected, and every minted token is stored, discarded or refused. - gatekeeper-kit gains fast-check and its test task fingerprints FAST_CHECK_SEED. --- .../__tests__/actions-properties.test.ts | 298 ++++++++++++++++++ .../__tests__/credentials-properties.test.ts | 257 +++++++++++++++ packages/gatekeeper-kit/package.json | 1 + packages/gatekeeper-kit/vite.config.ts | 14 +- packages/integration-tests/README.md | 4 +- pnpm-lock.yaml | 7 +- scripts/env-passthrough.test.ts | 4 +- 7 files changed, 578 insertions(+), 7 deletions(-) create mode 100644 packages/gatekeeper-kit/__tests__/actions-properties.test.ts create mode 100644 packages/gatekeeper-kit/__tests__/credentials-properties.test.ts diff --git a/packages/gatekeeper-kit/__tests__/actions-properties.test.ts b/packages/gatekeeper-kit/__tests__/actions-properties.test.ts new file mode 100644 index 0000000000..8139038f17 --- /dev/null +++ b/packages/gatekeeper-kit/__tests__/actions-properties.test.ts @@ -0,0 +1,298 @@ +// Property: an action set and its journal under an agent's turns and its approver, in the +// SharePoint shape. fast-check generates a sequence in which the agent creates items, some inside +// a folder it has only proposed, and edits items it has proposed, while the user approves and +// denies them in any order and the provider succeeds, fails retryably, refuses +// (`ActionApplyError`) or times out (`ActionOutcomeUnknownError`). Each sequence is replayed +// against `defineActions` over a fresh journal. After every step the journal's records, the +// `listPending` projection, the handler dispatches, the release hooks, the ids sent to the overseer +// and whether the call succeeded are checked against a model, and no pending action may wait on a +// reference nothing can still bind. A regression shows as an upload applied into a folder whose +// creation was denied, a dependent stuck in the queue, or a handler dispatched twice. +// +// The model follows USAGE.md's "Actions and files". An action is pending, failed, applied or gone. +// A submission is refused unless the reference it names is bound or a pending action will create +// it. Approving dispatches only a pending action whose reference is bound. Success binds what the +// action creates and a retryable failure leaves it pending. Either terminal failure fails it, and +// a refusal also retires every action waiting on what it was to create, transitively. Denying +// withdraws the action, retires those waiting on it, and runs the release hook unless the handler +// already ran. Nothing is retired over a reference that was bound: a call can fail `late`, after +// the provider created the item, as a SharePoint upload is bound before its workbook recalculates. +// +// A decision usually names an action awaiting one, but may name any submitted so far: the overseer +// can fail to record a decision that completed (`Gatekeeper.applyAction`), and the user then +// decides the action again. Scenarios a seed might miss are pinned as `examples`. + +import * as fc from "fast-check"; +import { expect, it, vi } from "vitest"; +import type { ApprovalQueue } from "@gadgets/workshop-shared/gatekeeper"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; +import { + ActionApplyError, ActionJournal, ActionOutcomeUnknownError, defineActions, type ActionSubmitter, + type TaggedAction, +} from "../src/actions"; +import { fakeKv } from "./fake-kv"; + +/** What the provider does with a call that reaches it. */ +type Provider = "ok" | "retry" | "refused" | "unknown"; +type Actions = { create: { ref: string; parent?: string }; edit: { target: string } }; +type Action = TaggedAction; +type Host = { + provider: Provider; late: boolean; bound: Set; dispatched: number[]; released: number[]; +}; + +// `parent` and `target` pick, modulo the count, among the provisional references the agent holds +// (`parent: null` is a folder that already exists). `pick` does among the actions the approver +// still has to decide, or among `any` submitted so far. +type Step = + | { do: "create"; parent: number | null } + | { do: "edit"; target: number } + | { do: "apply"; pick: number; any: boolean; provider: Provider; late: boolean } + | { do: "reject"; pick: number; any: boolean }; + +const created = (action: Action) => (action.kind === "create" ? action.payload.ref : undefined); +const needed = (action: Action) => + action.kind === "create" ? action.payload.parent : action.payload.target; +const pick = (from: readonly T[], n: number): T | undefined => from[n % from.length]; + +function reach(host: Host, id: number, ref?: string) { + host.dispatched.push(id); + // A `late` failure follows the creation, so the handler has already bound what it made. + if (ref !== undefined && (host.provider === "ok" || host.late)) host.bound.add(ref); + if (host.provider === "retry") throw new Error("provider unreachable"); + if (host.provider === "refused") throw new ActionApplyError("the provider refused"); + if (host.provider === "unknown") throw new ActionOutcomeUnknownError("the request timed out"); +} + +function declare(retainApplied: boolean) { + const common = { + kind: { tag: "files", label: "Change files" }, + autoApprovable: true, + delivery: "continue-with-simulation", + // As SharePoint's creations declare, and what an unknown outcome is documented to pair with. + claimBeforeApply: true, + describe: () => ({ title: "Change a file", description: "…", implementsRevert: false }), + reject: async (_payload: unknown, host: Host, { id }: { id: number }) => + void host.released.push(id), + } as const; + return defineActions({ + create: { + ...common, + provides: payload => [payload.ref], + dependsOn: payload => (payload.parent === undefined ? [] : [payload.parent]), + apply: async (payload, host, { id }) => reach(host, id, payload.ref), + }, + edit: { + ...common, + dependsOn: payload => [payload.target], + apply: async (_payload, host, { id }) => reach(host, id), + }, + }, { fence: "none", retainApplied, isResolvedReference: (host, ref) => host.bound.has(ref) }); +} + +type Modeled = { + action: Action; + status: "pending" | "failed" | "applied" | "gone"; + /** Failed before its handler ran, so denying it still releases what staging set up. */ + undispatched?: boolean; + /** Failed with the provider's answer unknown: what it creates may yet exist. */ + unknown?: boolean; +}; + +class ActionModel { + /** Every accepted submission; an action's id is its index plus one. */ + actions: Modeled[] = []; + bound = new Set(); + dispatched: number[] = []; + released: number[] = []; + + /** The id an accepted submission gets; undefined for one that is refused. */ + submit(action: Action): number | undefined { + const ref = needed(action); + const creatable = this.actions.some(a => a.status === "pending" && created(a.action) === ref); + if (ref !== undefined && !this.bound.has(ref) && !creatable) return undefined; + return this.actions.push({ action, status: "pending" }); + } + + /** Whether the approval succeeds. */ + apply(id: number, provider: Provider, late: boolean): boolean { + const entry = this.actions[id - 1]!; + const ref = needed(entry.action); + if (entry.status === "applied") return true; + if (entry.status !== "pending" || (ref !== undefined && !this.bound.has(ref))) return false; + this.dispatched.push(id); + const made = created(entry.action); + if (made !== undefined && (provider === "ok" || late)) this.bound.add(made); + if (provider === "retry") return false; + entry.status = provider === "ok" ? "applied" : "failed"; + if (provider === "unknown") entry.unknown = true; + if (provider === "refused") this.#strand(made); + return provider === "ok"; + } + + /** Whether the denial succeeds. */ + reject(id: number): boolean { + const entry = this.actions[id - 1]!; + if (entry.status === "applied") return false; + if (entry.status === "gone") return true; + if (entry.status === "pending" || entry.undispatched) this.released.push(id); + entry.status = "gone"; + this.#strand(created(entry.action)); + return true; + } + + /** Retires every pending action waiting on a reference nothing will now create. */ + #strand(ref: string | undefined) { + if (ref === undefined || this.bound.has(ref)) return; + for (const waiting of this.actions) { + if (waiting.status !== "pending" || needed(waiting.action) !== ref) continue; + waiting.status = "failed"; + waiting.undispatched = true; + this.#strand(created(waiting.action)); + } + } + + /** The ids a decision may name: those awaiting one, or every one. */ + decidable(any: boolean): number[] { + return this.actions.flatMap(({ status }, at) => + (any || status === "pending" || status === "failed" ? [at + 1] : [])); + } + + /** How the journal holds each action: its record's state, or how the record went away. */ + stored(retainApplied: boolean): string[] { + return this.actions.map(({ status, undispatched, unknown }) => { + if (status === "applied") return retainApplied ? "applied" : "retired"; + if (status !== "failed") return status; + return `failed${undispatched ? " undispatched" : ""}${unknown ? " unknown" : ""}`; + }); + } +} + +const succeeds = (call: Promise) => call.then(() => true, () => false); + +function stored(journal: ActionJournal, id: number): string { + const record = journal.get(id); + if (record === undefined) return journal.wasApplied(id) ? "retired" : "gone"; + return `${record.state}${record.undispatched ? " undispatched" : ""}` + + (record.outcome === "unknown" ? " unknown" : ""); +} + +async function replay(retainApplied: boolean, steps: Step[]) { + const host: Host = { provider: "ok", late: false, bound: new Set(), dispatched: [], released: [] }; + const journal = new ActionJournal(fakeKv(), { namespace: "pending" }); + const actions = declare(retainApplied).bind(journal, host); + const model = new ActionModel(); + const submitAction = vi.fn(async () => {}); + const queue: ActionSubmitter = { submitAction }; + /** The provisional references the agent was handed, one per accepted creation. */ + const refs: string[] = []; + + for (const [index, step] of steps.entries()) { + const label = `step ${index} (${JSON.stringify(step)})`; + const id = "pick" in step ? pick(model.decidable(step.any), step.pick) : undefined; + if (step.do === "create") { + const ref = `~${refs.length + 1}`; + const parent = step.parent === null ? undefined : pick(refs, step.parent); + const payload = parent === undefined ? { ref } : { ref, parent }; + const accepted = await actions.submit(queue, "create", payload).catch(() => undefined); + expect(accepted, `${label}: id`).toBe(model.submit({ kind: "create", payload })); + if (accepted !== undefined) refs.push(ref); + } else if (step.do === "edit") { + const target = pick(refs, step.target); + if (target === undefined) continue; + const accepted = await actions.submit(queue, "edit", { target }).catch(() => undefined); + expect(accepted, `${label}: id`).toBe(model.submit({ kind: "edit", payload: { target } })); + } else if (id === undefined) { + continue; + } else if (step.do === "apply") { + host.provider = step.provider; + host.late = step.late; + expect(await succeeds(actions.apply(id)), `${label}: approving ${id} succeeds`) + .toBe(model.apply(id, step.provider, step.late)); + } else { + expect(await succeeds(actions.reject(id)), `${label}: denying ${id} succeeds`) + .toBe(model.reject(id)); + } + + const ids = model.actions.map((_, at) => at + 1); + expect(submitAction.mock.calls.map(([sent]) => sent), `${label}: sent to the overseer`) + .toEqual(ids); + expect(ids.map(each => stored(journal, each)), `${label}: journal records`) + .toEqual(model.stored(retainApplied)); + expect(journal.listPending(), `${label}: pending projection`).toEqual(model.actions.flatMap( + ({ action, status }, at) => (status === "pending" ? [{ id: at + 1, action }] : []))); + expect(host.dispatched, `${label}: handler dispatches`).toEqual(model.dispatched); + expect(host.released, `${label}: release hooks`).toEqual(model.released); + + // Read off the journal alone, so it holds even where the model shares a mistake with the kit: + // a reference can still be bound only by a pending creation or one whose outcome is unknown. + const bindable = new Set(host.bound); + for (const record of ids.map(each => journal.get(each))) { + if (record?.state === "pending" || record?.outcome === "unknown") { + bindable.add(created(record.action)); + } + } + for (const { id: waiting, action } of journal.listPending()) { + const ref = needed(action); + expect(ref === undefined || bindable.has(ref), `${label}: ${waiting} can still apply`) + .toBe(true); + } + } +} + +const choice = fc.nat({ max: 11 }); +// Most decisions are on an action still awaiting one. +const seldom = fc.constantFrom(false, false, false, true); +// Submissions outweigh decisions, so a decision usually lands on an action others wait on. +const step: fc.Arbitrary = fc.oneof( + { weight: 4, arbitrary: fc.record({ do: fc.constant("create" as const), parent: fc.option(choice) }) }, + { weight: 3, arbitrary: fc.record({ do: fc.constant("edit" as const), target: choice }) }, + { + weight: 4, + arbitrary: fc.record({ + do: fc.constant("apply" as const), pick: choice, any: seldom, + provider: fc.constantFrom("ok", "ok", "ok", "ok", "retry", "refused", "unknown"), + late: fc.constantFrom(false, false, true), + }), + }, + { weight: 2, arbitrary: fc.record({ do: fc.constant("reject" as const), pick: choice, any: seldom }) }, +); + +// Examples name an action by its id, and a reference by its creation's place, from 0, among the +// creations accepted so far. +const create = (parent: number | null = null): Step => ({ do: "create", parent }); +const edit = (target: number): Step => ({ do: "edit", target }); +const approve = (id: number, provider: Provider = "ok", late = false): Step => + ({ do: "apply", pick: id - 1, any: true, provider, late }); +const deny = (id: number): Step => ({ do: "reject", pick: id - 1, any: true }); + +const examples: [boolean, Step[]][] = [ + // A folder (1), an upload into it (2) and an edit of the upload (3). Denying the folder retires + // both: approving the upload reaches no handler, another upload into the folder is refused, and + // denying each retired action releases what its staging set up. + [true, [create(), create(0), edit(1), deny(1), approve(2), create(0), deny(2), deny(3)]], + // A creation that timed out keeps its dependent waiting, though no new one is accepted, and + // retires it once the user clears the failure -- without running the creation's release hook. + [false, [create(), edit(0), approve(1, "unknown"), approve(2), edit(0), deny(1), deny(2)]], + // Approved out of order, retried after an outage, then approved and denied again by a user whose + // approval the overseer never recorded: neither reaches the handler nor undoes the applied + // folder. The provider then refuses the upload into it, which retires the edit of that upload. + [true, [ + create(), create(0), edit(1), approve(2), approve(1, "retry"), approve(1), approve(1), deny(1), + approve(2, "refused"), edit(1), + ]], + // An upload the provider created before the call failed stays pending, and denying it leaves the + // edit of the file that now exists to apply. + [false, [create(), edit(0), approve(1, "retry", true), deny(1), edit(0), approve(2), approve(3)]], +]; + +it("matches a model of the queue after every submission, approval and denial", async () => { + // The kit logs each retirement at debug, thousands of lines over the runs. + const logged = vi.spyOn(console, "debug").mockImplementation(() => {}); + try { + await fc.assert( + fc.asyncProperty(fc.boolean(), fc.array(step, { maxLength: 40, size: "max" }), replay), + { ...PROPERTY_RUN_OPTIONS, numRuns: 1000, examples }); + } finally { + logged.mockRestore(); + } +}); diff --git a/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts b/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts new file mode 100644 index 0000000000..2f8bb0b53d --- /dev/null +++ b/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts @@ -0,0 +1,257 @@ +// Property: one account's credentials under the scheduling real usage produces. Two chats share a +// connection to a provider whose refresh tokens are single-use (GitLab, X) while its access token +// expires or is dropped early, the user reconnects or disconnects, and the provider revokes the +// grant. fast-check generates those steps in order, and its scheduler decides when each provider +// response, account reply and Workshop notification lands between them. +// +// Losing one rotation locks the account until the user reconnects, and replaying a write after a +// reconnect performs it as whoever reconnected. So, whatever the schedule: +// - an operation runs at most twice, once unless replayable, and never under two connections; +// - what is stored is the last grant connected, or nothing after a disconnect; +// - each token pair the provider minted went exactly one way: it was stored, or handed to +// `discardMint`, or refused by `connect()` because a later reconnect or disconnect won; +// - afterwards each chat can run an operation exactly when the account is still connected to a +// grant the provider has not revoked. +// The oracles are the test's own: each operation records the tokens it was called with, the +// provider maps every token it minted to its grant, and storage records what was written to it. +// Every run builds a fresh provider, account and chats, so runs and shrinks are independent, and +// the workflows a seed might miss are pinned as `examples`, each under a few fixed schedules. + +import * as fc from "fast-check"; +import { expect, it, vi } from "vitest"; +import { PROPERTY_RUN_OPTIONS } from "@gadgets/scripts/property-runs"; +import { + ConnectionSupersededError, CredentialCoordinator, CredentialsExpiredError, CredentialSource, + isCredentialsChanged, isCredentialsExpired, +} from "../src/credentials"; +import { fakeKv } from "./fake-kv"; + +type Creds = { access: string; refresh: string; expiresAt: number }; +type Step = + | { kind: "run"; chat: 0 | 1; replayable: boolean } + | { kind: "expire" | "staleBearer" | "reconnect" | "revoke" | "disconnect" }; + +/** + * A provider with expiring access tokens and single-use refresh tokens. Each connect flow + * authorizes a new grant, and only a grant's newest access token is accepted: GitLab invalidates + * one when it issues the next. Time is the model's own, since fast-check's time limit reads the + * real clock. + */ +function rotatingProvider() { + const minted: (Creds & { grant: number })[] = []; + let [now, grants, revokedBelow, staleBelow] = [0, 0, 0, 0]; + const mint = (grant: number): Creds => { + const serial = minted.length; + const creds = { access: `access${serial}`, refresh: `refresh${serial}`, expiresAt: now + 1 }; + minted.push({ ...creds, grant }); + return creds; + }; + const newest = (grant: number) => minted.findLast(pair => pair.grant === grant); + const live = (grant: number) => grant >= revokedBelow; + return { + /** Every token pair issued, in order. */ + minted, + now: () => now, + grantOf: (access: string) => minted.find(pair => pair.access === access)?.grant, + live, + authorize: () => mint(grants++), + refresh(current: Creds): Creds { + const used = minted.find(pair => pair.refresh === current.refresh); + if (used === undefined || !live(used.grant) || used !== newest(used.grant)) { + throw new CredentialsExpiredError("invalid_grant"); + } + return mint(used.grant); + }, + accepts(access: string): boolean { + const serial = minted.findIndex(pair => pair.access === access); + const used = minted[serial]; + return used !== undefined && live(used.grant) && used === newest(used.grant) + && serial >= staleBelow && used.expiresAt > now; + }, + expire: () => void now++, + /** Drops every access token early; their refresh tokens still work. */ + dropAccessTokens: () => void (staleBelow = minted.length), + /** The user revokes the app at the provider, ending every grant made so far. */ + revoke: () => void (revokedBelow = grants), + }; +} + +/** How often the schedules reached the races the property is about. */ +const reached = { reconnectMidRefresh: 0, discardedMint: 0, secondAttempt: 0 }; + +async function replay(steps: Step[], s: fc.Scheduler): Promise { + const provider = rotatingProvider(); + // Everything that crosses a network waits here for the scheduler to let it land. + const hop = (label: string) => s.schedule(Promise.resolve(), label); + // What became of each mint, by access token. + const [written, discarded, refused] = [0, 1, 2].map(() => new Set()); + const kv = fakeKv(); + const coordinator = new CredentialCoordinator({ + ...kv, + put: (key, value) => { + if (typeof value === "object" && value !== null && "access" in value) { + written.add(value.access); + } + kv.put(key, value); + }, + }, { + expiresAt: creds => creds.expiresAt > provider.now() ? Number.MAX_SAFE_INTEGER : 0, + discardMint: async mint => { + discarded.add(mint.access); + await hop("discardMint"); + }, + }); + + let refreshing = 0; + const refresh = async (current: Creds): Promise => { + refreshing++; + try { + await hop(`refresh with ${current.refresh}`); + return provider.refresh(current); + } finally { + // The provider has rotated by now, whatever happens to the account before its answer lands. + await hop("refresh answered"); + refreshing--; + } + }; + const notify = () => hop("notify"); + // The account answers at once and the reply arrives later, so a chat can act on a read the + // account has since moved past. + const reply = async (label: string, answer: Promise): Promise => { + try { + return await answer; + } finally { + await hop(label); + } + }; + const account = { + getCredentials: () => reply("getCredentials", coordinator.snapshot(refresh, { notify })), + reportCredentialsRejected: (identity: string) => reply("reportCredentialsRejected", + coordinator.adjudicateRejection(identity, { refresh, notify })), + }; + const chats = [0, 1].map(() => new CredentialSource({ + account: () => account, + isAuthError: error => error instanceof Error && error.message === "401", + expiredMessage: "Reconnect.", + })); + + const runs: { replayable: boolean; attempts: string[] }[] = []; + /** @returns Whether the operation succeeded, the chat otherwise told to re-enter or reconnect. */ + const run = (chat: number, replayable: boolean): Promise => { + const attempts: string[] = []; + runs.push({ replayable, attempts }); + return chats[chat].run(async creds => { + attempts.push(creds.access); + await hop(`chat ${chat} calls with ${creds.access}`); + if (!provider.accepts(creds.access)) throw new Error("401"); + }, { replayable }).then(() => true, error => { + if (!isCredentialsChanged(error) && !isCredentialsExpired(error)) throw error; + return false; + }); + }; + + // The test's own record of the connection: the grant connected, and a count of connects and + // disconnects that tells a reconnect whether another one overtook it. + let connected: number | undefined = 0; + let epoch = 0; + coordinator.connect(provider.authorize()); + const reconnect = async () => { + const [startedUnder, startedAt] = [coordinator.connectionGeneration(), epoch]; + await hop("authorize"); + const creds = provider.authorize(); + const connect = () => coordinator.connect(creds, { ifGeneration: startedUnder }); + if (epoch !== startedAt) { + expect(connect).toThrow(ConnectionSupersededError); + refused.add(creds.access); + return; + } + if (refreshing > 0) reached.reconnectMidRefresh++; + connect(); + connected = provider.grantOf(creds.access); + epoch++; + }; + + const inFlight: Promise[] = []; + const timeline = (async () => { + for (const [at, step] of steps.entries()) { + await hop(`step ${at}: ${step.kind}`); + if (step.kind === "run") inFlight.push(run(step.chat, step.replayable)); + else if (step.kind === "reconnect") inFlight.push(reconnect()); + else if (step.kind === "expire") provider.expire(); + else if (step.kind === "staleBearer") provider.dropAccessTokens(); + else if (step.kind === "revoke") provider.revoke(); + else { // disconnect + coordinator.clear(); + connected = undefined; + epoch++; + } + } + await Promise.all(inFlight); + })(); + await s.waitFor(timeline); + + // With the access token expired, each chat's next operation has to refresh with what is stored. + provider.expire(); + const healthy = connected !== undefined && provider.live(connected); + for (const chat of [0, 1]) { + expect(await s.waitFor(run(chat, false)), `chat ${chat} runs afterwards`).toBe(healthy); + } + + for (const { replayable, attempts } of runs) { + expect(attempts.length, "times one operation ran").toBeLessThanOrEqual(replayable ? 2 : 1); + expect(new Set(attempts.map(provider.grantOf)).size, "grants one operation ran under") + .toBeLessThanOrEqual(1); + } + const stored = coordinator.stored(); + expect(stored && provider.grantOf(stored.access), "the stored grant").toBe(connected); + for (const { access } of provider.minted) { + const fates = [written, discarded, refused].filter(tokens => tokens.has(access)); + expect(fates, `fates of ${access}`).toHaveLength(1); + } + + reached.discardedMint += discarded.size; + reached.secondAttempt += runs.filter(({ attempts }) => attempts.length === 2).length; +} + +const on = (kind: Exclude): Step => ({ kind }); +const operation = (chat: 0 | 1, replayable: boolean): Step => ({ kind: "run", chat, replayable }); +// Operations outweigh the rest: the races need some in flight when the connection changes. +const chat = fc.constantFrom<0 | 1>(0, 1); +const step: fc.Arbitrary = fc.oneof( + { weight: 6, arbitrary: fc.tuple(chat, fc.boolean()).map(args => operation(...args)) }, + { weight: 3, arbitrary: fc.constant(on("expire")) }, + { weight: 2, arbitrary: fc.constant(on("staleBearer")) }, + { weight: 2, arbitrary: fc.constant(on("reconnect")) }, + { weight: 1, arbitrary: fc.constant(on("revoke")) }, + { weight: 1, arbitrary: fc.constant(on("disconnect")) }, +); + +// A read may be replayed; a write may not. +const read = (chat: 0 | 1) => operation(chat, true); +const write = (chat: 0 | 1) => operation(chat, false); +const workflows: Step[][] = [ + // The access token expires under both chats and the user reconnects while it refreshes. + [on("expire"), write(0), read(1), on("reconnect"), read(0), write(1)], + // The provider drops the access token early: both chats' calls are refused and have to heal. + [write(0), on("staleBearer"), read(0), read(1), write(1), write(0)], + // The user revokes the app at the provider, is told to reconnect, and does. + [read(0), on("revoke"), read(0), write(1), on("reconnect"), read(1), write(0)], + // The user disconnects during a refresh, then connects again. + [on("expire"), read(0), write(1), on("disconnect"), read(1), on("reconnect"), write(0)], +]; + +it("never replays an operation across connections or loses a token rotation", async () => { + const examples = workflows.flatMap(steps => fc.sample(fc.scheduler(), { seed: 0, numRuns: 10 }) + .map((schedule): [Step[], fc.Scheduler] => [steps, schedule])); + // The kit logs each heal a reconnect or disconnect overtook, hundreds of lines over the runs. + const logged = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await fc.assert( + fc.asyncProperty(fc.array(step, { minLength: 1, maxLength: 12 }), fc.scheduler(), replay), + { ...PROPERTY_RUN_OPTIONS, numRuns: 1000, examples }); + } finally { + logged.mockRestore(); + } + // A schedule property that never reaches its races proves little. + expect(Object.entries(reached).filter(([, times]) => times === 0)).toEqual([]); +}); diff --git a/packages/gatekeeper-kit/package.json b/packages/gatekeeper-kit/package.json index 005ea4ac4c..64e277f369 100644 --- a/packages/gatekeeper-kit/package.json +++ b/packages/gatekeeper-kit/package.json @@ -49,6 +49,7 @@ "@cloudflare/vitest-plugin": "catalog:", "@cloudflare/workers-types": "catalog:", "@gadgets/scripts": "workspace:*", + "fast-check": "^4.10.2", "typescript": "catalog:", "vitest": "catalog:" } diff --git a/packages/gatekeeper-kit/vite.config.ts b/packages/gatekeeper-kit/vite.config.ts index e58df07510..750303db8e 100644 --- a/packages/gatekeeper-kit/vite.config.ts +++ b/packages/gatekeeper-kit/vite.config.ts @@ -1,15 +1,23 @@ // Vite+ per-package settings. `test` runs two vitest projects -- pure logic in Node, Workers-API // modules in workerd -- kept as separate commands so one can replay from the task cache while the // other reruns. Tasks and scripts cannot share a name, so package.json declares neither name. -import { withVitestTask } from "@gadgets/scripts/vitest-task"; +import { vitestTask } from "@gadgets/scripts/vitest-task"; -export default withVitestTask({ +const testTask = vitestTask(["vitest run", "vitest run -c vitest.worker.config.ts"]); + +export default { run: { tasks: { // A pure typecheck: the root tsconfig sets `noEmit`, so automatic tracking sees only sources. build: { command: "tsc" }, // Uncached: a cache hit restores archived outputs but never deletes files. clean: { command: "rm -rf dist", cache: false }, + test: { + ...testTask, + // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it + // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. + cache: { ...testTask.cache, env: [...testTask.cache.env, "FAST_CHECK_SEED"] }, + }, }, }, -}, ["vitest run", "vitest run -c vitest.worker.config.ts"]); +}; diff --git a/packages/integration-tests/README.md b/packages/integration-tests/README.md index acd466aeb2..ed2b2bf8cd 100644 --- a/packages/integration-tests/README.md +++ b/packages/integration-tests/README.md @@ -57,7 +57,9 @@ small; `interruptAfterTimeLimit` bounds the shrink of a failing property inside (`@gadgets/scripts/property-runs`). workshop-backend's in-isolate properties (sharing, blueprint releases, streamed tool input, decisions on a turn's staged writes, two chats' changes to one gadget, scope-widening restarts, unread counts) share those options, and its `vitest.config.ts` -forwards `FAST_CHECK_SEED` into workerd. +forwards `FAST_CHECK_SEED` into workerd. gatekeeper-kit's two share the options as well and run +under Node: the action set against a model of its journal, and credentials under interleaved +refresh and reconnect. `FAST_CHECK_SEED` pins the seed of every property, so the same cases run each time. CI sets it, so a pull request's result depends only on its code. Without it, each run draws a fresh seed, but a diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d298f9e1cc..8c2d8b5eb4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -685,6 +685,9 @@ importers: '@gadgets/scripts': specifier: workspace:* version: link:../../scripts + fast-check: + specifier: ^4.10.2 + version: 4.10.2 typescript: specifier: 'catalog:' version: 7.0.2 @@ -3023,7 +3026,9 @@ packages: '@smithy/core@3.34.1': resolution: {integrity: sha512-dLcOUxz8YCv1RZUMKq6GbyUf95pLbrqh34bPvpCZ1+CByFF31BEAFewZjsGCnVsZTKdThNENfGyAgk2TJqVwSw==} engines: {node: '>=18.0.0'} - deprecated: Deprecated due to a bug in custom logger data redaction. See https://github.com/aws/aws-sdk-js-v3/issues/8393. + deprecated: |- + Deprecated due to a bug in custom logger data redaction. Upgrade to the latest version. If you used a custom logger with this + version, review your logs and rotate any credentials or other secrets. See https://github.com/aws/aws-sdk-js-v3/issues/8393. '@smithy/credential-provider-imds@4.4.16': resolution: {integrity: sha512-QfuLWAkLzptffFW980AFeHZFdqds2B64rpEd3uJ6lgs3xVn9QegGMUgUcj+4d7dRrAsya3r58ZKpku97WcFb4w==} diff --git a/scripts/env-passthrough.test.ts b/scripts/env-passthrough.test.ts index 86d04c3ffd..24b6dc4561 100644 --- a/scripts/env-passthrough.test.ts +++ b/scripts/env-passthrough.test.ts @@ -98,8 +98,8 @@ const EXPECTED: Record = { // never as vp tasks. // Every cached watchdogged task declares the timeout policy via TESTS_WITH_TIMEOUT_ENV; // vitest-task.test.ts pins the shared declaration. - // `property-runs.ts` reads FAST_CHECK_SEED inside the `test` tasks of integration-tests and - // workshop-backend, each of which declares it. + // `property-runs.ts` reads FAST_CHECK_SEED inside the `test` tasks of integration-tests, + // workshop-backend and gatekeeper-kit, each of which declares it. scripts: { forwarded: [ "FAST_CHECK_SEED", From a9354b8b1c5d84e9fbc87e7ce5d70cc3631446a8 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sun, 8 Nov 2026 15:05:09 -0600 Subject: [PATCH 08/12] Bring the observer re-verification property up to shared observer identities Since #1001 an admission reserves the collaborator's observer id before registering anywhere, and a failed one keeps every registration. The property still modelled the rules it replaced: a failed first-ever open rolling back under a discarded id, and a later open minting a new one. It now checks that each verified pass registers at its binding, that nothing is ever removed, and that every registration names the one observer id. The prompt and error checks are unchanged. --- .../__tests__/observer-reverification.test.ts | 59 +++++++------------ 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/packages/integration-tests/__tests__/observer-reverification.test.ts b/packages/integration-tests/__tests__/observer-reverification.test.ts index b65e276500..8be9516a85 100644 --- a/packages/integration-tests/__tests__/observer-reverification.test.ts +++ b/packages/integration-tests/__tests__/observer-reverification.test.ts @@ -386,12 +386,12 @@ describe("observer re-verification", () => { // --------------------------------------------------------------------------------------------- // A collaborator's opens over time, while each connection's credential lapses or is repaired -// between opens and during the one re-prompt. Checked against ensureObserver()'s documented rules: -// a first open prompts for every binding; every pass verifies every binding; only failed bindings -// are re-prompted, once, with their failure; a terminal error names exactly the bindings still -// failing; a failed first-ever open removes everything it registered, under its own discarded -// observer id; a returning observer's failed open removes nothing (0d7793c); and a successful open -// persists the observer id every later open reuses. +// between opens and during the one re-prompt. Checked against ensureObserver()'s documented rules +// (docs/observers.md): until an open succeeds, each one prompts for every binding; every pass +// verifies every binding; only failed bindings are re-prompted, once, with their failure; a +// terminal error names exactly the bindings still failing; no failed open de-registers anything, +// a first-ever one included; and every open registers under the one observer id the first +// reserved. /** One open: per binding, whether its credential verifies on the first pass, then after a repair. */ type ObserverOpen = { firstPass: boolean[]; afterReprompt: boolean[] }; @@ -404,11 +404,8 @@ const observerOpens: fc.Arbitrary = fc.integer({ min: 1, max: 3 afterReprompt: fc.array(verifies, { minLength: bindings, maxLength: bindings }), }), { minLength: 1, maxLength: 3 })); -/** An event as the model predicts it; `observer` is the open that minted the observer id. */ -type ExpectedEvent = { type: "add" | "remove"; observer: number }; - describe("observer re-verification over a collaborator's opens", () => { - it.concurrent("registers, re-prompts, rolls back and reports exactly as the bindings verify", + it.concurrent("registers, re-prompts and reports exactly as the bindings verify, removing nothing", async () => { await fc.assert(fc.asyncProperty(observerOpens, async opens => withSession(async publicApi => { const [prefix] = nextUsernames("obsprop"); @@ -420,13 +417,12 @@ describe("observer re-verification over a collaborator's opens", () => { setVerifyOutcome(shared.bobLabel, allow ? { allow: true } : { allow: false, reason: EXPIRED_REASON }, urls[i]))); - // The open whose observer id is persisted; undefined until one succeeds. - let persisted: number | undefined; - const expected: ExpectedEvent[][] = urls.map(() => []); + // Whether an earlier open succeeded, which is what publishes Bob's account choices. + let returning = false; + // How many passes verified each binding; each registers Bob there. + const registrations = urls.map(() => 0); for (const [k, { firstPass, afterReprompt }] of opens.entries()) { - const returning = persisted !== undefined; - const observer = persisted ?? k; await setOutcomes(firstPass); const recorder = new ObserverConfigRecorder(); if (!returning) recorder.alwaysChoose(bob, 1); @@ -439,9 +435,7 @@ describe("observer re-verification over a collaborator's opens", () => { (error: unknown) => error as Error); const passes = firstPass.every(Boolean) ? [firstPass] : [firstPass, afterReprompt]; - for (const pass of passes) { - pass.forEach((ok, i) => { if (ok) expected[i]!.push({ type: "add", observer }); }); - } + for (const pass of passes) pass.forEach((ok, i) => { if (ok) registrations[i]!++; }); const failing = names.filter((_, i) => !passes.at(-1)![i]); const prompts = recorder.calls.map(needs => needs @@ -458,36 +452,26 @@ describe("observer re-verification over a collaborator's opens", () => { if (failing.length === 0) { expect(opened, `open ${k}`).toBeNull(); - persisted = observer; + returning = true; } else { expect(opened?.message, `open ${k}`).toMatch(/could not confirm/i); const named = opened!.message.split("\n") .filter(line => line.includes(shared.bobLabel)) .map(line => /^Test Thing (\S+) \(/.exec(line)?.[1]); expect(named.toSorted(), `bindings named by open ${k}`).toEqual(failing.toSorted()); - if (!returning) { - for (const events of expected) events.push({ type: "remove", observer }); - } } } - // Every event names an observer id: the same id for the same open's observer, and a - // different one for each other. A binding that registered on one pass and failed on the next - // is removed once for each (`newlyAdded` and `invalidated` both hold it); removal is - // idempotent, so a repeat is collapsed rather than pinned. - const idOf = new Map(); + // Each verified pass registered Bob at its binding, nothing removed him, and every + // registration names the one observer id. + const ids = new Set(); for (const [i, url] of urls.entries()) { - const events = (await observerEvents(url)).filter((event, j, all) => - event.type !== "remove" || all[j - 1]?.type !== "remove" || all[j - 1]?.id !== event.id); + const events = await observerEvents(url); expect(events.map(({ type }) => type), `events at ${names[i]}`) - .toEqual(expected[i]!.map(({ type }) => type)); - for (const [j, { id }] of events.entries()) { - const { observer } = expected[i]![j]!; - expect(idOf.get(observer) ?? id, `observer id of open ${observer}`).toBe(id); - idOf.set(observer, id); - } + .toEqual(Array.from({ length: registrations[i]! }, () => "add")); + for (const { id } of events) ids.add(id); } - expect(new Set(idOf.values()).size, "distinct observer ids").toBe(idOf.size); + expect(ids.size, "distinct observer ids").toBeLessThanOrEqual(1); })), { ...PROPERTY_RUN_OPTIONS, numRuns: 8, @@ -495,7 +479,8 @@ describe("observer re-verification over a collaborator's opens", () => { // The 0d7793c reproduction: a lapses and is repaired as b lapses for good. [[{ firstPass: [true, true], afterReprompt: [true, true] }, { firstPass: [false, true], afterReprompt: [true, false] }]], - // A first-ever open fails terminally and rolls back; a later one starts afresh. + // A first-ever open fails terminally and keeps what it registered; the next succeeds under + // the same observer id. [[{ firstPass: [false, true], afterReprompt: [false, true] }, { firstPass: [true, true], afterReprompt: [true, true] }]], ], From 083586c8f462190a96a4ca2e5ff6553fbb1aa0b5 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sun, 8 Nov 2026 15:13:21 -0600 Subject: [PATCH 09/12] Give gatekeeper-kit's two properties an explicit test timeout Both ran under vitest's 5s default. The action-set property takes 2-3s locally and took 4.7s and then 9.7s on two CI runners, so the second run timed out. They now get the 120s the workshop-backend properties have, which also leaves a failing run fast-check's 90s to shrink. --- packages/gatekeeper-kit/__tests__/actions-properties.test.ts | 2 +- .../gatekeeper-kit/__tests__/credentials-properties.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/gatekeeper-kit/__tests__/actions-properties.test.ts b/packages/gatekeeper-kit/__tests__/actions-properties.test.ts index 8139038f17..6f9df48e41 100644 --- a/packages/gatekeeper-kit/__tests__/actions-properties.test.ts +++ b/packages/gatekeeper-kit/__tests__/actions-properties.test.ts @@ -295,4 +295,4 @@ it("matches a model of the queue after every submission, approval and denial", a } finally { logged.mockRestore(); } -}); +}, 120_000); diff --git a/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts b/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts index 2f8bb0b53d..553b9cd64a 100644 --- a/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts +++ b/packages/gatekeeper-kit/__tests__/credentials-properties.test.ts @@ -254,4 +254,4 @@ it("never replays an operation across connections or loses a token rotation", as } // A schedule property that never reaches its races proves little. expect(Object.entries(reached).filter(([, times]) => times === 0)).toEqual([]); -}); +}, 120_000); From c377b93486d45f0462c28312bfdb730d13a94b10 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 9 Nov 2026 14:30:06 -0600 Subject: [PATCH 10/12] Enable hooks through the owner's interface in the scope-widening property The property flipped the hook record itself, because the test Worker did not export the loopback entrypoint enableHook() hands the gatekeeper. It is now exported beside the other loopbacks, and the step calls enableHook() as the Connections tab does, so the property also fails if that method stops reaching the scope check after its gatekeeper round trip. --- .../__tests__/observer-scope-restart.test.ts | 5 +---- packages/workshop-backend/__tests__/test-worker.ts | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts index 58736567e5..b939e9bc01 100644 --- a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts +++ b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts @@ -1639,10 +1639,7 @@ async function replayOwnerSteps(steps: Step[], from = 0): Promise { case "enableHook": case "disableHook": case "deleteHook": { let id = pick([...model.hooks.keys()], step.hook); if (id === undefined) break; - // (enableHook's loopback entrypoint is not among the test Worker's exports either; this - // is the record flip its gatekeeper round trip ends on.) - if (step.kind === "enableHook") impl.enableHookRecord(hookRecords.get(id)); - else await owner[step.kind](id); + await owner[step.kind](id); if (step.kind === "deleteHook") model.hooks.delete(id); else model.hooks.get(id)!.enabled = step.kind === "enableHook"; break; diff --git a/packages/workshop-backend/__tests__/test-worker.ts b/packages/workshop-backend/__tests__/test-worker.ts index fa002f7596..aec3fff101 100644 --- a/packages/workshop-backend/__tests__/test-worker.ts +++ b/packages/workshop-backend/__tests__/test-worker.ts @@ -57,6 +57,8 @@ export { AgentSelfLoopback } from "../src/server.js"; export { GatekeeperLoopback, GadgetTailLoopback } from "../src/server.js"; /** What an executeCode run's tail worker is minted from, likewise. */ export { CodeModeTailLoopback } from "../src/server.js"; +/** What an enabled hook delivers its events through, likewise. */ +export { GatekeeperHookLoopback } from "../src/server.js"; /** * The Workshop's connect callback, reachable through `ctx.exports`: the pool derives those from this * module's own declarations, so an entrypoint a test reaches that way has to be named here rather From 8424c77cd535e07c9b86ff9b7174cad010064d60 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 9 Nov 2026 15:13:13 -0600 Subject: [PATCH 11/12] Copy hook records in the scope property's fake, and quiet the turn-resume property The scope-widening property's hook table handed out the stored objects, so a change to a record persisted without a put(): the property passed with the write removed from enableHookRecord() or disableHook(). It now copies records in and out, as real storage does, and fails either way. The turn-resume property logged every generated turn, about 45% of the lines in workshop-backend's CI test log. It now silences the console for its duration, as the chat and unread-count properties do. --- packages/gatekeeper-kit/vite.config.ts | 14 +++----------- packages/integration-tests/vite.config.ts | 13 ++++--------- .../__tests__/durable-decision-resume.test.ts | 5 +++++ .../__tests__/observer-scope-restart.test.ts | 7 ++++--- packages/workshop-backend/vite.config.ts | 13 ++++--------- 5 files changed, 20 insertions(+), 32 deletions(-) diff --git a/packages/gatekeeper-kit/vite.config.ts b/packages/gatekeeper-kit/vite.config.ts index 750303db8e..e58df07510 100644 --- a/packages/gatekeeper-kit/vite.config.ts +++ b/packages/gatekeeper-kit/vite.config.ts @@ -1,23 +1,15 @@ // Vite+ per-package settings. `test` runs two vitest projects -- pure logic in Node, Workers-API // modules in workerd -- kept as separate commands so one can replay from the task cache while the // other reruns. Tasks and scripts cannot share a name, so package.json declares neither name. -import { vitestTask } from "@gadgets/scripts/vitest-task"; +import { withVitestTask } from "@gadgets/scripts/vitest-task"; -const testTask = vitestTask(["vitest run", "vitest run -c vitest.worker.config.ts"]); - -export default { +export default withVitestTask({ run: { tasks: { // A pure typecheck: the root tsconfig sets `noEmit`, so automatic tracking sees only sources. build: { command: "tsc" }, // Uncached: a cache hit restores archived outputs but never deletes files. clean: { command: "rm -rf dist", cache: false }, - test: { - ...testTask, - // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it - // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. - cache: { ...testTask.cache, env: [...testTask.cache.env, "FAST_CHECK_SEED"] }, - }, }, }, -}; +}, ["vitest run", "vitest run -c vitest.worker.config.ts"]); diff --git a/packages/integration-tests/vite.config.ts b/packages/integration-tests/vite.config.ts index ca9d218a83..486d71d623 100644 --- a/packages/integration-tests/vite.config.ts +++ b/packages/integration-tests/vite.config.ts @@ -5,11 +5,6 @@ import { WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, } from '@gadgets/scripts/vitest-task' -const testTask = vitestTaskWithExclusions('vitest run', [ - ...VITEST_TOOL_SCRATCH_EXCLUSIONS, - ...WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, -]) - export default { run: { tasks: { @@ -51,10 +46,10 @@ export default { * does write: `tmp/` (per-boot bundles) and `state/` (local DO/KV storage). */ test: { - ...testTask, - // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it - // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. - cache: { ...testTask.cache, env: [...testTask.cache.env, 'FAST_CHECK_SEED'] }, + ...vitestTaskWithExclusions('vitest run', [ + ...VITEST_TOOL_SCRATCH_EXCLUSIONS, + ...WRANGLER_RUNTIME_SCRATCH_EXCLUSIONS, + ]), // Real gatekeepers boot from prebuilt validated trees, tracked as inputs like the fixture, // plus the release the Google upgrade suite starts from, and `cli-admins-only.test.ts` runs // `cf-os api`, whose command table is generated. diff --git a/packages/workshop-backend/__tests__/durable-decision-resume.test.ts b/packages/workshop-backend/__tests__/durable-decision-resume.test.ts index 8d20d9f468..fa06747ffb 100644 --- a/packages/workshop-backend/__tests__/durable-decision-resume.test.ts +++ b/packages/workshop-backend/__tests__/durable-decision-resume.test.ts @@ -993,6 +993,11 @@ describe("decisions on a turn's staged writes, for generated sessions", () => { ]; it("resumes a suspended turn once, when the last write it waits on is approved", async () => { + // Every generated turn logs a screenful, failures of the faked services and of the applies + // generated to fail included, which would bury a failure's report. + for (const level of ["debug", "info", "warn", "error"] as const) { + vi.spyOn(console, level).mockImplementation(() => {}); + } await fc.assert(fc.asyncProperty(sessions, replayDecisions), {...PROPERTY_RUN_OPTIONS, numRuns: 50, examples}); }, 120_000); diff --git a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts index b939e9bc01..baf7b13fb6 100644 --- a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts +++ b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts @@ -1532,12 +1532,13 @@ async function replayOwnerSteps(steps: Step[], from = 0): Promise { }); // A live hook controller can't be persisted through the real storage, as above. let controller = { enable: async () => {}, disable: async () => {} }; + // Records are copied in and out, as real storage does: a change takes a put() to persist. let hookRecords = new Map(); impl.storage.boundHooks = { - get: (id: number) => hookRecords.get(id), - put: (record: { id: number }) => hookRecords.set(record.id, record), + get: (id: number) => { let record = hookRecords.get(id); return record && { ...record }; }, + put: (record: { id: number }) => hookRecords.set(record.id, { ...record }), delete: (id: number) => hookRecords.delete(id), - list: () => [...hookRecords.values()], + list: () => [...hookRecords.values()].map(record => ({ ...record })), }; for (let id of GADGETS) seedGadget(impl, id); for (let id of CHATS) { diff --git a/packages/workshop-backend/vite.config.ts b/packages/workshop-backend/vite.config.ts index f32f4c2ff4..b3ef5899fc 100644 --- a/packages/workshop-backend/vite.config.ts +++ b/packages/workshop-backend/vite.config.ts @@ -4,11 +4,6 @@ import { TESTS_WITH_TIMEOUT_ENV, vitestTask, withTestTimeout, } from '@gadgets/scripts/vitest-task' -const unitTask = vitestTask([ - { command: 'vitest run', idleSeconds: 120 }, - 'vitest run --config vitest.integration.config.ts', -]) - /** * Codegen steps stay separate commands rather than one `&&` string so each caches on its own. */ @@ -86,10 +81,10 @@ export default { * Import-heavy unit tests need a longer idle allowance; integration uses the shared defaults. */ test: { - ...unitTask, - // The property suites' seed (@gadgets/scripts/property-runs) decides which cases run, so it - // is passed and fingerprinted: a different seed is a cache miss, not a replayed result. - cache: { ...unitTask.cache, env: [...unitTask.cache.env, 'FAST_CHECK_SEED'] }, + ...vitestTask([ + { command: 'vitest run', idleSeconds: 120 }, + 'vitest run --config vitest.integration.config.ts', + ]), dependsOn: [ '@gadgets/typed-storage#build', 'build:bundled-blueprints', 'build:browser-runtime', ], From f4d997fd4cb1b0cd8e3762db2db01a67c998c2e8 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 9 Nov 2026 15:13:13 -0600 Subject: [PATCH 12/12] Declare the property seed on every generated test task Three packages each added FAST_CHECK_SEED to their own test task's cache.env, and nothing checked that two of them did: without it a cached run strips the seed, so CI would run that suite on a fresh seed with no sign of it. The shared vitest task now declares the seed, vitest-task.test.ts pins that, and the env-passthrough guard counts a generated task's shared list as declared. The three per-package overrides are gone. Also names gatekeeper-kit in property-runs.ts, and moves fast-check to a catalog entry so the three packages stay on one version. --- packages/gatekeeper-kit/package.json | 2 +- packages/integration-tests/package.json | 2 +- packages/workshop-backend/package.json | 2 +- pnpm-lock.yaml | 9 ++++++--- pnpm-workspace.yaml | 2 ++ scripts/env-passthrough.test.ts | 11 ++++++++--- scripts/property-runs.ts | 6 +++--- scripts/vitest-task-vite-config.ts | 9 ++++++++- scripts/vitest-task.test.ts | 7 +++++++ 9 files changed, 37 insertions(+), 13 deletions(-) diff --git a/packages/gatekeeper-kit/package.json b/packages/gatekeeper-kit/package.json index 64e277f369..11ca06a54b 100644 --- a/packages/gatekeeper-kit/package.json +++ b/packages/gatekeeper-kit/package.json @@ -49,7 +49,7 @@ "@cloudflare/vitest-plugin": "catalog:", "@cloudflare/workers-types": "catalog:", "@gadgets/scripts": "workspace:*", - "fast-check": "^4.10.2", + "fast-check": "catalog:", "typescript": "catalog:", "vitest": "catalog:" } diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 4c4fc22663..b6e3c015f1 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -28,7 +28,7 @@ "@gadgets/scripts": "workspace:*", "@types/node": "^26.1.0", "capnweb-validate": "catalog:", - "fast-check": "^4.10.2", + "fast-check": "catalog:", "typescript": "catalog:", "vitest": "catalog:", "wrangler": "catalog:" diff --git a/packages/workshop-backend/package.json b/packages/workshop-backend/package.json index 873ef26b08..53c7b93192 100644 --- a/packages/workshop-backend/package.json +++ b/packages/workshop-backend/package.json @@ -36,7 +36,7 @@ "@gadgets/scripts": "workspace:*", "dompurify": "^3.4.17", "esbuild": "^0.28.2", - "fast-check": "^4.10.2", + "fast-check": "catalog:", "typescript": "catalog:", "vitest": "catalog:", "wrangler": "catalog:" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8c2d8b5eb4..edd562f494 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,6 +24,9 @@ catalogs: diff: specifier: ^9.0.0 version: 9.0.0 + fast-check: + specifier: ^4.10.2 + version: 4.10.2 miniflare: specifier: 5.20261023.1-beta version: 5.20261023.1-beta @@ -686,7 +689,7 @@ importers: specifier: workspace:* version: link:../../scripts fast-check: - specifier: ^4.10.2 + specifier: 'catalog:' version: 4.10.2 typescript: specifier: 'catalog:' @@ -1212,7 +1215,7 @@ importers: specifier: 'catalog:' version: 0.3.0(capnweb@0.12.0)(esbuild@0.28.2)(rollup@4.62.3)(vite@7.3.6(@types/node@26.1.0)(jiti@2.7.0)(lightningcss@1.33.0)(terser@5.51.2)(yaml@2.9.1)) fast-check: - specifier: ^4.10.2 + specifier: 'catalog:' version: 4.10.2 typescript: specifier: 'catalog:' @@ -1414,7 +1417,7 @@ importers: specifier: ^0.28.2 version: 0.28.2 fast-check: - specifier: ^4.10.2 + specifier: 'catalog:' version: 4.10.2 typescript: specifier: 'catalog:' diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9548d04eee..e72cbfea66 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -14,6 +14,8 @@ catalog: # simulated PR/MR diff's hunks, used by the git-host gatekeepers). One catalog entry so the two # can't drift apart. diff: ^9.0.0 + # The property suites' generator (workshop-backend, integration-tests, gatekeeper-kit). + fast-check: ^4.10.2 # Exact: pinned in lockstep with capnweb (declared as a >=0.7.0 peer) capnweb-validate: 0.3.0 # Exact: a prerelease, kept in step with the Wrangler/workerd stack the test pool runs on. diff --git a/scripts/env-passthrough.test.ts b/scripts/env-passthrough.test.ts index 24b6dc4561..27ba4fd593 100644 --- a/scripts/env-passthrough.test.ts +++ b/scripts/env-passthrough.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { readdirSync, readFileSync, statSync } from "node:fs"; import { join } from "node:path"; import { describe, it } from "node:test"; +import { vitestTask } from "./vitest-task-vite-config.ts"; /** * A cached `vp` run executes each task in a clean environment: only a built-in set (`PATH`, `HOME`, @@ -87,7 +88,7 @@ const EXPECTED: Record = { injected: ["NODE_ENV"], }, // `vitest.config.ts` forwards FAST_CHECK_SEED into workerd for the property suites; the `test` - // task declares it. + // task declares it, as every generated test task does. "packages/workshop-backend": { forwarded: ["FAST_CHECK_SEED"], uncached: ["BUNDLED_BLUEPRINTS_DIR"], @@ -98,8 +99,8 @@ const EXPECTED: Record = { // never as vp tasks. // Every cached watchdogged task declares the timeout policy via TESTS_WITH_TIMEOUT_ENV; // vitest-task.test.ts pins the shared declaration. - // `property-runs.ts` reads FAST_CHECK_SEED inside the `test` tasks of integration-tests, - // workshop-backend and gatekeeper-kit, each of which declares it. + // `property-runs.ts` reads FAST_CHECK_SEED inside the `test` task of whichever package runs a + // property suite. Every generated test task declares it, which vitest-task.test.ts pins too. scripts: { forwarded: [ "FAST_CHECK_SEED", @@ -196,6 +197,10 @@ function declarationsIn(directory: string): { patterns: Set; hasUncached for (const file of readdirSync(directory).filter(name => /^vite.*\.config\.ts$/.test(name))) { const source = stripComments(readFileSync(join(directory, file), "utf8")); if (/cache:\s*false/.test(source)) hasUncachedTask = true; + // A generated test task declares the shared list, which is not spelled out in the config. + if (/\b(vitestTask\w*|withVitestTask)\(/.test(source)) { + for (const name of vitestTask("vitest run").cache.env) patterns.add(name); + } for (const block of source.matchAll(/env:\s*\[([^\]]*)\]/g)) { for (const entry of block[1].matchAll(/["']([A-Z0-9_*]+)["']/g)) patterns.add(entry[1]); } diff --git a/scripts/property-runs.ts b/scripts/property-runs.ts index dedebb681a..313caa807a 100644 --- a/scripts/property-runs.ts +++ b/scripts/property-runs.ts @@ -1,6 +1,6 @@ -// fast-check options shared by the property-based suites: integration-tests' under Node, and -// workshop-backend's inside workerd, whose vitest config forwards `FAST_CHECK_SEED` as a binding -// that `nodejs_compat` copies into `process.env`. +// fast-check options shared by the property-based suites: integration-tests' and gatekeeper-kit's +// under Node, and workshop-backend's inside workerd, whose vitest config forwards +// `FAST_CHECK_SEED` as a binding that `nodejs_compat` copies into `process.env`. /** * `FAST_CHECK_SEED` pins every property's seed, so a run generates the same cases each time; CI diff --git a/scripts/vitest-task-vite-config.ts b/scripts/vitest-task-vite-config.ts index bc1adf35c3..fe79f656b3 100644 --- a/scripts/vitest-task-vite-config.ts +++ b/scripts/vitest-task-vite-config.ts @@ -166,6 +166,13 @@ export const TESTS_WITH_TIMEOUT_ENV: string[] = [ 'TESTS_WITH_TIMEOUT_MAX_SECONDS', ] +/** + * Pins the cases the property suites generate (`property-runs.ts`). Every generated test task + * passes and fingerprints it, so a suite that adopts fast-check needs no wiring of its own, and a + * different seed is a cache miss rather than a replayed result. + */ +const PROPERTY_SEED_ENV = 'FAST_CHECK_SEED' + /** * The worker budget `scripts/vp/run.ts` sets (see `scripts/vp/concurrency.ts`). Defined here * because every package's task graph loads this module, which must not pull in that one. @@ -257,7 +264,7 @@ export function vitestTaskWithExclusions( cache: { input: [{ auto: true }, ...exclusions], output: [{ auto: true }, ...exclusions], - env: TESTS_WITH_TIMEOUT_ENV, + env: [...TESTS_WITH_TIMEOUT_ENV, PROPERTY_SEED_ENV], untrackedEnv: pinned ? [] : VITEST_WORKERS_ENV, }, } diff --git a/scripts/vitest-task.test.ts b/scripts/vitest-task.test.ts index f467140196..1174ca226a 100644 --- a/scripts/vitest-task.test.ts +++ b/scripts/vitest-task.test.ts @@ -140,6 +140,13 @@ describe("the watchdog environment policy", () => { } }); + // What `property-runs.ts` reads. A suite whose task dropped it would run a fresh seed in CI. + it("is joined by the property suites' seed on every generated test task", () => { + for (const task of [vitestTask("vitest run"), vitestTaskWithExclusions("vitest run", [])]) { + assert.ok(task.cache.env.includes("FAST_CHECK_SEED")); + } + }); + it("reaches every hand-declared cached task that wraps the watchdog", async () => { let seen = 0; for (const config of HAND_DECLARED_CONFIGS) {