diff --git a/.changeset/clean-tabs-snapshot.md b/.changeset/clean-tabs-snapshot.md new file mode 100644 index 0000000..35b9d9a --- /dev/null +++ b/.changeset/clean-tabs-snapshot.md @@ -0,0 +1,22 @@ +--- +"@understudy/protocol": minor +"@understudy/connector": minor +--- + +Bind every snapshot and accessibility ref to the exact browser target that +produced it. + +- `@understudy/protocol`: `snapshot_result` and `screenshot_result` now require + the attached CDP `tabId` and main-frame `url`. Accessibility refs remain + opaque, but are now namespaced to the extension session, CDP attachment, and + snapshot generation so a ref cannot resolve against a replacement browser + connection or tab. +- `@understudy/connector`: snapshot reads expose `target: { tabId, url }`. + Driver failures, including expected-tab mismatches and pages that change + during capture, return structured `{ ok: false, error }` output with the + original reason. + +This is a breaking wire change. Upgrade and deploy the protocol, service, +extension, and connector together. Protocol 0.6 rejects snapshot events from +older extensions because they lack the required target fields, and connector +0.4 requires protocol 0.6. diff --git a/README.md b/README.md index 9078e4f..509073e 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ drive it over HTTP (Topology 1). no LLM and embeds no agent framework — the brain and governance (breakwater/flowsafe) live in the consumers. See its README. -M4 status: `@understudy/protocol@0.5.0` and `@understudy/connector@0.3.0` are +M4 status: `@understudy/protocol@0.6.0` and `@understudy/connector@0.4.0` are published on npm. Metamind contains the cross-repository Mastra workflow, flowsafe approval gate, breakwater browser connectors, and attended runbook. The remaining M4 step is running that proof with a connected Chromium diff --git a/apps/backend/README.md b/apps/backend/README.md index 9257600..7ab6841 100644 --- a/apps/backend/README.md +++ b/apps/backend/README.md @@ -35,7 +35,7 @@ route (internally the coordinator still rejects with the `src/coordinator.ts` prefix constants; `SessionAgent.dispatchFailure` maps them in-isolate). The route maps reasons to statuses: **503** `{error: "extension not connected"}` when the session has no live, -onConnect-authorized extension socket — the gate consults that delivery +authoritative onConnect-authorized extension socket — the gate consults that delivery predicate directly (not the persisted `status` scalar), fails fast instead of burning the 30s timeout, and answers non-2xx deliberately: a 200 `ok:false` Event would be cached by a consumer's idempotency store and replayed after @@ -86,17 +86,22 @@ all. The in-DO gate remains as defense in depth for any path that reaches the DO without that router: `onConnect` runs the same async checks (`verifyExtensionToken` + `scopeSession`) before marking a connection -`connection.setState({ authorized: true })`, closing with 1008 otherwise. +`connection.setState({ authorized: true })`, closing with 1008 otherwise. Once +authenticated, the newest socket becomes the session's sole authority through +persisted `SessionState.activeConnectionId`; prior authorized sockets are +demoted and closed with 4001 (`"replaced by newer extension connection"`). The Agents SDK accepts a socket — and admits it to the connection set `getConnections()` returns — before that async check resolves, so an unauthenticated or wrong-tenant socket could sit in the connection set during that window. Four things close this gap: -- `sendToExtension` (the coordinator's outbound path) iterates - `getConnections()` but filters to `isAuthorizedConnection`, so a command is - never written to a socket still pending auth. -- `onMessage` returns immediately for any connection that isn't yet authorized, - so an unauthenticated socket cannot inject events. +- `sendToExtension` (the coordinator's outbound path) resolves exactly the + authorized socket named by `activeConnectionId`, so a command is never + broadcast, sent to a socket still pending auth, or duplicated during a + reconnect overlap. +- `onMessage` returns immediately unless its sender is that same authoritative, + authorized socket, so neither an unauthenticated nor a replaced socket can + inject events or resolve another socket's command. - `shouldSendProtocolMessages` returns `false` unconditionally, suppressing the SDK's own connect-time protocol frames — the extension speaks only the `@understudy/protocol` wire shape and already discards anything else, so this @@ -106,6 +111,13 @@ during that window. Four things close this gap: any accepted connection, including one still awaiting auth, and this DO's state is server-driven only. +Persisted sessions created before `activeConnectionId` existed migrate lazily +only when exactly one authorized socket is live. Multiple legacy candidates are +ambiguous and fail closed until a newly authenticated socket claims authority; +the service never falls back to the former broadcast behavior. Closing a +replaced socket cannot detach its replacement because `onClose` clears status +only when the closing connection owns the persisted authority. + ## Design decisions - **Per-session DO, not per-user**: a user can have multiple concurrent @@ -232,6 +244,10 @@ during that window. Four things close this gap: - One Durable Object per `sessionId`; a sessionId whose embedded tenant disagrees with the authenticated caller is refused with 404, never 403 — on the /v1 API and on the agent WS/HTTP path alike. +- Exactly one authenticated extension socket is authoritative per session. + Commands and Events use only its persisted `activeConnectionId`; a newer + authenticated socket atomically replaces and closes prior sockets, and a + late predecessor close cannot detach the replacement. - A mid-command DO hibernation cannot happen (see above); an interrupting shutdown/restart is bounded by the per-command timeout, and the persisted awaiting-marker reconciles any orphaned late result rather than mis-resolving it. diff --git a/apps/backend/scripts/stub-consumer.mjs b/apps/backend/scripts/stub-consumer.mjs index 67747e8..d80d625 100644 --- a/apps/backend/scripts/stub-consumer.mjs +++ b/apps/backend/scripts/stub-consumer.mjs @@ -17,12 +17,13 @@ * 3. Load the M2 extension (apps/extension) in a real, logged-in * Chromium and connect it to that printed WS URL. * 4. Press Enter in this terminal once the extension shows connected. - * 5. Watch snapshot -> type -> click -> fill_secret drive the page; each + * 5. Confirm the snapshot's tabId and URL before the script uses its refs. + * 6. Watch snapshot -> type -> click -> fill_secret drive the page; each * returned Event is printed as it comes back. * - * Without the extension connected, every command below will time out (the - * service's per-command timeout - 30s by default) because nothing ever - * answers the forwarded command. The `POST /v1/sessions` step (through + * Without an authoritative extension connected, every command below fails + * fast with HTTP 503 (`extension not connected`) instead of waiting for the + * service's per-command timeout. The `POST /v1/sessions` step (through * printing the WS URL) is verifiable standalone, with no extension attached. * * `type` and `click` target the first ref found in the snapshot's a11y tree @@ -102,6 +103,33 @@ function findFirstRef(nodes) { return undefined; } +function snapshotTarget(event) { + if (event.type !== "snapshot_result") { + throw new Error(`snapshot failed: ${JSON.stringify(event)}`); + } + if (!Number.isInteger(event.tabId) || event.tabId < 0) { + throw new Error(`snapshot returned an invalid tabId: ${JSON.stringify(event.tabId)}`); + } + if (typeof event.url !== "string" || event.url.length === 0) { + throw new Error(`snapshot returned an invalid URL: ${JSON.stringify(event.url)}`); + } + return { tabId: event.tabId, url: event.url }; +} + +async function confirmSnapshotTarget(target) { + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer = await rl.question( + `Confirm snapshot target tabId=${target.tabId}, url=${JSON.stringify(target.url)} [y/N]: `, + ); + if (!["y", "yes"].includes(answer.trim().toLowerCase())) { + throw new Error("snapshot target was not confirmed; refusing to use its refs"); + } + } finally { + rl.close(); + } +} + async function main() { console.log("understudy stub consumer - M3 runbook harness"); console.log(`BASE_URL=${BASE_URL}`); @@ -124,7 +152,8 @@ async function main() { commandId: "stub-1", mode: "a11y", }); - const ref = snapshotEvent.type === "snapshot_result" ? findFirstRef(snapshotEvent.tree) : undefined; + await confirmSnapshotTarget(snapshotTarget(snapshotEvent)); + const ref = findFirstRef(snapshotEvent.tree); if (!ref) { console.log("(no ref found in the snapshot - type/click below will return ok:false)"); } diff --git a/apps/backend/src/coordinator-cf.ts b/apps/backend/src/coordinator-cf.ts index e22e5cc..a992420 100644 --- a/apps/backend/src/coordinator-cf.ts +++ b/apps/backend/src/coordinator-cf.ts @@ -21,15 +21,16 @@ const DEFAULT_TIMEOUT_MS = 30_000; * imports session.ts or `agents` directly. */ export interface CoordinatorHost { - /** Writes a JSON frame to the live extension WebSocket. */ + /** Writes a JSON frame to the session's one authoritative extension WebSocket. */ sendToExtension(payload: string): void; /** - * The delivery predicate: does a live, onConnect-authorized extension - * socket exist right now? This is the exact precondition sendToExtension - * relies on - NOT the persisted SessionState.status scalar, which is a - * last-writer-wins echo maintained by lifecycle hooks (onClose guards the - * known stamping races, but the scalar is still eventually-consistent - * bookkeeping, not the delivery truth). + * The delivery predicate: does the persisted authority identify a live, + * onConnect-authorized extension socket right now? This is the exact + * precondition sendToExtension relies on - NOT the persisted + * SessionState.status scalar, which is a last-writer-wins echo maintained + * by lifecycle hooks (onClose guards the known stamping races, but the + * scalar is still eventually-consistent bookkeeping, not the delivery + * truth). */ hasAuthorizedConnection(): boolean; /** Reads the persisted awaiting-commandId marker (SessionState.awaitingCommandIds). */ @@ -112,7 +113,18 @@ export class CfSessionCoordinator implements SessionCoordinator { // log, by construction. console.log("coordinator.send", { commandId: cmd.commandId, type: cmd.type }); - this.host.sendToExtension(JSON.stringify(cmd)); + try { + this.host.sendToExtension(JSON.stringify(cmd)); + } catch { + clearTimeout(timer); + this.pending.delete(cmd.commandId); + this.dropAwaiting(cmd.commandId); + reject( + new Error( + `${SESSION_NOT_CONNECTED}: authoritative extension connection unavailable during send`, + ), + ); + } }); } diff --git a/apps/backend/src/session.ts b/apps/backend/src/session.ts index 5a3e482..6747643 100644 --- a/apps/backend/src/session.ts +++ b/apps/backend/src/session.ts @@ -33,6 +33,7 @@ export class SessionAgent extends Agent { generation: 0, awaitingCommandIds: [], status: "pending", + activeConnectionId: null, completedWrites: [], dialogs: [], }; @@ -42,16 +43,12 @@ export class SessionAgent extends Agent { constructor(ctx: AgentContext, env: Env) { super(ctx, env); this.coordinator = new CfSessionCoordinator({ - // getConnections() is hibernation-safe like broadcast(), but filtered to - // connections onConnect has marked authorized: the SDK accepts a socket - // (and admits it to the connection set) before onConnect's async auth - // check resolves, so an unverified or wrong-tenant socket can sit here - // during that gap - sending to it directly would hand it a plaintext - // command. sendToExtension: (payload) => { - for (const connection of this.getConnections()) { - if (this.isAuthorizedConnection(connection)) connection.send(payload); + const connection = this.authoritativeConnection(); + if (connection === undefined) { + throw new Error("authoritative extension connection disappeared before send"); } + connection.send(payload); }, hasAuthorizedConnection: () => this.hasAuthorizedConnection(), getAwaitingCommandIds: () => this.state.awaitingCommandIds, @@ -72,8 +69,28 @@ export class SessionAgent extends Agent { connection.close(1008, "tenant mismatch"); return; } + this.makeConnectionAuthoritative(connection); + } + + private makeConnectionAuthoritative(connection: Connection): void { connection.setState({ authorized: true }); - this.setState({ ...this.state, status: "connected" }); + this.setState({ + ...this.state, + activeConnectionId: connection.id, + status: "connected", + }); + + for (const previous of this.getConnections()) { + if (previous.id === connection.id || !this.isAuthorizedConnection(previous)) continue; + previous.setState({ authorized: false }); + try { + previous.close(4001, "replaced by newer extension connection"); + } catch { + // Authority already moved and the predecessor is demoted. A socket + // that raced to CLOSED must not make the successful replacement's + // onConnect reject. + } + } } /** @@ -105,7 +122,7 @@ export class SessionAgent extends Agent { } async onMessage(connection: Connection, message: WSMessage): Promise { - if (!this.isAuthorizedConnection(connection)) return; + if (!this.isAuthoritativeConnection(connection)) return; if (typeof message !== "string") return; let parsed: unknown; @@ -147,19 +164,24 @@ export class SessionAgent extends Agent { } async onClose(connection: Connection, code: number, reason: string, wasClean: boolean): Promise { - // A socket that never passed onConnect's auth check never contributed - // to the session's status, so its close must not change it either - a - // rejected/never-authorized socket closing on a fresh session would - // otherwise stamp "pending" over with "detached". if (!this.isAuthorizedConnection(connection)) return; - // Only the LAST authorized socket's close detaches the session: a late - // close event from a replaced socket must not stamp "detached" over a - // healthy reconnect (the closing connection is excluded explicitly, in - // case the SDK has not yet reaped it from the connection set). - const stillLive = [...this.getConnections()].some( - (c) => c !== connection && this.isAuthorizedConnection(c), - ); - if (!stillLive) this.setState({ ...this.state, status: "detached" }); + + const activeConnectionId = this.persistedActiveConnectionId(); + if (activeConnectionId === undefined) { + const anotherAuthorizedConnection = [...this.getConnections()].some( + (candidate) => + candidate.id !== connection.id && this.isAuthorizedConnection(candidate), + ); + if (anotherAuthorizedConnection) return; + } else if (activeConnectionId !== connection.id) { + return; + } + + this.setState({ + ...this.state, + activeConnectionId: null, + status: "detached", + }); } async dispatch(command: Command, dryRun?: boolean): Promise { @@ -426,14 +448,56 @@ export class SessionAgent extends Agent { return (connection.state as { authorized?: boolean } | null)?.authorized === true; } - // The delivery predicate the coordinator's fail-fast gate consults: the - // same precondition sendToExtension relies on, NOT the persisted status - // scalar (onClose guards the stamping races, but the scalar remains an - // eventually-consistent echo, not the delivery truth). private hasAuthorizedConnection(): boolean { - for (const connection of this.getConnections()) { - if (this.isAuthorizedConnection(connection)) return true; + return this.authoritativeConnection() !== undefined; + } + + private isAuthoritativeConnection(connection: Connection): boolean { + if (!this.isAuthorizedConnection(connection)) return false; + + const activeConnectionId = this.persistedActiveConnectionId(); + if (activeConnectionId !== undefined) { + return activeConnectionId !== null && connection.id === activeConnectionId; } - return false; + + return this.authoritativeConnection()?.id === connection.id; + } + + /** + * Returns the one live socket Commands may be sent to. A state persisted + * before activeConnectionId existed is migrated only when its connection + * set has exactly one authorized socket; zero or multiple candidates fail + * closed rather than reviving the old broadcast behavior. + */ + private authoritativeConnection(): Connection | undefined { + const activeConnectionId = this.persistedActiveConnectionId(); + if (activeConnectionId === null) return undefined; + + const authorized = [...this.getConnections()].filter((connection) => + this.isAuthorizedConnection(connection), + ); + if (activeConnectionId !== undefined) { + return authorized.find((connection) => connection.id === activeConnectionId); + } + if (authorized.length !== 1) return undefined; + + const [connection] = authorized; + if (connection === undefined) return undefined; + this.setState({ + ...this.state, + activeConnectionId: connection.id, + status: "connected", + }); + return connection; + } + + // Persisted before this field existed, a session's state can lack it; + // initialState only seeds brand-new DOs. + private persistedActiveConnectionId(): string | null | undefined { + return ( + this.state as SessionState & { + activeConnectionId?: string | null; + } + ).activeConnectionId; } } diff --git a/apps/backend/src/types.ts b/apps/backend/src/types.ts index 5be43ec..e9c181b 100644 --- a/apps/backend/src/types.ts +++ b/apps/backend/src/types.ts @@ -90,6 +90,15 @@ export interface SessionState { generation: number; awaitingCommandIds: string[]; status: SessionStatus; + /** + * The one authenticated extension connection allowed to receive Commands + * and submit Events. `null` means no authoritative connection. Sessions + * persisted before this field existed can lack it at runtime; session.ts + * migrates that legacy shape only when exactly one authorized connection + * exists, so an ambiguous multi-connection state never falls back to + * broadcasting. + */ + activeConnectionId: string | null; /** * Completed WRITE commands' Events, oldest first, capped in session.ts - * the service half of the idempotent-retry contract. A consumer retrying diff --git a/apps/backend/test/coordinator.test.ts b/apps/backend/test/coordinator.test.ts index 8f98451..a3a5d2b 100644 --- a/apps/backend/test/coordinator.test.ts +++ b/apps/backend/test/coordinator.test.ts @@ -30,7 +30,13 @@ describe("CfSessionCoordinator", () => { const promise = coordinator.send(cmd); expect(host.getAwaitingCommandIds()).toEqual(["c1"]); expect(host.sent).toEqual([JSON.stringify(cmd)]); - const event: Event = { type: "snapshot_result", commandId: "c1", tree: [] }; + const event: Event = { + type: "snapshot_result", + commandId: "c1", + tree: [], + tabId: 7, + url: "https://example.com/", + }; coordinator.resolvePending(event); // #then the promise resolves with that event and the marker is cleared @@ -80,6 +86,48 @@ describe("CfSessionCoordinator", () => { expect(host.sent).toEqual([]); }); + it("rolls back all pending bookkeeping when the authoritative socket throws during send", async () => { + // #given liveness passed, but the authoritative socket closes in the + // checked-to-send window and its synchronous send throws + vi.useFakeTimers(); + try { + const host = createFakeHost(); + host.sendToExtension = () => { + throw new Error("WebSocket is not open"); + }; + const coordinator = new CfSessionCoordinator(host); + const cmd: Command = { type: "get_tabs", commandId: "c-send-race" }; + + // #when the coordinator attempts delivery + const err = await coordinator.send(cmd).catch((e: unknown) => e); + + // #then it maps to the existing not-connected family and immediately + // clears the timer, in-memory pending entry, and persisted marker + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe( + "session not connected: authoritative extension connection unavailable during send", + ); + expect(host.getAwaitingCommandIds()).toEqual([]); + expect(vi.getTimerCount()).toBe(0); + + // #then the same commandId can dispatch after reconnect; no stale + // pending-map entry trips the duplicate-in-flight guard + host.sendToExtension = (payload: string) => { + host.sent.push(payload); + }; + const retry = coordinator.send(cmd); + expect(host.sent).toEqual([JSON.stringify(cmd)]); + coordinator.resolvePending({ type: "tabs_result", commandId: cmd.commandId, tabs: [] }); + await expect(retry).resolves.toEqual({ + type: "tabs_result", + commandId: cmd.commandId, + tabs: [], + }); + } finally { + vi.useRealTimers(); + } + }); + it("no-ops on an unknown commandId without throwing or touching the marker", () => { // #given a coordinator with nothing pending and an empty marker const host = createFakeHost(); diff --git a/apps/backend/test/service.test.ts b/apps/backend/test/service.test.ts index 646e3ac..bc826e9 100644 --- a/apps/backend/test/service.test.ts +++ b/apps/backend/test/service.test.ts @@ -117,6 +117,23 @@ function waitForCommand(socket: WebSocket): Promise { }); } +function waitForSocketClose(socket: WebSocket): Promise<{ code: number; reason: string }> { + return new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error("timed out waiting for WebSocket close")), + 10_000, + ); + socket.addEventListener("close", (event: CloseEvent) => { + clearTimeout(timeout); + resolve({ code: event.code, reason: event.reason }); + }); + socket.addEventListener("error", () => { + clearTimeout(timeout); + reject(new Error("WebSocket error while waiting for close")); + }); + }); +} + /** Collects every real dispatched Command received on `socket` (framework messages excluded). */ function collectCommands(socket: WebSocket): Command[] { const commands: Command[] = []; @@ -301,14 +318,28 @@ describe("command round-trip via a live extension WebSocket", () => { // #then the extension receives the forwarded command expect(await incoming).toEqual({ type: "snapshot", commandId: "c1", mode: "a11y" }); - socket.send(JSON.stringify({ type: "snapshot_result", commandId: "c1", tree: [] })); + socket.send( + JSON.stringify({ + type: "snapshot_result", + commandId: "c1", + tree: [], + tabId: 7, + url: "https://example.com/", + }), + ); // #then the POST resolves with that one schema-valid Event const res = await commandRes; expect(res.status).toBe(200); const event = await res.json(); expect(safeParseEvent(event).success).toBe(true); - expect(event).toEqual({ type: "snapshot_result", commandId: "c1", tree: [] }); + expect(event).toEqual({ + type: "snapshot_result", + commandId: "c1", + tree: [], + tabId: 7, + url: "https://example.com/", + }); } finally { socket.close(1000, "done"); } @@ -847,43 +878,64 @@ describe("extension liveness fail-fast", () => { } }); - it("stays connected while any authorized socket survives - a replaced socket's close is not a detach", async () => { - // #given two authorized sockets on one session + it("routes an overlap dispatch only through the replacement and closes the old socket", async () => { + // #given one live extension and collectors installed before its + // replacement connects const sessionId = await openSession(CALLER_TOKEN_A); const old = await connectFakeExtension(sessionId); + const oldCommands = collectCommands(old); + const oldClosed = waitForSocketClose(old); + + // #when a newer extension authenticates and a command dispatches while + // the old client is still observing the close handshake const replacement = await connectFakeExtension(sessionId); try { - // #when the old socket closes late (after its replacement is live) - old.close(1000, "replaced"); - await new Promise((resolve) => setTimeout(resolve, 250)); + const incoming = waitForCommand(replacement); + const response = postCommand(sessionId, CALLER_TOKEN_A, { + type: "get_tabs", + commandId: "l5", + }); + + // #then only the authoritative replacement receives and answers it + expect(await incoming).toEqual({ type: "get_tabs", commandId: "l5" }); + replacement.send( + JSON.stringify({ type: "tabs_result", commandId: "l5", tabs: [] }), + ); + expect((await response).status).toBe(200); - // #then the session is NOT stamped detached and commands still flow + // #then the predecessor receives no command and is closed with the + // replacement-specific application code/reason + expect(await oldClosed).toEqual({ + code: 4001, + reason: "replaced by newer extension connection", + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(oldCommands).toEqual([]); + + // #then its close did not detach the authoritative replacement const stub = await getSessionStub(sessionId); expect((await stub.getStatus()).status).toBe("connected"); - expect((await roundTrip(replacement, sessionId, "l5")).status).toBe(200); } finally { replacement.close(1000, "done"); } }); - it("detaches once the LAST authorized socket closes, after a replacement briefly coexisted", async () => { - // #given two authorized sockets, the old one replaced by a newer one + it("detaches when the authoritative replacement closes after evicting its predecessor", async () => { + // #given a newer socket that has replaced and closed the old authority const sessionId = await openSession(CALLER_TOKEN_A); const old = await connectFakeExtension(sessionId); + const oldClosed = waitForSocketClose(old); const replacement = await connectFakeExtension(sessionId); - - // #when the old one closes first - the replacement keeps the session live - old.close(1000, "replaced"); - await new Promise((resolve) => setTimeout(resolve, 150)); + expect((await oldClosed).code).toBe(4001); const stub = await getSessionStub(sessionId); expect((await stub.getStatus()).status).toBe("connected"); - // #when the replacement then also closes - now nothing authorized remains + // #when the authoritative replacement closes replacement.close(1000, "gone"); await waitForStatus(sessionId, "detached"); - // #then the session finally detaches (the full replaced-then-both-closed order) + // #then the session detaches expect((await stub.getStatus()).status).toBe("detached"); }); }); diff --git a/apps/backend/test/session.test.ts b/apps/backend/test/session.test.ts index ec565a7..b08b5ef 100644 --- a/apps/backend/test/session.test.ts +++ b/apps/backend/test/session.test.ts @@ -5,11 +5,15 @@ import type { Connection, ConnectionContext } from "agents"; import type { Command } from "@understudy/protocol"; import { mintSessionId } from "../src/auth"; import type { SessionAgent } from "../src/session"; +import type { SessionState } from "../src/types"; import { EXTENSION_TOKEN_A, EXTENSION_TOKEN_B } from "./tokens"; import { BASE, getSessionStub, getWebSocket } from "./helpers"; -/** onMessage only reads `connection.state.authorized` (the onConnect auth gate) - see src/session.ts. */ -const FAKE_CONNECTION = { state: { authorized: true } } as unknown as Connection; +const FAKE_CONNECTION = { + id: "fake-authoritative", + state: { authorized: true }, + send: () => {}, +} as unknown as Connection; /** * A mutable stand-in for a Connection at the onConnect/onClose surface. @@ -18,18 +22,26 @@ const FAKE_CONNECTION = { state: { authorized: true } } as unknown as Connection * .state the way the SDK does, so isAuthorizedConnection() reads what * onConnect wrote. */ -function fakeConnection(initialState: { authorized?: boolean } | null = null): { +function fakeConnection( + initialState: { authorized?: boolean } | null = null, + id = crypto.randomUUID(), +): { connection: Connection; close: ReturnType; + send: ReturnType; setState: ReturnType; } { const close = vi.fn(); - const holder: { state: { authorized?: boolean } | null } = { state: initialState }; + const send = vi.fn(); + const holder: { id: string; state: { authorized?: boolean } | null } = { + id, + state: initialState, + }; const setState = vi.fn((next: unknown) => { holder.state = next as { authorized?: boolean } | null; }); - const connection = Object.assign(holder, { close, setState }) as unknown as Connection; - return { connection, close, setState }; + const connection = Object.assign(holder, { close, send, setState }) as unknown as Connection; + return { connection, close, send, setState }; } function upgradeContextFor(sessionId: string, token: string): ConnectionContext { @@ -38,6 +50,23 @@ function upgradeContextFor(sessionId: string, token: string): ConnectionContext } as ConnectionContext; } +function setAuthoritative( + instance: SessionAgent, + connection: Connection = FAKE_CONNECTION, +): void { + instance.setState({ + ...instance.state, + activeConnectionId: connection.id, + status: "connected", + }); +} + +function withoutActiveConnectionId(state: SessionState): SessionState { + const legacy = { ...state } as Partial; + delete legacy.activeConnectionId; + return legacy as SessionState; +} + /** * The worker-level gate (index.ts onBeforeConnect) now refuses bad upgrades * before the DO accepts anything - service.test.ts covers that layer. These @@ -101,6 +130,48 @@ describe("onConnect token verification (in-DO defense in depth)", () => { expect(close).toHaveBeenCalledWith(1008, "tenant mismatch"); expect(setState).not.toHaveBeenCalled(); }); + + it("makes the newest authenticated socket authoritative even when closing its predecessor throws", async () => { + // #given an existing authorized socket and a newer socket that has + // completed the token/tenant checks + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const previous = fakeConnection({ authorized: true }, "previous"); + const replacement = fakeConnection(null, "replacement"); + previous.close.mockImplementationOnce(() => { + throw new Error("already closed"); + }); + + await runInDurableObject(stub, async (instance: SessionAgent) => { + Object.assign(instance, { + getConnections: () => [previous.connection, replacement.connection], + }); + + // #when onConnect's post-auth step promotes the replacement + ( + instance as unknown as { + makeConnectionAuthoritative(connection: Connection): void; + } + ).makeConnectionAuthoritative(replacement.connection); + + // #then one persisted state write makes it authoritative before the + // prior socket is demoted and closed + expect(instance.state.activeConnectionId).toBe("replacement"); + expect(instance.state.status).toBe("connected"); + expect(replacement.setState).toHaveBeenCalledWith({ authorized: true }); + expect(previous.connection.state).toEqual({ authorized: false }); + expect(previous.close).toHaveBeenCalledWith( + 4001, + "replaced by newer extension connection", + ); + + // #then a late close callback from the replaced socket cannot detach + // or clear the replacement + await instance.onClose(previous.connection, 4001, "replaced", true); + expect(instance.state.activeConnectionId).toBe("replacement"); + expect(instance.state.status).toBe("connected"); + }); + }); }); describe("onClose status stamping", () => { @@ -121,6 +192,81 @@ describe("onClose status stamping", () => { expect((await stub.getStatus()).status).toBe("pending"); }); + it("migrates a legacy single authorized socket to activeConnectionId on first use", async () => { + // #given persisted pre-migration state with exactly one authorized socket + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const legacy = fakeConnection({ authorized: true }, "legacy-only"); + + await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState(withoutActiveConnectionId(instance.state)); + Object.assign(instance, { getConnections: () => [legacy.connection] }); + + // #when its inbound hello is processed + await instance.onMessage( + legacy.connection, + JSON.stringify({ type: "hello", browser: "chrome", extVersion: "1.0.0", tabs: [] }), + ); + + // #then the sole authorized socket is persisted as the authority and + // its event is accepted + expect(instance.state.activeConnectionId).toBe("legacy-only"); + expect(instance.state.generation).toBe(1); + expect(instance.state.status).toBe("connected"); + }); + }); + + it("fails closed instead of choosing among multiple authorized sockets in legacy state", async () => { + // #given persisted pre-migration state with an ambiguous pair of + // authorized sockets + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const first = fakeConnection({ authorized: true }, "legacy-a"); + const second = fakeConnection({ authorized: true }, "legacy-b"); + + await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState(withoutActiveConnectionId(instance.state)); + Object.assign(instance, { + getConnections: () => [first.connection, second.connection], + }); + + // #when either socket submits an otherwise valid event + await instance.onMessage( + first.connection, + JSON.stringify({ type: "hello", browser: "chrome", extVersion: "1.0.0", tabs: [] }), + ); + + // #then neither is silently promoted and the event is ignored + expect(instance.state.activeConnectionId).toBeUndefined(); + expect(instance.state.generation).toBe(0); + expect(instance.state.status).toBe("pending"); + }); + }); + + it("detaches when a legacy session's sole authorized socket closes after leaving getConnections", async () => { + // #given persisted pre-migration state whose sole authorized socket is + // already absent from getConnections by the time onClose runs + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const legacy = fakeConnection({ authorized: true }, "legacy-closing"); + + await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState({ + ...withoutActiveConnectionId(instance.state), + status: "connected", + }); + Object.assign(instance, { getConnections: () => [] }); + + // #when that sole legacy socket closes + await instance.onClose(legacy.connection, 1000, "gone", true); + + // #then migration identifies it as authoritative before clearing the + // authority and stamping detached + expect(instance.state.activeConnectionId).toBeNull(); + expect(instance.state.status).toBe("detached"); + }); + }); + it("an authorized socket's close still detaches when it was the last one", async () => { // #given a session an authorized socket connected to (status: connected) const sessionId = crypto.randomUUID(); @@ -128,7 +274,11 @@ describe("onClose status stamping", () => { const { connection: authorized } = fakeConnection({ authorized: true }); await runInDurableObject(stub, async (instance: SessionAgent) => { - instance.setState({ ...instance.state, status: "connected" }); + instance.setState({ + ...instance.state, + activeConnectionId: authorized.id, + status: "connected", + }); // #when it closes with no surviving authorized socket await instance.onClose(authorized, 1000, "gone", true); @@ -140,6 +290,88 @@ describe("onClose status stamping", () => { }); describe("dispatch / resolvePending", () => { + it("maps an authoritative socket's synchronous send failure to not_connected", async () => { + // #given a persisted authoritative socket that closes in the + // checked-to-send window + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const active = fakeConnection({ authorized: true }, "send-race"); + active.send.mockImplementationOnce(() => { + throw new Error("WebSocket is not open"); + }); + const cmd: Command = { type: "get_tabs", commandId: "send-race-1" }; + + const outcome = await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState({ + ...instance.state, + activeConnectionId: active.connection.id, + status: "connected", + }); + Object.assign(instance, { getConnections: () => [active.connection] }); + return instance.dispatch(cmd); + }); + + // #then SessionAgent maps the coordinator's delivery-race prefix to the + // typed not_connected outcome and no awaiting marker survives + expect(outcome).toEqual({ + ok: false, + reason: "not_connected", + message: + "session not connected: authoritative extension connection unavailable during send", + }); + await runInDurableObject(stub, (instance: SessionAgent) => { + expect(instance.state.awaitingCommandIds).toEqual([]); + }); + }); + + it("sends to and accepts a result from only the authoritative socket while two authorized sockets overlap", async () => { + // #given two still-authorized sockets in the connection set, with the + // replacement persisted as the sole authority + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const previous = fakeConnection({ authorized: true }, "overlap-old"); + const replacement = fakeConnection({ authorized: true }, "overlap-new"); + const cmd: Command = { type: "get_tabs", commandId: "overlap-1" }; + + const result = await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState({ + ...instance.state, + activeConnectionId: replacement.connection.id, + status: "connected", + }); + Object.assign(instance, { + getConnections: () => [previous.connection, replacement.connection], + }); + + // #when a command dispatches during the overlap + const dispatchPromise = instance.dispatch(cmd); + + // #then only the replacement receives it; there is no broadcast + expect(previous.send).not.toHaveBeenCalled(); + expect(replacement.send).toHaveBeenCalledWith(JSON.stringify(cmd)); + + // #when the old socket forges the matching result, it is ignored even + // though its authorized bit is still true + await instance.onMessage( + previous.connection, + JSON.stringify({ type: "tabs_result", commandId: cmd.commandId, tabs: [] }), + ); + expect(instance.state.awaitingCommandIds).toEqual([cmd.commandId]); + + // #when the authoritative replacement answers, it alone resolves the command + await instance.onMessage( + replacement.connection, + JSON.stringify({ type: "tabs_result", commandId: cmd.commandId, tabs: [] }), + ); + return dispatchPromise; + }); + + expect(result).toEqual({ + ok: true, + event: { type: "tabs_result", commandId: cmd.commandId, tabs: [] }, + }); + }); + it("resolves a dispatched command's promise with the matching result event", async () => { // #given a command sent via dispatch, and its matching result delivered // via onMessage - both driven from a SINGLE runInDurableObject callback @@ -154,11 +386,8 @@ describe("dispatch / resolvePending", () => { const cmd: Command = { type: "get_tabs", commandId: "s1" }; const result = await runInDurableObject(stub, async (instance: SessionAgent) => { - // Stand in for a live authorized extension socket: the coordinator's - // fail-fast gate consults the DO's connection set, which - // runInDurableObject cannot populate; the service-level suite covers - // the real predicate. This test is about correlation. - Object.assign(instance, { hasAuthorizedConnection: () => true }); + Object.assign(instance, { getConnections: () => [FAKE_CONNECTION] }); + setAuthoritative(instance); const dispatchPromise = instance.dispatch(cmd); // The marker is parked synchronously before dispatch() suspends. expect(instance.state.awaitingCommandIds).toContain("s1"); @@ -197,7 +426,11 @@ describe("DO eviction resilience (DL-007)", () => { const sessionId = crypto.randomUUID(); const stub = await getSessionStub(sessionId); await runInDurableObject(stub, (instance: SessionAgent) => { - instance.setState({ ...instance.state, awaitingCommandIds: ["hib-1"] }); + instance.setState({ + ...instance.state, + activeConnectionId: FAKE_CONNECTION.id, + awaitingCommandIds: ["hib-1"], + }); }); await runInDurableObject(stub, (instance: SessionAgent) => { expect(instance.state.awaitingCommandIds).toEqual(["hib-1"]); @@ -242,10 +475,8 @@ describe("hello resync", () => { let outcome!: ReturnType; await runInDurableObject(stub, (instance: SessionAgent) => { - // Stand in for a live authorized socket (see "resolves a dispatched - // command's promise" above), or the fail-fast gate refuses before the - // marker is ever parked and there is nothing for the resync to abandon. - Object.assign(instance, { hasAuthorizedConnection: () => true }); + Object.assign(instance, { getConnections: () => [FAKE_CONNECTION] }); + setAuthoritative(instance); outcome = instance.dispatch(cmd); expect(instance.state.awaitingCommandIds).toContain("resync-1"); }); @@ -300,6 +531,7 @@ describe("dialog recording (onMessage → SessionState.dialogs)", () => { const sessionId = crypto.randomUUID(); const stub = await getSessionStub(sessionId); await runInDurableObject(stub, async (instance: SessionAgent) => { + setAuthoritative(instance); for (let i = 0; i < 51; i++) { await instance.onMessage(FAKE_CONNECTION, dialogEvent(`d${i}`)); } diff --git a/apps/extension/CLAUDE.md b/apps/extension/CLAUDE.md index 6e1d1a6..6fc0c9b 100644 --- a/apps/extension/CLAUDE.md +++ b/apps/extension/CLAUDE.md @@ -17,15 +17,20 @@ WXT + React MV3 extension driving protocol Commands into a real Chromium tab via | `src/events.ts` | `errorMessage`, `actionError` — shared error-to-`action_result` helpers | Adding an executor that can fail | | `src/tabs.ts` | `queryTabInfos` — `chrome.tabs.query` to `TabInfo[]` | Changing tab metadata reported to the backend | | `src/messaging.ts` | `PanelMsg`/`SwMsg` discriminated unions for the sidepanel↔service-worker `Port` | Changing panel/background message shapes | -| `src/driver/a11y.ts` | `buildA11ySnapshot` — pure AX-tree → pruned `A11yNode[]` + generation-namespaced ref map | Changing which roles are surfaced, ref format, or snapshot pruning | -| `src/driver/a11y.test.ts` | Unit tests for `buildA11ySnapshot` against a hand-authored AX fixture | Verifying a11y pruning/re-parenting behavior | +| `src/driver/a11y.ts` | `buildA11ySnapshot` — pure AX-tree → pruned `A11yNode[]` + opaque session/attachment/generation-bound ref map | Changing which roles are surfaced, ref format, or snapshot pruning | +| `src/driver/a11y.test.ts` | Unit tests for `buildA11ySnapshot` against a hand-authored AX fixture, including scope isolation | Verifying a11y pruning/re-parenting or ref-binding behavior | | `src/driver/keymap.ts` | `parseKeys` — key-spec string → CDP `Input.dispatchKeyEvent` fields | Adding a named key or modifier alias | | `src/driver/keymap.test.ts` | Unit tests for `parseKeys` (modifiers, named keys, printable chars) | Verifying key-spec parsing | | `src/driver/cdp-events.ts` | `classifyCdpEvent` — raw CDP event → effects decision (`CdpDecision`) | Handling a new CDP event type, changing navigation/dialog handling | | `src/driver/cdp-events.test.ts` | Unit tests for `classifyCdpEvent` + `dialogDisposition` (main-frame filter, load URL, generation bumps, per-type dialog disposition) | Verifying CDP event classification | | `src/driver/cdp.ts` | `CdpSession` — FIFO-queued `chrome.debugger` channel; executors for every protocol Command (`snapshotA11y`, `screenshot`, `click`, `type`, `key`, `scroll`, `wait`, `navigate`, `resolveRefCheck`) | Adding/changing a command executor, debugging a CDP call | -| `src/driver/cdp.test.ts` | Unit tests for `resolveRefCheck` — the dry-run probe's no-snapshot/no-generation-bump invariant | Changing resolveRefCheck or the ref/generation model | +| `src/driver/cdp.test.ts` | Unit tests for ref probes/binding, exact snapshot target brackets, failure invalidation, and aggregate deadlines | Changing resolveRefCheck, snapshot identity/deadlines, or the ref/generation model | | `src/core/ws-client.ts` | `ReconnectingWs` — WebSocket with backoff reconnect and self-driven pong heartbeat | Changing reconnect/backoff/heartbeat behavior | +| `src/core/ws-client.test.ts` | Unit tests for ordinary reconnect and terminal backend replacement close code 4001 | Changing reconnect termination behavior | +| `src/core/command-ingress.ts` | `CommandIngress` — serial command-admission queue plus drain barrier for WebSocket session changes | Changing wire-order or session-switch command draining | +| `src/core/command-ingress.test.ts` | Unit tests for dedupe-hydration order, drain ordering, and captured-peer responses | Verifying command ingress/session-switch concurrency | +| `src/core/peer-binding.ts` | `sendIfPeerCurrent` — drops delayed hello/page/dialog sends after peer authority changes | Changing post-await WebSocket event routing | +| `src/core/peer-binding.test.ts` | Unit tests for current-peer send/drop behavior | Verifying cross-session event isolation | | `src/core/router.ts` | `routeCommand` — dispatches a parsed `Command` to a `CdpSession` executor or tab handler | Adding a new protocol Command type | | `src/core/router.test.ts` | Unit tests for `routeCommand` (one Event per Command, error paths) | Verifying command routing | | `src/core/dedupe.ts` | `WriteDedupe` — `claim()` (execute / replay a completed write / drop an in-flight duplicate) + `remember`/`release`/`clear`; idempotent-retry contract, storage.session-mirrored, cap 100 | Changing write replay/dedupe/in-flight behavior | diff --git a/apps/extension/README.md b/apps/extension/README.md index 29dd728..80f7719 100644 --- a/apps/extension/README.md +++ b/apps/extension/README.md @@ -12,11 +12,12 @@ M3. | Path | What | | --- | --- | -| `src/driver/a11y.ts` | `buildA11ySnapshot` — pure AX-tree → pruned, generation-namespaced `A11yNode[]` + ref map. No `chrome.*`. | +| `src/driver/a11y.ts` | `buildA11ySnapshot` — pure AX-tree → pruned `A11yNode[]` + opaque session/attachment/generation-bound ref map. No `chrome.*`. | | `src/driver/keymap.ts` | `parseKeys` — pure key-spec parser (`"Ctrl+Enter"` etc.) → CDP `Input.dispatchKeyEvent` fields. No `chrome.*`. | | `src/driver/cdp-events.ts` | `classifyCdpEvent` — pure classifier turning a raw CDP event into the effects the background worker should apply. No `chrome.*`. | | `src/driver/cdp.ts` | `CdpSession` — the one `chrome.debugger` channel per attached tab: FIFO command queue, per-command timeouts, and the executors backing every protocol Command. | | `src/core/ws-client.ts` | `ReconnectingWs` — WebSocket with backoff reconnect and a self-driven pong heartbeat. | +| `src/core/command-ingress.ts` | `CommandIngress` — preserves command arrival order through async dedupe claims and provides a drain barrier for WebSocket session changes. | | `src/core/router.ts` | `routeCommand` — dispatches a parsed `Command` to a `CdpSession` executor or a tab-management handler, always returning exactly one `Event`. | | `src/events.ts`, `src/tabs.ts`, `src/messaging.ts` | Shared leaf helpers (`action_result` builder, tab-info query) and the sidepanel↔service-worker `Port` message types. | | `src/entrypoints/background.ts` | The MV3 service worker: owns the WS connection, the CDP session, wake-time reattachment, and the alarm/heartbeat keepalive. | diff --git a/apps/extension/RUNBOOK.md b/apps/extension/RUNBOOK.md index b1a79a2..a71498f 100644 --- a/apps/extension/RUNBOOK.md +++ b/apps/extension/RUNBOOK.md @@ -11,7 +11,7 @@ what M3 replaces (and a reference for M3's `onMessage`). ## Baseline / environment -- Branch `master`, base commit `4a9a946` (`git rev-parse HEAD` to confirm). +- Branch `master`, base commit `4a48b94` (`git rev-parse HEAD` to confirm). - **Node ≥ 22, pnpm 11.5.2** (repo `packageManager`). This machine: Node 24, pnpm 11.5.2. - **Real Chromium via `wxt build` + Load unpacked — NOT `wxt dev`.** `wxt dev` starts a throwaway profile with no logins, which defeats the whole point @@ -90,14 +90,18 @@ press Enter. `commandId` is auto-filled — you do not type it. Blank lines and lines starting with `#` are ignored. After each send the stub echoes `> sent …` and then prints the extension's reply. -> Tip: replace `` with a real ref (e.g. `s1e7`) copied from the most recent -> `snapshot_result` output. Refs are generation-namespaced — a ref is only valid -> for the snapshot generation that produced it. +> Tip: replace `` with the opaque ref copied verbatim from the most recent +> `snapshot_result` output. Do not parse or construct refs: each is a capability +> bound to the extension WebSocket session, CDP attachment, and snapshot +> generation that produced it. ```jsonc -# read the page — prints a nested node tree; copy a textbox/searchbox/button ref +# read the page — first confirm tabId + exact URL, then copy a textbox/searchbox/button ref {"type":"snapshot","mode":"a11y"} +# capture the same exact target as an image +{"type":"snapshot","mode":"screenshot"} + # type into a field you copied a ref for (submit:false = don't press Enter) {"type":"type","ref":"","text":"hello","submit":false} @@ -127,11 +131,12 @@ Expected replies (watch the stub terminal): | Line | Expected stub output | Visible in Chromium | |---|---|---| -| `snapshot` a11y | `snapshot_result` with a node count + the first ~15 `{ref role "name"}` indented | — | +| `snapshot` a11y | `snapshot_result` with the attached `tabId`, exact bracketed URL (including any fragment), node count, and first ~15 `{ref role "name"}` indented. Confirm the target matches the panel/current tab before using a ref. | — | +| `snapshot` screenshot | `screenshot_result` with the same attached `tabId`, exact bracketed URL, MIME type, and payload length | — | | `type` | `action_result … ok=true` | the text appears in the field | | `click` | `action_result … ok=true` | the element is clicked | | `navigate` | `action_result … ok=true url=…` **and** a `page_event navigated …` | the tab navigates | -| stale `click` | `action_result … ok=false error=…` (generation mismatch — no input dispatched) | nothing happens | +| stale `click` | `action_result … ok=false error=…` (stale/unknown capability — no input dispatched) | nothing happens | | `get_tabs` | `tabs_result` listing the open tabs (tabId/title/url) | — | | `switch_tab` | `action_result … ok=true` | the other tab becomes active | | `snapshot` dom | `action_result … ok=false error="dom snapshot unsupported"` | — | @@ -173,10 +178,11 @@ and the command returns its normal result. **Zero `EVENT SCHEMA VIOLATION` logs across the entire session.** -Every event the extension emitted — `hello`, `snapshot_result`, `action_result`, -`page_event`, `tabs_result`, `pong` — passed `safeParseEvent` against the real -protocol schemas on the real WS wire. Combined with the visible page effects in -step 6, the stale-ref rejection, `get_tabs`, idle survival (step 7), and +Every event the extension emitted — `hello`, `snapshot_result`, +`screenshot_result`, `action_result`, `page_event`, `dialog`, `tabs_result`, +`pong` — passed `safeParseEvent` against the real protocol schemas on the real WS wire. +Combined with explicit snapshot target confirmation, the visible page effects +in step 6, the stale-ref rejection, `get_tabs`, idle survival (step 7), and eviction reconcile (step 8), that is M2 proven end-to-end. If a violation *does* print, it is loud and boxed and includes the raw event plus diff --git a/apps/extension/scripts/stub-server.mjs b/apps/extension/scripts/stub-server.mjs index dc848a6..eb638c5 100644 --- a/apps/extension/scripts/stub-server.mjs +++ b/apps/extension/scripts/stub-server.mjs @@ -50,7 +50,7 @@ function printNodes(nodes, depth, budget) { } } -// ── pretty-printers, one per Event.type (all 7 union members) ──────────────── +// ── pretty-printers, one per Event.type (all 8 union members) ──────────────── function printEvent(ev) { switch (ev.type) { @@ -61,13 +61,13 @@ function printEvent(ev) { break; case "snapshot_result": console.log( - `< snapshot_result [${ev.commandId}] ${countNodes(ev.tree)} nodes (first 15 shown — copy a ref):`, + `< snapshot_result [${ev.commandId}] tabId=${ev.tabId} url=${ev.url} ${countNodes(ev.tree)} nodes (first 15 shown — copy a ref):`, ); printNodes(ev.tree, 0, { left: 15 }); break; case "screenshot_result": console.log( - `< screenshot_result [${ev.commandId}] ${ev.mime} ${ev.b64.length} b64 chars`, + `< screenshot_result [${ev.commandId}] tabId=${ev.tabId} url=${ev.url} ${ev.mime} ${ev.b64.length} b64 chars`, ); break; case "tabs_result": @@ -88,6 +88,16 @@ function printEvent(ev) { case "page_event": console.log(`< page_event ${ev.kind} tabId=${ev.tabId} ${ev.url}`); break; + case "dialog": { + const prompt = + ev.defaultPrompt === undefined + ? "" + : ` defaultPrompt=${JSON.stringify(ev.defaultPrompt)}`; + console.log( + `< dialog ${ev.dialogType} disposition=${ev.disposition} tabId=${ev.tabId} url=${ev.url} message=${JSON.stringify(ev.message)}${prompt}`, + ); + break; + } case "pong": console.log(`< pong · ${new Date().toLocaleTimeString()}`); break; diff --git a/apps/extension/src/core/command-ingress.test.ts b/apps/extension/src/core/command-ingress.test.ts new file mode 100644 index 0000000..95fef53 --- /dev/null +++ b/apps/extension/src/core/command-ingress.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import type { Command } from "@understudy/protocol"; +import { WriteDedupe, type SessionStorageArea } from "./dedupe"; +import { CommandIngress } from "./command-ingress"; + +const WRITE: Command = { + type: "navigate", + commandId: "write-first", + url: "https://example.com/next", +}; +const READ: Command = { type: "snapshot", commandId: "read-second", mode: "a11y" }; + +describe("CommandIngress", () => { + it("preserves wire order when an earlier write waits for dedupe hydration", async () => { + let releaseHydration!: (value: Record) => void; + const storage: SessionStorageArea = { + get: () => + new Promise((resolve) => { + releaseHydration = resolve; + }), + set: async () => {}, + remove: async () => {}, + }; + const dedupe = new WriteDedupe(storage); + const ingress = new CommandIngress(); + const started: string[] = []; + + const write = ingress.enqueue(async () => { + await dedupe.claim(WRITE); + started.push(WRITE.commandId); + return { completion: Promise.resolve() }; + }); + const read = ingress.enqueue(async () => { + await dedupe.claim(READ); + started.push(READ.commandId); + return { completion: Promise.resolve() }; + }); + + await Promise.resolve(); + expect(started).toEqual([]); + + releaseHydration({}); + await Promise.all([write, read]); + + expect(started).toEqual(["write-first", "read-second"]); + }); + + it("runs an invalidation barrier after every already-started execution settles", async () => { + let finishSnapshot!: () => void; + const ingress = new CommandIngress(); + const calls: string[] = []; + + await ingress.enqueue(async () => { + calls.push("snapshot:start"); + return { + completion: new Promise((resolve) => { + finishSnapshot = () => { + calls.push("snapshot:finish"); + resolve(); + }; + }), + }; + }); + const barrier = ingress.barrier(async () => { + calls.push("invalidate"); + }); + + await Promise.resolve(); + expect(calls).toEqual(["snapshot:start"]); + + finishSnapshot(); + await barrier; + + expect(calls).toEqual(["snapshot:start", "snapshot:finish", "invalidate"]); + }); + + it("keeps an accepted command's response bound to its old peer across a switch barrier", async () => { + let finish!: () => void; + const ingress = new CommandIngress(); + const oldPeer: string[] = []; + const newPeer: string[] = []; + let activePeer = oldPeer; + + await ingress.enqueue(async () => { + const acceptedPeer = activePeer; + return { + completion: new Promise((resolve) => { + finish = () => { + acceptedPeer.push("snapshot_result"); + resolve(); + }; + }), + }; + }); + const switching = ingress.barrier(async () => { + activePeer = newPeer; + }); + + finish(); + await switching; + + expect(oldPeer).toEqual(["snapshot_result"]); + expect(newPeer).toEqual([]); + }); +}); diff --git a/apps/extension/src/core/command-ingress.ts b/apps/extension/src/core/command-ingress.ts new file mode 100644 index 0000000..928357b --- /dev/null +++ b/apps/extension/src/core/command-ingress.ts @@ -0,0 +1,39 @@ +export interface StartedCommand { + completion: Promise; +} + +export class CommandIngress { + private tail: Promise = Promise.resolve(); + private readonly executions = new Set>(); + + enqueue(start: () => Promise): Promise { + return this.serialize(async () => { + const started = await start(); + if (started === undefined) return; + + const completion = started.completion; + this.executions.add(completion); + void completion + .finally(() => { + this.executions.delete(completion); + }) + .catch(() => {}); + }); + } + + barrier(run: () => Promise): Promise { + return this.serialize(async () => { + await Promise.allSettled([...this.executions]); + await run(); + }); + } + + private serialize(task: () => Promise): Promise { + const result = this.tail.then(task, task); + this.tail = result.then( + () => undefined, + () => undefined, + ); + return result; + } +} diff --git a/apps/extension/src/core/peer-binding.test.ts b/apps/extension/src/core/peer-binding.test.ts new file mode 100644 index 0000000..8d277cc --- /dev/null +++ b/apps/extension/src/core/peer-binding.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it, vi } from "vitest"; +import { sendIfPeerCurrent } from "./peer-binding"; + +describe("sendIfPeerCurrent", () => { + it("sends through the peer that still owns the session", () => { + const peer = {}; + const send = vi.fn(); + + sendIfPeerCurrent(peer, peer, send); + + expect(send).toHaveBeenCalledWith(peer); + }); + + it("drops a delayed event after its admitted peer has been retired", () => { + const oldPeer = {}; + const newPeer = {}; + const send = vi.fn(); + + sendIfPeerCurrent(oldPeer, newPeer, send); + sendIfPeerCurrent(null, newPeer, send); + + expect(send).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/extension/src/core/peer-binding.ts b/apps/extension/src/core/peer-binding.ts new file mode 100644 index 0000000..14ea2fe --- /dev/null +++ b/apps/extension/src/core/peer-binding.ts @@ -0,0 +1,9 @@ +export function sendIfPeerCurrent( + admittedPeer: T | null, + currentPeer: T | null, + send: (peer: T) => void, +): void { + if (admittedPeer !== null && admittedPeer === currentPeer) { + send(admittedPeer); + } +} diff --git a/apps/extension/src/core/router.test.ts b/apps/extension/src/core/router.test.ts index db95367..4c8a088 100644 --- a/apps/extension/src/core/router.test.ts +++ b/apps/extension/src/core/router.test.ts @@ -5,6 +5,7 @@ import type { CdpSession } from "../driver/cdp"; import { routeCommand } from "./router"; interface MockSession { + tabId: number; snapshotA11y: Mock; screenshot: Mock; click: Mock; @@ -18,6 +19,7 @@ interface MockSession { function createMockSession(): MockSession { return { + tabId: 7, snapshotA11y: vi.fn(), screenshot: vi.fn(), click: vi.fn(), @@ -48,7 +50,13 @@ afterEach(() => { describe("routeCommand", () => { it("routes snapshot mode a11y to session.snapshotA11y and returns its event", async () => { const mock = createMockSession(); - const event: Event = { type: "snapshot_result", commandId: "c-a11y", tree: [] }; + const event: Event = { + type: "snapshot_result", + commandId: "c-a11y", + tree: [], + tabId: 7, + url: "https://example.com/", + }; mock.snapshotA11y.mockResolvedValue(event); const cmd: Command = { type: "snapshot", commandId: "c-a11y", mode: "a11y" }; @@ -60,7 +68,14 @@ describe("routeCommand", () => { it("routes snapshot mode screenshot to session.screenshot and returns its event", async () => { const mock = createMockSession(); - const event: Event = { type: "screenshot_result", commandId: "c-shot", mime: "image/png", b64: "QQ==" }; + const event: Event = { + type: "screenshot_result", + commandId: "c-shot", + mime: "image/png", + b64: "QQ==", + tabId: 7, + url: "https://example.com/", + }; mock.screenshot.mockResolvedValue(event); const cmd: Command = { type: "snapshot", commandId: "c-shot", mode: "screenshot" }; @@ -70,6 +85,21 @@ describe("routeCommand", () => { expect(result).toEqual(event); }); + it("rejects a snapshot requested for a tab other than the attached CDP session", async () => { + const mock = createMockSession(); + const cmd: Command = { type: "snapshot", commandId: "c-mismatch", mode: "a11y", tabId: 8 }; + + const result = await routeCommand(cmd, asSession(mock)); + + expect(result).toEqual({ + type: "action_result", + commandId: "c-mismatch", + ok: false, + error: "attached CDP session is tab 7, not requested tab 8", + }); + expect(mock.snapshotA11y).not.toHaveBeenCalled(); + }); + it("returns action_result unsupported for snapshot mode dom without touching the session", async () => { const mock = createMockSession(); const cmd: Command = { type: "snapshot", commandId: "c-dom", mode: "dom" }; @@ -98,6 +128,26 @@ describe("routeCommand", () => { expect(result).toEqual(event); }); + it("rejects navigate when its requested tab differs from the attached CDP session", async () => { + const mock = createMockSession(); + const cmd: Command = { + type: "navigate", + commandId: "c-nav-mismatch", + url: "https://example.com/", + tabId: 8, + }; + + const result = await routeCommand(cmd, asSession(mock)); + + expect(result).toEqual({ + type: "action_result", + commandId: "c-nav-mismatch", + ok: false, + error: "attached CDP session is tab 7, not requested tab 8", + }); + expect(mock.navigate).not.toHaveBeenCalled(); + }); + it("routes click to session.click with the ref", async () => { const mock = createMockSession(); const event: Event = { type: "action_result", commandId: "c-click", ok: true }; diff --git a/apps/extension/src/core/router.ts b/apps/extension/src/core/router.ts index a80683d..5231274 100644 --- a/apps/extension/src/core/router.ts +++ b/apps/extension/src/core/router.ts @@ -7,10 +7,17 @@ async function withSession( session: CdpSession | null, commandId: string, run: (session: CdpSession) => Promise, + expectedTabId?: number, ): Promise { if (session === null) { return actionError(commandId, "no active CDP session"); } + if (expectedTabId !== undefined && expectedTabId !== session.tabId) { + return actionError( + commandId, + `attached CDP session is tab ${session.tabId}, not requested tab ${expectedTabId}`, + ); + } return run(session); } @@ -31,17 +38,29 @@ export async function routeCommand(cmd: Command, session: CdpSession | null): Pr if (cmd.mode === "dom") { return actionError(cmd.commandId, "dom snapshot unsupported"); } - if (session === null) { - return actionError(cmd.commandId, "no active CDP session"); - } if (cmd.mode === "a11y") { - return await session.snapshotA11y(cmd.commandId); + return await withSession( + session, + cmd.commandId, + (s) => s.snapshotA11y(cmd.commandId), + cmd.tabId, + ); } - return await session.screenshot(cmd.commandId); + return await withSession( + session, + cmd.commandId, + (s) => s.screenshot(cmd.commandId), + cmd.tabId, + ); } case "navigate": { const { url } = cmd; - return await withSession(session, cmd.commandId, (s) => s.navigate(cmd.commandId, url)); + return await withSession( + session, + cmd.commandId, + (s) => s.navigate(cmd.commandId, url), + cmd.tabId, + ); } case "click": { const { ref } = cmd; diff --git a/apps/extension/src/core/ws-client.test.ts b/apps/extension/src/core/ws-client.test.ts new file mode 100644 index 0000000..5703300 --- /dev/null +++ b/apps/extension/src/core/ws-client.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ReconnectingWs } from "./ws-client"; + +type Listener = (event: Event & { code?: number; data?: unknown }) => void; + +class FakeWebSocket { + static readonly OPEN = 1; + static instances: FakeWebSocket[] = []; + + readyState = 0; + private readonly listeners = new Map(); + + constructor(readonly url: string) { + FakeWebSocket.instances.push(this); + } + + addEventListener(type: string, listener: Listener): void { + const listeners = this.listeners.get(type) ?? []; + listeners.push(listener); + this.listeners.set(type, listeners); + } + + send(): void {} + + close(): void { + this.emit("close", { code: 1000 }); + } + + emit(type: string, init: { code?: number; data?: unknown } = {}): void { + const event = { type, ...init } as Event & { code?: number; data?: unknown }; + for (const listener of this.listeners.get(type) ?? []) listener(event); + } +} + +describe("ReconnectingWs", () => { + beforeEach(() => { + vi.useFakeTimers(); + FakeWebSocket.instances = []; + vi.stubGlobal("WebSocket", FakeWebSocket); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + }); + + it("does not reconnect after the backend replaces this extension with close code 4001", () => { + const onClose = vi.fn(); + new ReconnectingWs(() => "ws://example.test/session", { + onCommand: vi.fn(), + onOpen: vi.fn(), + onClose, + }); + + FakeWebSocket.instances[0]?.emit("close", { code: 4001 }); + vi.advanceTimersByTime(60_000); + + expect(onClose).toHaveBeenCalledOnce(); + expect(FakeWebSocket.instances).toHaveLength(1); + }); + + it("reconnects an ordinary close with backoff", () => { + new ReconnectingWs(() => "ws://example.test/session", { + onCommand: vi.fn(), + onOpen: vi.fn(), + }); + + FakeWebSocket.instances[0]?.emit("close", { code: 1006 }); + vi.advanceTimersByTime(499); + expect(FakeWebSocket.instances).toHaveLength(1); + + vi.advanceTimersByTime(1); + expect(FakeWebSocket.instances).toHaveLength(2); + }); +}); diff --git a/apps/extension/src/core/ws-client.ts b/apps/extension/src/core/ws-client.ts index c6d5076..742b80d 100644 --- a/apps/extension/src/core/ws-client.ts +++ b/apps/extension/src/core/ws-client.ts @@ -9,6 +9,7 @@ interface WsHandlers { const BACKOFF_BASE_MS = 500; const BACKOFF_CAP_MS = 30_000; +const REPLACED_BY_NEW_EXTENSION_CODE = 4001; // The browser WS API exposes no protocol ping frame to JS, so an app-level pong // is the only lever; sending one under the MV3 SW's ~30s idle timeout keeps the // worker alive as long as the socket stays open (chrome.alarms is the backstop @@ -86,11 +87,18 @@ export class ReconnectingWs { this.handlers.onCommand(parsed); }); - socket.addEventListener("close", () => { + socket.addEventListener("close", (event) => { this.clearHeartbeat(); if (this.socket === socket) this.socket = null; if (this.stopped) return; this.handlers.onClose?.(); + if (event.code === REPLACED_BY_NEW_EXTENSION_CODE) { + // The backend has selected another extension connection for this + // session. Reconnecting would make the two extensions evict each other. + this.stopped = true; + this.clearReconnect(); + return; + } this.scheduleReconnect(); }); diff --git a/apps/extension/src/driver/a11y.test.ts b/apps/extension/src/driver/a11y.test.ts index 386612e..a9adeae 100644 --- a/apps/extension/src/driver/a11y.test.ts +++ b/apps/extension/src/driver/a11y.test.ts @@ -109,7 +109,10 @@ function preorderRefs(tree: A11yNodeList): string[] { describe("buildA11ySnapshot", () => { it("keeps only meaningful, non-ignored nodes with a backend id; drops-but-descends", () => { - const { tree } = buildA11ySnapshot(FIXTURE, 4); + const { tree } = buildA11ySnapshot(FIXTURE, { + scopeId: "fixture", + generation: 4, + }); // Top level, in child order: button(2), link(4, re-parented off the dropped // ignored generic 3), textbox(6), heading(10). expect(tree.map((node) => node.role)).toEqual(["button", "link", "textbox", "heading"]); @@ -125,28 +128,56 @@ describe("buildA11ySnapshot", () => { expect(tree.some((node) => node.role === "link" && node.name === "Home")).toBe(true); }); - it("assigns deterministic s{gen}e{seq} refs in DFS pre-order", () => { - const first = buildA11ySnapshot(FIXTURE, 4); - const second = buildA11ySnapshot(FIXTURE, 4); + it("assigns deterministic attachment-and-generation-scoped refs in DFS pre-order", () => { + const scope = { scopeId: "fixture", generation: 4 }; + const first = buildA11ySnapshot(FIXTURE, scope); + const second = buildA11ySnapshot(FIXTURE, scope); // Pre-order: button(e0), link(e1), textbox(e2), the textbox's button(e3), heading(e4). - expect(preorderRefs(first.tree)).toEqual(["s4e0", "s4e1", "s4e2", "s4e3", "s4e4"]); + expect(preorderRefs(first.tree)).toEqual([ + "afixture:s4e0", + "afixture:s4e1", + "afixture:s4e2", + "afixture:s4e3", + "afixture:s4e4", + ]); // Same input -> identical refs across runs. expect(preorderRefs(second.tree)).toEqual(preorderRefs(first.tree)); }); + it("does not collide when two attachments mint the same generation and sequence", () => { + const first = buildA11ySnapshot(FIXTURE, { + scopeId: "tab-a", + generation: 1, + }); + const second = buildA11ySnapshot(FIXTURE, { + scopeId: "tab-b", + generation: 1, + }); + + expect(preorderRefs(first.tree)[0]).toBe("atab-a:s1e0"); + expect(preorderRefs(second.tree)[0]).toBe("atab-b:s1e0"); + expect(preorderRefs(first.tree)[0]).not.toBe(preorderRefs(second.tree)[0]); + }); + it("maps every ref to the correct backendDOMNodeId", () => { - const { refMap } = buildA11ySnapshot(FIXTURE, 4); + const { refMap } = buildA11ySnapshot(FIXTURE, { + scopeId: "fixture", + generation: 4, + }); expect([...refMap.entries()]).toEqual([ - ["s4e0", 100], - ["s4e1", 102], - ["s4e2", 104], - ["s4e3", 105], - ["s4e4", 107], + ["afixture:s4e0", 100], + ["afixture:s4e1", 102], + ["afixture:s4e2", 104], + ["afixture:s4e3", 105], + ["afixture:s4e4", 107], ]); }); it("reconstructs nested hierarchy among kept nodes", () => { - const { tree } = buildA11ySnapshot(FIXTURE, 4); + const { tree } = buildA11ySnapshot(FIXTURE, { + scopeId: "fixture", + generation: 4, + }); const textbox = tree.find((node) => node.role === "textbox"); expect(textbox?.children?.map((child) => ({ role: child.role, name: child.name }))).toEqual([ { role: "button", name: "Clear" }, @@ -157,7 +188,10 @@ describe("buildA11ySnapshot", () => { }); it("carries role/name/value and leaves a missing name undefined", () => { - const { tree } = buildA11ySnapshot(FIXTURE, 4); + const { tree } = buildA11ySnapshot(FIXTURE, { + scopeId: "fixture", + generation: 4, + }); const textbox = tree.find((node) => node.role === "textbox"); expect(textbox).toMatchObject({ role: "textbox", name: "Search", value: "hello" }); const heading = tree.find((node) => node.role === "heading"); @@ -166,7 +200,10 @@ describe("buildA11ySnapshot", () => { }); it("returns an empty tree and empty refMap for empty input", () => { - const { tree, refMap } = buildA11ySnapshot([], 4); + const { tree, refMap } = buildA11ySnapshot([], { + scopeId: "fixture", + generation: 4, + }); expect(tree).toEqual([]); expect(refMap.size).toBe(0); }); diff --git a/apps/extension/src/driver/a11y.ts b/apps/extension/src/driver/a11y.ts index 0446418..fe20a18 100644 --- a/apps/extension/src/driver/a11y.ts +++ b/apps/extension/src/driver/a11y.ts @@ -21,6 +21,15 @@ export const MEANINGFUL_ROLES: Set = new Set([ "cell", ]); +export interface A11yRefScope { + scopeId: string; + generation: number; +} + +export function a11yRefPrefix(scope: A11yRefScope): string { + return `a${scope.scopeId}:s${scope.generation}e`; +} + function axString(value: Protocol.Accessibility.AXValue | undefined): string | undefined { const raw = value?.value; return typeof raw === "string" ? raw : undefined; @@ -28,13 +37,14 @@ function axString(value: Protocol.Accessibility.AXValue | undefined): string | u export function buildA11ySnapshot( axNodes: Protocol.Accessibility.AXNode[], - gen: number, + scope: A11yRefScope, ): { tree: A11yNode[]; refMap: Map } { const refMap = new Map(); const byId = new Map(); for (const node of axNodes) byId.set(node.nodeId, node); const seen = new Set(); + const refPrefix = a11yRefPrefix(scope); let seq = 0; // DFS pre-order. Returns the kept forest rooted at `nodeId`: a kept node comes @@ -60,7 +70,7 @@ export function buildA11ySnapshot( !node.ignored && backendId !== undefined ) { - const ref = `s${gen}e${seq++}`; + const ref = `${refPrefix}${seq++}`; refMap.set(ref, backendId); self = { ref, role }; const name = axString(node.name); diff --git a/apps/extension/src/driver/cdp-events.test.ts b/apps/extension/src/driver/cdp-events.test.ts index f9657ff..2f9be84 100644 --- a/apps/extension/src/driver/cdp-events.test.ts +++ b/apps/extension/src/driver/cdp-events.test.ts @@ -2,19 +2,27 @@ import { describe, it, expect, vi } from "vitest"; import { applyDialogDecision, classifyCdpEvent, dialogDisposition } from "./cdp-events"; import type { DialogEventFields } from "./cdp-events"; -const ctx = { currentUrl: "https://example.com/current" }; +const ctx = { currentUrl: "https://example.com/current", mainFrameId: "F1" }; describe("classifyCdpEvent", () => { it("bumps generation, sets newUrl, and emits a navigated pageEvent for the main frame", () => { const decision = classifyCdpEvent( "Page.frameNavigated", - { frame: { id: "F1", url: "https://example.com/next" } }, + { + frame: { + id: "F1", + url: "https://example.com/next", + urlFragment: "#details", + }, + }, ctx, ); expect(decision).toEqual({ - newUrl: "https://example.com/next", + newMainFrameId: "F1", + newUrl: "https://example.com/next#details", bumpGeneration: true, - pageEvent: { kind: "navigated", url: "https://example.com/next" }, + loadStarted: true, + pageEvent: { kind: "navigated", url: "https://example.com/next#details" }, }); }); @@ -27,6 +35,39 @@ describe("classifyCdpEvent", () => { expect(decision).toEqual({}); }); + it("tracks a same-document navigation for the main frame without starting a load", () => { + const decision = classifyCdpEvent( + "Page.navigatedWithinDocument", + { + frameId: "F1", + url: "https://example.com/current#details", + navigationType: "fragment", + }, + ctx, + ); + expect(decision).toEqual({ + newUrl: "https://example.com/current#details", + bumpGeneration: true, + pageEvent: { + kind: "navigated", + url: "https://example.com/current#details", + }, + }); + }); + + it("ignores a same-document navigation from a subframe", () => { + const decision = classifyCdpEvent( + "Page.navigatedWithinDocument", + { + frameId: "F2", + url: "https://frame.example/#details", + navigationType: "fragment", + }, + ctx, + ); + expect(decision).toEqual({}); + }); + it("emits a load pageEvent carrying ctx.currentUrl, not a url from the event", () => { const decision = classifyCdpEvent("Page.loadEventFired", { timestamp: 12345 }, ctx); expect(decision).toEqual({ pageEvent: { kind: "load", url: "https://example.com/current" } }); diff --git a/apps/extension/src/driver/cdp-events.ts b/apps/extension/src/driver/cdp-events.ts index d9dad5a..170a369 100644 --- a/apps/extension/src/driver/cdp-events.ts +++ b/apps/extension/src/driver/cdp-events.ts @@ -12,7 +12,9 @@ export type DialogEventFields = Omit; // can branch on `decision.bumpGeneration` / `decision.pageEvent` / etc. export interface CdpDecision { bumpGeneration?: boolean; + loadStarted?: boolean; pageEvent?: { kind: "navigated" | "load"; url: string }; + newMainFrameId?: string; newUrl?: string; // A page dialog to answer locally, and optionally report. `accept` is how to // answer Page.handleJavaScriptDialog and is ALWAYS set when present, so an @@ -51,6 +53,15 @@ function asFrameNavigated(params: unknown): Protocol.Page.FrameNavigatedEvent | return params as Protocol.Page.FrameNavigatedEvent; } +function asNavigatedWithinDocument( + params: unknown, +): Protocol.Page.NavigatedWithinDocumentEvent | null { + if (typeof params !== "object" || params === null) return null; + const event = params as { frameId?: unknown; url?: unknown }; + if (typeof event.frameId !== "string" || typeof event.url !== "string") return null; + return params as Protocol.Page.NavigatedWithinDocumentEvent; +} + // Narrow a raw Page.javascriptDialogOpening payload to a KNOWN dialog type, or // null otherwise. DialogTypeSchema is the protocol's source of truth for the // four types we model; anything else (not expected from chrome.debugger) is @@ -74,15 +85,30 @@ function asDialogOpening(params: unknown): Omit + `a${TEST_SCOPE}:s${generation}e${sequence}`; +const ACTIONABLE_AX_TREE = [ + { + nodeId: "root", + ignored: false, + role: { type: "role", value: "RootWebArea" }, + childIds: ["button"], + }, + { + nodeId: "button", + ignored: false, + role: { type: "role", value: "button" }, + backendDOMNodeId: 42, + }, +]; + // Only the storage surface CdpSession.create touches; resolveRefCheck itself // must never reach the debugger or storage. function stubBrowserStorage(): void { @@ -15,6 +33,7 @@ function stubBrowserStorage(): void { } afterEach(() => { + vi.useRealTimers(); vi.unstubAllGlobals(); }); @@ -26,12 +45,12 @@ describe("CdpSession.resolveRefCheck", () => { it("answers ok:true from the live ref map without bumping the generation", async () => { // #given a session whose current generation holds the ref stubBrowserStorage(); - const session = await CdpSession.create(1); - session.refMap = new Map([["s0e1", 42]]); + const session = await CdpSession.create(1, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); const generationBefore = session.generation; // #when the ref is probed - const event = await session.resolveRefCheck("c1", "s0e1"); + const event = await session.resolveRefCheck("c1", testRef(0, 1)); // #then it resolves ok and the generation is untouched (probing must not // invalidate the consumer's outstanding refs) @@ -42,19 +61,19 @@ describe("CdpSession.resolveRefCheck", () => { it("answers ok:false for a stale-generation ref without bumping the generation", async () => { // #given a session that has never seen this ref's generation stubBrowserStorage(); - const session = await CdpSession.create(1); - session.refMap = new Map([["s0e1", 42]]); + const session = await CdpSession.create(1, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); const generationBefore = session.generation; // #when a ref from another generation is probed - const event = await session.resolveRefCheck("c2", "s9e9"); + const event = await session.resolveRefCheck("c2", testRef(9, 9)); // #then it reports stale without side effects expect(event).toEqual({ type: "action_result", commandId: "c2", ok: false, - error: "stale or unknown ref: s9e9", + error: `stale or unknown ref: ${testRef(9, 9)}`, }); expect(session.generation).toBe(generationBefore); }); @@ -62,18 +81,18 @@ describe("CdpSession.resolveRefCheck", () => { it("answers ok:false for a current-generation ref absent from the map", async () => { // #given a session whose current generation does not contain this ref stubBrowserStorage(); - const session = await CdpSession.create(1); - session.refMap = new Map([["s0e1", 42]]); + const session = await CdpSession.create(1, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); // #when a right-generation but unknown ref is probed - const event = await session.resolveRefCheck("c3", "s0e9"); + const event = await session.resolveRefCheck("c3", testRef(0, 9)); // #then it reports stale expect(event).toEqual({ type: "action_result", commandId: "c3", ok: false, - error: "stale or unknown ref: s0e9", + error: `stale or unknown ref: ${testRef(0, 9)}`, }); }); @@ -87,6 +106,13 @@ describe("CdpSession.resolveRefCheck", () => { releaseTree = resolve; }); } + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } return Promise.resolve({}); }); vi.stubGlobal("browser", { @@ -98,13 +124,13 @@ describe("CdpSession.resolveRefCheck", () => { }, debugger: { sendCommand }, }); - const session = await CdpSession.create(1); - session.refMap = new Map([["s0e1", 42]]); + const session = await CdpSession.create(1, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); const snapshotPromise = session.snapshotA11y("c-snap"); // #when a probe for the pre-bump ref is enqueued behind the snapshot, // which then completes (bumping to generation 1 and re-minting refs) - const probePromise = session.resolveRefCheck("c-probe", "s0e1"); + const probePromise = session.resolveRefCheck("c-probe", testRef(0, 1)); // The queued snapshot body starts on a microtask; wait until it has // actually issued the AX-tree fetch before releasing it. await vi.waitFor(() => { @@ -122,7 +148,494 @@ describe("CdpSession.resolveRefCheck", () => { type: "action_result", commandId: "c-probe", ok: false, - error: "stale or unknown ref: s0e1", + error: `stale or unknown ref: ${testRef(0, 1)}`, + }); + }); +}); + +describe("CdpSession snapshot target identity", () => { + function stubSnapshotBrowser( + sendCommand: ReturnType, + ): ReturnType { + const storageSet = vi.fn().mockResolvedValue(undefined); + vi.stubGlobal("browser", { + storage: { + session: { + get: vi.fn().mockResolvedValue({}), + set: storageSet, + }, + }, + debugger: { sendCommand }, + }); + return storageSet; + } + + it("returns the attached tab and bracketed main-frame URL with an accessibility snapshot", async () => { + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: "loader-1", + url: "https://example.com/about", + urlFragment: "#details", + }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") return Promise.resolve({ nodes: [] }); + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + + const event = await session.snapshotA11y("c-snapshot"); + + expect(event).toEqual({ + type: "snapshot_result", + commandId: "c-snapshot", + tree: [], + tabId: 7, + url: "https://example.com/about#details", + }); + expect(sendCommand.mock.calls.map((call) => call[1])).toEqual([ + "Page.getFrameTree", + "Accessibility.getFullAXTree", + "Page.getFrameTree", + ]); + }); + + it("fails closed when the attached page redirects while its tree is captured", async () => { + let frameReads = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + frameReads += 1; + return Promise.resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: `loader-${frameReads}`, + url: + frameReads === 1 + ? "https://example.com/" + : "https://evil.example/redirected", + }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") return Promise.resolve({ nodes: [] }); + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); + + const event = await session.snapshotA11y("c-redirect"); + + expect(event).toEqual({ + type: "action_result", + commandId: "c-redirect", + ok: false, + error: "page changed during snapshot", + }); + expect(session.refMap.size).toBe(0); + expect(session.generation).toBe(1); + }); + + it("fails closed when only the main-frame URL fragment changes during capture", async () => { + let frameReads = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + frameReads += 1; + return Promise.resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: "loader-1", + url: "https://example.com/", + urlFragment: frameReads === 1 ? "#before" : "#after", + }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") return Promise.resolve({ nodes: [] }); + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + + await expect(session.snapshotA11y("c-fragment")).resolves.toEqual({ + type: "action_result", + commandId: "c-fragment", + ok: false, + error: "page changed during snapshot", + }); + expect(session.generation).toBe(1); + }); + + it("fails closed when the main document changes at the same URL before its event arrives", async () => { + let frameReads = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + frameReads += 1; + return Promise.resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: `loader-${frameReads}`, + url: "https://example.com/", + }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") return Promise.resolve({ nodes: [] }); + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + + await expect(session.snapshotA11y("c-loader")).resolves.toEqual({ + type: "action_result", + commandId: "c-loader", + ok: false, + error: "page changed during snapshot", + }); + }); + + it("fails closed when the document generation changes at the same URL during capture", async () => { + let releaseTree!: (value: { nodes: unknown[] }) => void; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") { + return new Promise((resolve) => { + releaseTree = resolve; + }); + } + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + const snapshot = session.snapshotA11y("c-generation"); + await vi.waitFor(() => { + expect( + sendCommand.mock.calls.some((call) => call[1] === "Accessibility.getFullAXTree"), + ).toBe(true); + }); + + await session.bumpGeneration(); + releaseTree({ nodes: [] }); + + await expect(snapshot).resolves.toEqual({ + type: "action_result", + commandId: "c-generation", + ok: false, + error: "page changed during snapshot", + }); + }); + + it("invalidates prior refs when the artifact capture fails before the closing identity read", async () => { + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") { + return Promise.reject(new Error("AX capture failed")); + } + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); + + await expect(session.snapshotA11y("c-capture-failed")).resolves.toEqual({ + type: "action_result", + commandId: "c-capture-failed", + ok: false, + error: "AX capture failed", + }); + expect(session.refMap.size).toBe(0); + expect(session.generation).toBe(1); + }); + + it("invalidates prior refs when the closing identity read fails", async () => { + let frameReads = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + frameReads += 1; + if (frameReads === 2) return Promise.reject(new Error("identity read failed")); + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") return Promise.resolve({ nodes: [] }); + return Promise.resolve({}); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); + + await expect(session.snapshotA11y("c-identity-failed")).resolves.toEqual({ + type: "action_result", + commandId: "c-identity-failed", + ok: false, + error: "identity read failed", + }); + expect(session.refMap.size).toBe(0); + expect(session.generation).toBe(1); + }); + + it("does not publish refs when generation changes while the snapshot generation persists", async () => { + let releaseFirstWrite!: () => void; + let storageWrites = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") { + return Promise.resolve({ nodes: ACTIONABLE_AX_TREE }); + } + return Promise.resolve({}); + }); + vi.stubGlobal("browser", { + storage: { + session: { + get: vi.fn().mockResolvedValue({}), + set: vi.fn().mockImplementation(() => { + storageWrites += 1; + if (storageWrites !== 1) return Promise.resolve(); + return new Promise((resolve) => { + releaseFirstWrite = resolve; + }); + }), + }, + }, + debugger: { sendCommand }, + }); + const session = await CdpSession.create(7, TEST_SCOPE); + const snapshot = session.snapshotA11y("c-persist-race"); + await vi.waitFor(() => { + expect(storageWrites).toBe(1); + }); + + const navigationBump = session.bumpGeneration(); + releaseFirstWrite(); + + await expect(snapshot).resolves.toEqual({ + type: "action_result", + commandId: "c-persist-race", + ok: false, + error: "page changed during snapshot", + }); + await navigationBump; + expect(session.refMap.size).toBe(0); + expect(session.generation).toBe(2); + }); + + it("applies one 25s deadline across sequential identity, capture, persistence, and identity work", async () => { + vi.useFakeTimers(); + let calls = 0; + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + calls += 1; + const call = calls; + return new Promise((resolve, reject) => { + setTimeout(() => { + if (call === 3) { + reject(new Error("late closing identity failure")); + return; + } + if (method === "Page.getFrameTree") { + resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: "loader-1", + url: "https://example.com/", + }, + }, + }); + return; + } + resolve({ nodes: ACTIONABLE_AX_TREE }); + }, 10_000); + }); + }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + session.refMap = new Map([[testRef(0, 1), 42]]); + const snapshot = session.snapshotA11y("c-aggregate-timeout"); + + await vi.advanceTimersByTimeAsync(25_000); + + await expect(snapshot).resolves.toEqual({ + type: "action_result", + commandId: "c-aggregate-timeout", + ok: false, + error: "snapshot timed out after 25000ms", + }); + expect(sendCommand.mock.calls.map((call) => call[1])).toEqual([ + "Page.getFrameTree", + "Accessibility.getFullAXTree", + "Page.getFrameTree", + ]); + expect(session.refMap.size).toBe(0); + + // Let the losing browser promise reject after the aggregate race. Vitest + // treats an unhandled rejection as a test failure. + await vi.advanceTimersByTimeAsync(5_000); + }); + + it("spends the snapshot deadline while waiting in the CDP FIFO and never starts an expired capture", async () => { + vi.useFakeTimers(); + const sendCommand = vi.fn(); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(7, TEST_SCOPE); + const earlier = session.wait("c-earlier", "ms", 30_000); + const snapshot = session.snapshotA11y("c-queued-timeout"); + + await vi.advanceTimersByTimeAsync(25_000); + + await expect(snapshot).resolves.toEqual({ + type: "action_result", + commandId: "c-queued-timeout", + ok: false, + error: "snapshot timed out after 25000ms", + }); + expect(sendCommand).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(5_000); + await earlier; + await Promise.resolve(); + expect(sendCommand).not.toHaveBeenCalled(); + }); + + it("binds screenshots to the same attached target identity", async () => { + const sendCommand = vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { id: "frame-1", loaderId: "loader-1", url: "https://example.com/" }, + }, + }); + } + if (method === "Page.captureScreenshot") return Promise.resolve({ data: "QQ==" }); + return Promise.resolve({}); }); + stubSnapshotBrowser(sendCommand); + const session = await CdpSession.create(9, TEST_SCOPE); + + await expect(session.screenshot("c-shot")).resolves.toEqual({ + type: "screenshot_result", + commandId: "c-shot", + mime: "image/png", + b64: "QQ==", + tabId: 9, + url: "https://example.com/", + }); + }); +}); + +describe("CdpSession ref target binding", () => { + function stubActionableSnapshotBrowser(): void { + vi.stubGlobal("browser", { + storage: { + session: { + get: vi.fn().mockResolvedValue({}), + set: vi.fn().mockResolvedValue(undefined), + }, + }, + debugger: { + sendCommand: vi.fn().mockImplementation((_target, method: string) => { + if (method === "Page.getFrameTree") { + return Promise.resolve({ + frameTree: { + frame: { + id: "frame-1", + loaderId: "loader-1", + url: "https://example.com/", + }, + }, + }); + } + if (method === "Accessibility.getFullAXTree") { + return Promise.resolve({ nodes: ACTIONABLE_AX_TREE }); + } + return Promise.resolve({}); + }), + }, + }); + } + + it("does not resolve a same-generation ref minted by another attachment", async () => { + stubActionableSnapshotBrowser(); + const first = await CdpSession.create(7, "attachment-a"); + const second = await CdpSession.create(8, "attachment-b"); + const firstEvent = await first.snapshotA11y("c-first"); + const secondEvent = await second.snapshotA11y("c-second"); + if (firstEvent.type !== "snapshot_result" || secondEvent.type !== "snapshot_result") { + throw new Error("expected snapshot results"); + } + const firstRef = firstEvent.tree[0]?.ref; + const secondRef = secondEvent.tree[0]?.ref; + + expect(firstRef).toBe("aattachment-a:s1e0"); + expect(secondRef).toBe("aattachment-b:s1e0"); + expect(second.resolveRef(firstRef ?? "")).toBeNull(); + expect(second.resolveRef(secondRef ?? "")).toBe(42); + }); + + it("rotates the ref namespace and invalidates the old map at a WS-session barrier", async () => { + stubActionableSnapshotBrowser(); + const session = await CdpSession.create(7, "session-a"); + const firstEvent = await session.snapshotA11y("c-first"); + if (firstEvent.type !== "snapshot_result") throw new Error("expected snapshot result"); + const oldRef = firstEvent.tree[0]?.ref ?? ""; + + await session.invalidateRefsForSessionChange("session-b"); + + expect(session.resolveRef(oldRef)).toBeNull(); + expect(session.refMap.size).toBe(0); + + const secondEvent = await session.snapshotA11y("c-second"); + if (secondEvent.type !== "snapshot_result") throw new Error("expected snapshot result"); + expect(secondEvent.tree[0]?.ref).toBe("asession-b:s3e0"); + }); + + it("finishes session invalidation even when generation persistence never settles", async () => { + vi.stubGlobal("browser", { + storage: { + session: { + get: vi.fn().mockResolvedValue({}), + set: vi.fn(() => new Promise(() => {})), + }, + }, + }); + const session = await CdpSession.create(7, "session-a"); + const oldRef = "asession-a:s0e0"; + session.refMap = new Map([[oldRef, 42]]); + + await expect( + session.invalidateRefsForSessionChange("session-b"), + ).resolves.toBeUndefined(); + + expect(session.generation).toBe(1); + expect(session.refMap.size).toBe(0); + expect(session.resolveRef(oldRef)).toBeNull(); }); }); diff --git a/apps/extension/src/driver/cdp.ts b/apps/extension/src/driver/cdp.ts index b3cb7f5..e37ce00 100644 --- a/apps/extension/src/driver/cdp.ts +++ b/apps/extension/src/driver/cdp.ts @@ -1,7 +1,7 @@ import type { Event } from "@understudy/protocol"; import type { Protocol } from "devtools-protocol"; import { actionError, errorMessage } from "../events"; -import { buildA11ySnapshot } from "./a11y"; +import { a11yRefPrefix, buildA11ySnapshot } from "./a11y"; import { parseKeys } from "./keymap"; type WaitFor = "load" | "idle" | "ms"; @@ -11,6 +11,10 @@ type WaitFor = "load" | "idle" | "ms"; const SEND_TIMEOUT_MS = 15000; const LOAD_TIMEOUT_MS = 15000; const IDLE_QUIET_MS = 500; +// The backend coordinator abandons commands at 30s. One budget covers the +// entire identity/capture/persist/identity bracket so its sequential CDP calls +// cannot each consume their independent 15s send timeout. +const SNAPSHOT_DEADLINE_MS = 25_000; function isBoxModelError(cause: unknown): boolean { return errorMessage(cause).includes("Could not compute box model"); @@ -40,6 +44,7 @@ export class CdpSession { generation = 0; refMap: Map = new Map(); currentUrl = ""; + mainFrameId = ""; private loadInFlight = false; private readonly loadWaiters = new Set<() => void>(); @@ -48,10 +53,16 @@ export class CdpSession { // in order instead of racing to overwrite browser.storage.session. private genPersistChain: Promise = Promise.resolve(); - private constructor(readonly tabId: number) {} + private constructor( + readonly tabId: number, + private refScopeId: string, + ) {} - static async create(tabId: number): Promise { - const session = new CdpSession(tabId); + static async create( + tabId: number, + refScopeId: string = crypto.randomUUID(), + ): Promise { + const session = new CdpSession(tabId, refScopeId); await session.loadGeneration(); return session; } @@ -69,6 +80,7 @@ export class CdpSession { bumpGeneration(): Promise { this.generation += 1; + this.refMap.clear(); const value = this.generation; const write = this.genPersistChain.then(() => browser.storage.session.set({ [CdpSession.genKey(this.tabId)]: value }), @@ -117,6 +129,9 @@ export class CdpSession { await this.send("DOM.enable"); await this.send("Page.enable"); await this.send("Runtime.enable"); + const identity = await this.mainFrameIdentity(); + this.mainFrameId = identity.frameId; + this.currentUrl = identity.url; this.enabled = true; } @@ -129,10 +144,24 @@ export class CdpSession { // Generation-namespaced refs (see driver/a11y.ts) make staleness detectable: // a ref from a prior snapshot generation fails the prefix check below. resolveRef(ref: string): number | null { - if (!ref.startsWith(`s${this.generation}e`)) return null; + const prefix = a11yRefPrefix({ + scopeId: this.refScopeId, + generation: this.generation, + }); + if (!ref.startsWith(prefix)) return null; return this.refMap.get(ref) ?? null; } + invalidateRefsForSessionChange(nextScopeId: string = crypto.randomUUID()): Promise { + return this.enqueue(async () => { + this.refScopeId = nextScopeId; + // Scope rotation + the synchronous generation increment/refMap clear are + // the security boundary. A stuck storage.session write must not prevent + // the WebSocket session barrier from connecting its replacement peer. + void this.bumpGeneration().catch(() => {}); + }); + } + markLoadStarted(): void { this.loadInFlight = true; } @@ -170,14 +199,59 @@ export class CdpSession { return result; } - private run(commandId: string, body: () => Promise): Promise { - return this.enqueue(async () => { + private run( + commandId: string, + body: () => Promise, + deadlineAt?: number, + ): Promise { + let started = false; + let expiredInQueue = false; + const timeoutEvent = (): Event => ({ + type: "action_result", + commandId, + ok: false, + error: `snapshot timed out after ${SNAPSHOT_DEADLINE_MS}ms`, + }); + const execution = this.enqueue(async () => { + if ( + expiredInQueue || + (deadlineAt !== undefined && Date.now() >= deadlineAt) + ) { + return timeoutEvent(); + } + started = true; try { return await body(); } catch (cause) { return { type: "action_result", commandId, ok: false, error: errorMessage(cause) }; } }); + if (deadlineAt === undefined) return execution; + + const remainingMs = deadlineAt - Date.now(); + if (remainingMs <= 0) { + expiredInQueue = true; + void execution.catch(() => {}); + return Promise.resolve(timeoutEvent()); + } + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + if (!started) { + expiredInQueue = true; + resolve(timeoutEvent()); + } + }, remainingMs); + void execution.then( + (event) => { + clearTimeout(timer); + resolve(event); + }, + (cause: unknown) => { + clearTimeout(timer); + reject(cause); + }, + ); + }); } private async optional(action: Promise): Promise { @@ -188,6 +262,101 @@ export class CdpSession { } } + private async mainFrameIdentity(deadlineAt?: number): Promise<{ + frameId: string; + loaderId: string; + url: string; + }> { + const read = (): Promise => + this.send("Page.getFrameTree"); + const { frameTree } = + deadlineAt === undefined + ? await read() + : await this.withSnapshotDeadline(deadlineAt, read); + const frame = frameTree.frame; + return { + frameId: frame.id, + loaderId: frame.loaderId, + url: `${frame.url}${frame.urlFragment ?? ""}`, + }; + } + + private invalidateIncompleteSnapshot(generation: number): void { + this.refMap.clear(); + if (this.generation !== generation) return; + // bumpGeneration mutates the security boundary synchronously. Persistence + // remains best-effort and must not extend an already-expired snapshot past + // the coordinator's response deadline. + void this.bumpGeneration().catch(() => {}); + } + + private async withSnapshotDeadline( + deadlineAt: number, + start: () => Promise, + ): Promise { + const remainingMs = deadlineAt - Date.now(); + if (remainingMs <= 0) { + throw new Error(`snapshot timed out after ${SNAPSHOT_DEADLINE_MS}ms`); + } + const operation = start(); + let timer: ReturnType | undefined; + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout(() => { + reject(new Error(`snapshot timed out after ${SNAPSHOT_DEADLINE_MS}ms`)); + }, remainingMs); + }); + try { + // Promise.race installs rejection handlers on both inputs, so a browser + // API promise that rejects after the aggregate deadline is still handled. + return await Promise.race([operation, timeout]); + } finally { + clearTimeout(timer); + } + } + + /** + * Binds a snapshot artifact to one stable main document. CDP events can run + * while a queued command awaits a response, so main-frame identity reads + * bracket the capture and the generation catches same-document DOM changes. + * A changed page invalidates the prior ref map and yields no snapshot result + * for a consumer to trust. + */ + private async captureStableSnapshot( + deadlineAt: number, + capture: () => Promise, + mintGeneration = false, + ): Promise<{ captured: T; generation: number; url: string }> { + const baselineGeneration = this.generation; + try { + const before = await this.mainFrameIdentity(deadlineAt); + const captured = await this.withSnapshotDeadline(deadlineAt, capture); + let generation = baselineGeneration; + if (mintGeneration) { + if (this.generation !== baselineGeneration) { + throw new Error("page changed during snapshot"); + } + generation = await this.withSnapshotDeadline(deadlineAt, () => + this.bumpGeneration(), + ); + } + const after = await this.mainFrameIdentity(deadlineAt); + if ( + this.generation !== generation || + after.frameId !== before.frameId || + after.loaderId !== before.loaderId || + after.url !== before.url + ) { + throw new Error("page changed during snapshot"); + } + this.mainFrameId = after.frameId; + this.currentUrl = after.url; + return { captured, generation, url: after.url }; + } catch (cause) { + this.invalidateIncompleteSnapshot(baselineGeneration); + throw cause; + } + } + private async dispatchClick(backendNodeId: number): Promise { await this.optional(this.send("DOM.scrollIntoViewIfNeeded", { backendNodeId })); let model: Protocol.DOM.BoxModel; @@ -246,15 +415,27 @@ export class CdpSession { } snapshotA11y(commandId: string): Promise { - return this.run(commandId, async () => { - const { nodes } = await this.send( - "Accessibility.getFullAXTree", - ); - const gen = await this.bumpGeneration(); - const { tree, refMap } = buildA11ySnapshot(nodes, gen); - this.refMap = refMap; - return { type: "snapshot_result", commandId, tree }; - }); + const deadlineAt = Date.now() + SNAPSHOT_DEADLINE_MS; + return this.run( + commandId, + async () => { + const { captured, generation, url } = await this.captureStableSnapshot( + deadlineAt, + () => + this.send( + "Accessibility.getFullAXTree", + ), + true, + ); + const { tree, refMap } = buildA11ySnapshot(captured.nodes, { + scopeId: this.refScopeId, + generation, + }); + this.refMap = refMap; + return { type: "snapshot_result", commandId, tree, tabId: this.tabId, url }; + }, + deadlineAt, + ); } // Pure ref-map lookup: MUST NOT snapshot or bump the generation. This is @@ -271,13 +452,29 @@ export class CdpSession { } screenshot(commandId: string): Promise { - return this.run(commandId, async () => { - const { data } = await this.send( - "Page.captureScreenshot", - { format: "png" }, - ); - return { type: "screenshot_result", commandId, mime: "image/png", b64: data }; - }); + const deadlineAt = Date.now() + SNAPSHOT_DEADLINE_MS; + return this.run( + commandId, + async () => { + const { captured, url } = await this.captureStableSnapshot(deadlineAt, () => + this.send( + "Page.captureScreenshot", + { + format: "png", + }, + ), + ); + return { + type: "screenshot_result", + commandId, + mime: "image/png", + b64: captured.data, + tabId: this.tabId, + url, + }; + }, + deadlineAt, + ); } click(commandId: string, ref: string): Promise { diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 17c2e3c..c6fadda 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -1,8 +1,10 @@ import { safeParseCommand } from "@understudy/protocol"; import type { Browser } from "wxt/browser"; -import { ReconnectingWs } from "../core/ws-client"; +import { CommandIngress, type StartedCommand } from "../core/command-ingress"; import { WriteDedupe } from "../core/dedupe"; +import { sendIfPeerCurrent } from "../core/peer-binding"; import { routeCommand } from "../core/router"; +import { ReconnectingWs } from "../core/ws-client"; import { CdpSession } from "../driver/cdp"; import { applyDialogDecision, classifyCdpEvent } from "../driver/cdp-events"; import { errorMessage } from "../events"; @@ -29,6 +31,10 @@ const LOG_CAP = 50; // WXT re-runs main() when the service worker is revived, so these are re-created // from scratch each wake; durable state lives in browser.storage.{local,session}. let ws: ReconnectingWs | null = null; +// Input acceptance is retired independently from the socket itself. A URL +// switch stops admitting messages immediately, but keeps the old peer alive +// until every command already admitted through CommandIngress has replied. +let acceptingPeer: ReconnectingWs | null = null; let wsConnecting = false; // Tracked from ReconnectingWs's onConnecting/onOpen/onClose callbacks. let wsStatus: WsStatus = "connecting"; @@ -39,6 +45,10 @@ let currentWsUrl = DEFAULT_WS_URL; // after it and clobber a newer in-memory URL with a stale (or unwritten) disk copy. let wsUrlHydrated = false; let wsUrlEpoch = 0; +let wsSwitching = false; +let wsSwitchRequest = 0; +let requestedWsUrl: string | null = null; +let wsSwitchTail: Promise = Promise.resolve(); let session: CdpSession | null = null; let attachedTitle: string | undefined; @@ -46,6 +56,7 @@ let attachedTitle: string | undefined; // Write-replay record (idempotent-retry contract); hydrates lazily from // storage.session, so rebuilding it each wake loses nothing. const dedupe = new WriteDedupe(browser.storage.session); +const commandIngress = new CommandIngress(); const logBuffer: LogEntry[] = []; const ports = new Set(); @@ -89,7 +100,7 @@ async function readWsUrl(): Promise { } async function ensureConnection(): Promise { - if (ws !== null || wsConnecting) return; + if (ws !== null || wsConnecting || wsSwitching) return; wsConnecting = true; try { if (!wsUrlHydrated) { @@ -102,7 +113,7 @@ async function ensureConnection(): Promise { wsUrlHydrated = true; } } - if (ws === null) { + if (ws === null && !wsSwitching) { connectWs(); } } finally { @@ -111,14 +122,23 @@ async function ensureConnection(): Promise { } function connectWs(): void { - ws = new ReconnectingWs(getUrl, { onCommand, onOpen, onClose, onConnecting }); + let peer!: ReconnectingWs; + peer = new ReconnectingWs(getUrl, { + onCommand: (raw) => onCommand(raw, peer), + onOpen: () => onOpen(peer), + onClose: () => onClose(peer), + onConnecting, + }); + ws = peer; + acceptingPeer = peer; } // ReconnectingWs starts its own pong heartbeat on open, so we only (re)send hello. -function onOpen(): void { +function onOpen(peer: ReconnectingWs): void { + if (peer !== ws || wsSwitching) return; wsStatus = "open"; log("ws connected"); - fireAndForget("hello", sendHello); + fireAndForget("hello", () => sendHello(peer)); broadcastState(); } @@ -127,36 +147,45 @@ function onConnecting(): void { broadcastState(); } -function onClose(): void { +function onClose(peer: ReconnectingWs): void { + if (peer !== ws) return; wsStatus = "closed"; broadcastState(); } // A fresh hello on every (re)connect is the resync signal: any commands in flight // when the SW was evicted are abandoned, and the peer tolerates repeated hellos. -async function sendHello(): Promise { +async function sendHello(peer: ReconnectingWs): Promise { const tabs = await queryTabInfos(); - ws?.send({ - type: "hello", - browser: navigator.userAgent, - extVersion: browser.runtime.getManifest().version, - tabs, + sendIfPeerCurrent(peer, acceptingPeer, (current) => { + current.send({ + type: "hello", + browser: navigator.userAgent, + extVersion: browser.runtime.getManifest().version, + tabs, + }); }); } -function onCommand(raw: unknown): void { - fireAndForget("command", () => handleCommand(raw)); +function onCommand(raw: unknown, peer: ReconnectingWs): void { + if (peer !== acceptingPeer) return; + fireAndForget("command ingress", () => + commandIngress.enqueue(() => startCommand(raw, peer)), + ); } -async function handleCommand(raw: unknown): Promise { +async function startCommand( + raw: unknown, + peer: ReconnectingWs, +): Promise { const parsed = safeParseCommand(raw); if (!parsed.success) { log(`invalid command dropped: ${parsed.error.message}`, "warn"); const commandId = extractCommandId(raw); if (commandId !== null) { - ws?.send({ type: "action_result", commandId, ok: false, error: "invalid command" }); + peer.send({ type: "action_result", commandId, ok: false, error: "invalid command" }); } - return; + return undefined; } // Idempotent-retry gate for WRITE commands (reads always execute). A retry @@ -167,25 +196,30 @@ async function handleCommand(raw: unknown): Promise { const decision = await dedupe.claim(parsed.data); if (decision.kind === "replay") { log(`replayed recorded result for duplicate write ${parsed.data.commandId}`); - ws?.send(decision.event); - return; + peer.send(decision.event); + return undefined; } if (decision.kind === "drop") { log(`dropped duplicate in-flight write ${parsed.data.commandId}; the original execution will answer`); - return; + return undefined; } - try { - const ev = await routeCommand(parsed.data, session); - // Record before sending: once the write executed, a crash between the two - // must leave the record (a replayable result), not a re-executable gap. - await dedupe.remember(parsed.data, ev); - ws?.send(ev); - } finally { - // No-op once remember() cleared the mark; guarantees a thrown execution - // still frees its in-flight slot so a later retry can re-run it. - dedupe.release(parsed.data); - } + const activeSession = session; + const completion = (async () => { + try { + const ev = await routeCommand(parsed.data, activeSession); + // Record before sending: once the write executed, a crash between the two + // must leave the record (a replayable result), not a re-executable gap. + await dedupe.remember(parsed.data, ev); + peer.send(ev); + } finally { + // No-op once remember() cleared the mark; guarantees a thrown execution + // still frees its in-flight slot so a later retry can re-run it. + dedupe.release(parsed.data); + } + })(); + fireAndForget("command execution", async () => completion); + return { completion }; } function extractCommandId(raw: unknown): string | null { @@ -208,26 +242,37 @@ async function onCdpEvent( ): Promise { const active = session; if (active === null || source.tabId !== active.tabId) return; + const eventPeer = acceptingPeer; try { - const decision = classifyCdpEvent(method, params, { currentUrl: active.currentUrl }); + const decision = classifyCdpEvent(method, params, { + currentUrl: active.currentUrl, + mainFrameId: active.mainFrameId, + }); + if (decision.newMainFrameId !== undefined) { + active.mainFrameId = decision.newMainFrameId; + } if (decision.newUrl !== undefined) { active.currentUrl = decision.newUrl; } - if (decision.pageEvent?.kind === "navigated") { + if (decision.loadStarted === true) { active.markLoadStarted(); - await active.bumpGeneration(); - } else if (decision.bumpGeneration === true) { + } + if (decision.bumpGeneration === true) { await active.bumpGeneration(); } + if (session !== active) return; if (decision.pageEvent?.kind === "load") { active.notifyLoadEventFired(); } - if (decision.pageEvent !== undefined) { - ws?.send({ - type: "page_event", - kind: decision.pageEvent.kind, - tabId: active.tabId, - url: decision.pageEvent.url, + const pageEvent = decision.pageEvent; + if (pageEvent !== undefined) { + sendIfPeerCurrent(eventPeer, acceptingPeer, (current) => { + current.send({ + type: "page_event", + kind: pageEvent.kind, + tabId: active.tabId, + url: pageEvent.url, + }); }); } if (decision.dialog !== undefined) { @@ -238,7 +283,12 @@ async function onCdpEvent( await applyDialogDecision( decision.dialog, (accept) => active.send("Page.handleJavaScriptDialog", { accept }), - (event) => ws?.send({ type: "dialog", tabId: active.tabId, ...event }), + (event) => { + if (session !== active) return; + sendIfPeerCurrent(eventPeer, acceptingPeer, (current) => { + current.send({ type: "dialog", tabId: active.tabId, ...event }); + }); + }, ); log( `handled ${decision.dialog.event?.dialogType ?? "unknown"} dialog: ${ @@ -290,7 +340,6 @@ async function attach(): Promise { throw cause; } } - next.currentUrl = tab.url ?? ""; session = next; attachedTitle = tab.title; await persistAttachedTabId(tabId); @@ -350,7 +399,6 @@ async function reconcileAttachment(): Promise { if (target !== undefined && target.attached) { const next = await CdpSession.create(tabId); await next.reconcile(); - next.currentUrl = target.url; session = next; attachedTitle = target.title; log(`reconciled attachment to tab ${tabId}`); @@ -373,27 +421,56 @@ async function persistAttachedTabId(tabId: number): Promise { } async function setWsUrl(url: string): Promise { - // A different WS URL means a different session (the sessionId is in the URL - // path): drop the write-replay record so a reused idempotency key can't - // replay the previous session's result. - if (url !== currentWsUrl) await dedupe.clear(); - currentWsUrl = url; + const previousRequestedUrl = requestedWsUrl ?? currentWsUrl; + const sessionChanged = url !== previousRequestedUrl; + requestedWsUrl = url; + const request = ++wsSwitchRequest; + wsSwitching = true; wsUrlHydrated = true; wsUrlEpoch += 1; + wsStatus = "connecting"; + const oldPeer = acceptingPeer; + acceptingPeer = null; + broadcastState(); + + const change = wsSwitchTail.then(async () => { + await commandIngress.barrier(async () => { + oldPeer?.stop(); + if (ws === oldPeer) ws = null; + if (!sessionChanged) return; + // The session id lives in the URL. Wait for every command accepted from + // the old peer before clearing replay state and rotating the ref scope; + // this prevents an old snapshot from repopulating refs after invalidation. + await dedupe.clear(); + const active = session; + if (active !== null) { + try { + await active.invalidateRefsForSessionChange(); + } catch (cause) { + log(`persist ref invalidation failed: ${errorMessage(cause)}`, "warn"); + } + } + }); + currentWsUrl = url; + try { + await browser.storage.local.set({ [WS_URL_KEY]: url }); + } catch (cause) { + log(`persist wsUrl failed: ${errorMessage(cause)}`, "warn"); + } + log(`ws url set to ${url}; reconnecting`); + }); + wsSwitchTail = change.then( + () => undefined, + () => undefined, + ); try { - await browser.storage.local.set({ [WS_URL_KEY]: url }); - } catch (cause) { - log(`persist wsUrl failed: ${errorMessage(cause)}`, "warn"); - } - // Tear down and rebuild directly against the now-authoritative in-memory URL — - // routing through ensureConnection would re-read storage, which can be stale - // (e.g. the write above just failed) and would clobber this value. - if (ws !== null) { - ws.stop(); - ws = null; + await change; + } finally { + if (request === wsSwitchRequest) { + wsSwitching = false; + connectWs(); + } } - log(`ws url set to ${url}; reconnecting`); - connectWs(); } // ── Panel Port host ────────────────────────────────────────────────────────── diff --git a/docs/technical-plan.md b/docs/technical-plan.md index 9b9736f..363e11d 100644 --- a/docs/technical-plan.md +++ b/docs/technical-plan.md @@ -100,11 +100,11 @@ the substrate role. ┌───────────────────── UNDERSTUDY SERVICE (Cloudflare Worker) ─────────────────┐ │ Hono: /v1/sessions/:id/commands · session mgmt · /auth · /health │ │ SessionAgent extends Agent (one Durable Object per SESSION, per tenant/case)│ -│ • terminates the extension WebSocket, holds the CDP session + refMap gen │ +│ • terminates one authoritative extension WebSocket and tracks session state│ │ • coordinator.send(cmd) ─ WS ▶ extension ─ Event ▶ resolves pending map │ │ • fill_secret: resolve secretRef (vault) → type plaintext service-side │ └───────────────────────────────┬──────────────────────────────────────────────┘ - wss:///session/:sessionId (auth token) + wss:///agents/session/:sessionId (auth token) ▼ ┌───────────────────── User's real Chrome (Chromium) — the EXTENSION (M2) ─────┐ │ Background SW: WS client · keepalive · command router · CDP session manager │ @@ -151,10 +151,10 @@ understudy/ The stable IP: shared across the service, the extension, AND consumer connectors; browser-agnostic; **published as `@understudy/protocol` on zod 4**. Every message is a discriminated union tagged by -`type`, carries a `commandId` for request/response correlation, and is validated with zod at every -boundary. +`type` and is validated with zod at every boundary. Commands and their correlated results carry a +`commandId`; connection, page, and heartbeat events do not. -> **Target contract.** The union below is the shape to converge on; `fill_secret`, `tabs_result`, the public publish, and the zod-4 bump land as milestone follow-ons (see Milestones — `tabs_result` + the zod-4 bump at M2, `fill_secret` + publish at M3). +> **Target contract.** The union below is the current public wire shape. Protocol 0.6 makes the snapshot target fields mandatory, so the service and extension must upgrade in the same rollout. ```ts // Consumer/Service → Extension @@ -166,6 +166,7 @@ export type Command = | { type: "key"; commandId: string; keys: string; ref?: string } | { type: "scroll"; commandId: string; ref?: string; dy: number } | { type: "wait"; commandId: string; for: "load" | "idle" | "ms"; value?: number } + | { type: "resolve_ref"; commandId: string; ref: string } | { type: "get_tabs"; commandId: string } | { type: "switch_tab"; commandId: string; tabId: number } | { type: "fill_secret"; commandId: string; ref: string; secretRef: string; submit?: boolean }; @@ -173,11 +174,12 @@ export type Command = // Extension → Service export type Event = | { type: "hello"; browser: string; extVersion: string; tabs: TabInfo[] } - | { type: "snapshot_result"; commandId: string; tree: A11yNode[] } - | { type: "screenshot_result"; commandId: string; mime: string; b64: string } - | { type: "action_result"; commandId: string; ok: boolean; error?: string; url?: string } + | { type: "snapshot_result"; commandId: string; tree: A11yNode[]; tabId: number; url: string } + | { type: "screenshot_result"; commandId: string; mime: string; b64: string; tabId: number; url: string } + | { type: "action_result"; commandId: string; ok: boolean; error?: string; url?: string; simulated?: boolean } | { type: "tabs_result"; commandId: string; tabs: TabInfo[] } | { type: "page_event"; kind: "navigated" | "load"; tabId: number; url: string } + | { type: "dialog"; tabId: number; dialogType: "alert" | "confirm" | "prompt" | "beforeunload"; message: string; url: string; defaultPrompt?: string; disposition: "accept" | "dismiss" } | { type: "pong" }; export interface A11yNode { ref: string; role: string; name?: string; value?: string; children?: A11yNode[] } @@ -185,8 +187,14 @@ export interface TabInfo { tabId: number; url: string; title: string; active: bo ``` Design notes: -- `ref` is the *only* thing the consumer's agent ever uses to address an element. Opaque to the - model; resolved by the extension (see targeting). +- `ref` is the *only* thing the consumer's agent ever uses to address an element. Its encoding is + opaque. The extension binds it to the extension session, exact CDP attachment, and snapshot + generation that minted it, then resolves it locally (see targeting). +- Snapshot results include the exact attached CDP `tabId` and main-frame `url` captured with the + artifact. Consumers validate those fields instead of inferring the session target from + `TabInfo.active`, which is true once per browser window. An optional command `tabId` is an + expected attachment and fails closed on mismatch. A ref also fails closed after another browser + connection or tab replaces the attachment, even if both tabs minted the same generation number. - **`fill_secret` is an agent↔service command.** The consumer's `fill_credential` connector sends `fill_secret{secretRef}`; the **service** resolves `secretRef` against the vault and drives the keystrokes into the session (over the trusted service↔extension hop, reusing the extension's @@ -208,15 +216,18 @@ Design notes: consumers express dry-run intent via the service API's `dryRun` flag, never by sending `resolve_ref` themselves. - `commandId` correlates the async round-trip: the service's `send(cmd)` returns a promise parked - in a `Map`; the matching `*_result` event resolves it. Only *in-flight* - commands are lost on DO hibernation; persisted session state is not (see the service section). + in a `Map`; the matching `*_result` event resolves it. The active request + and timeout prevent DO hibernation mid-command. A shutdown or restart settles through the + timeout/resync path, and the persisted awaiting marker reconciles a late result. - `tabs_result` answers `get_tabs` with a `commandId`-bearing event (added at M2). - **Published runtime exports** (what consumer connectors import): `parseCommand` / `parseEvent` (throwing) and `safeParseCommand` / `safeParseEvent`; `CommandSchema` / `EventSchema`, - `A11yNodeSchema`, `TabInfoSchema`, `SnapshotModeSchema`; `isWriteCommand` + + `A11yNodeSchema`, `TabInfoSchema`, `SnapshotModeSchema`, `SnapshotTargetSchema`, + `DialogTypeSchema`, `DialogDispositionSchema`, `DialogRecordSchema`; `isWriteCommand` + `WRITE_COMMAND_TYPES` (the single write-classification source of truth downstream layers derive from — the connector pins its gated union to it at compile time); and the types - `Command` / `Event` / `WriteCommandType` / `A11yNode` / `TabInfo` / `SnapshotMode`. + `Command` / `Event` / `WriteCommandType` / `A11yNode` / `TabInfo` / `SnapshotMode` / + `SnapshotTarget` / `DialogType` / `DialogDisposition` / `DialogRecord`. - **Write retries are idempotent end to end** (M5): the connector derives a write's `commandId` from the breakwater idempotency key (`ik_`); the service records completed write Events per session (`completedWrites`, cap 100) and replays a repeated commandId instead of @@ -234,16 +245,18 @@ The hard problem. Approach (matches Playwright / chrome-devtools-mcp): 1. **Build snapshot**: `Accessibility.getFullAXTree` (+ `DOM` for `backendNodeId` linkage). Prune to actionable/meaningful nodes (buttons, links, inputs, headings, text). Assign each a stable `ref` - (generation-namespaced `s{gen}e{seq}`) mapped to its CDP `backendNodeId`. Keep the - `ref → backendNodeId` map in the SW for the current snapshot generation. + whose opaque namespace binds the extension session, CDP attachment, and snapshot generation, + then map it to the CDP `backendNodeId`. Keep the `ref → backendNodeId` map in the SW for the + current attachment and snapshot generation. 2. **The consumer's agent sees** the pruned tree as compact indented text (role + name + ref), not raw HTML — far fewer tokens, far more reliable targeting. 3. **Resolve on action**: `click{ref}` → `backendNodeId` → `DOM.getBoxModel` → `Input.dispatchMouseEvent` (press+release at center), OR `Runtime.callFunctionOn` `.click()` when geometry is unreliable. `type{ref}` → focus node then `Input.insertText` / `Input.dispatchKeyEvent`. -4. **Staleness**: a `ref` is valid only for the snapshot generation that produced it. Every mutating - action is followed by an implicit re-snapshot before the next action needs one. A stale `ref` - returns `action_result{ok:false, error:"stale ref"}`; the consumer re-snapshots and retries. +4. **Staleness**: a `ref` is valid only for the extension session, CDP attachment, and snapshot + generation that produced it. Replacing the browser connection or attached tab invalidates every + outstanding ref, including refs whose generation and element sequence happen to collide. A stale + ref returns `action_result{ok:false, error:"stale ref"}`; the consumer re-snapshots and retries. 5. **Screenshots** (`mode:"screenshot"` → `Page.captureScreenshot`) are a *fallback* the agent can request when the a11y tree is insufficient (canvas, visual layout). DOM/a11y-first keeps cost down. @@ -442,18 +455,19 @@ auth, local `fill_secret` shim) and carries stale pre-Topology-1 prose; prefer t `POST /v1/sessions/:sessionId/commands` (with `dryRun`); the pending-map + hibernation-resume marker; `fill_secret` vault resolution; caller auth + per-tenant/session scoping. Verify: a stub consumer (or `curl`) drives a real logged-in page over HTTP through the service and gets schema-valid `Event`s; - session survives a mid-command DO hibernation without deadlock. + a shutdown/restart or late result settles through timeout/resync without deadlock or stale markers. - **M4 — Consumer integration + published contract.** Publish `@understudy/protocol` (zod 4) and a reference breakwater connector (`@understudy/connector`, mirroring the smart-compliance example). A real consumer (metamind / smart-compliance) drives understudy end-to-end with a Mastra agent + flowsafe approvals. *Cross-repo; understudy's deliverable is the published contract + reference connector, not - the agent.* **Status (2026-07-17): PACKAGES PUBLISHED** — observe (snapshot/get_tabs/wait) / + the agent.* **Status (2026-07-25): PACKAGES PUBLISHED** — observe + (snapshot/get_tabs/get_dialogs/wait) / act (click/type/navigate/key/scroll/switch_tab, grant-gated) / fill_credential (vaulted), egress-pinned `runtime.fetch`, caller bearer auth, and tests against the real breakwater wrapper (fail-closed grant, - idempotent replay, per-hop egress denial, dry-run). `@understudy/protocol@0.5.0` and - `@understudy/connector@0.3.0` are published on npm with MIT-licensed, `files`-scoped tarballs and - npm provenance. The changesets + GitHub Actions release flow is wired in - `.github/workflows/release.yml`. Metamind contains the consumer-side Mastra workflow, flowsafe + idempotent replay, per-hop egress denial, dry-run). `@understudy/protocol@0.6.0` and + `@understudy/connector@0.4.0` add target-bound snapshots and refs and are published on npm with + MIT-licensed, `files`-scoped tarballs and npm provenance. The changesets + GitHub Actions release + flow is wired in `.github/workflows/release.yml`. Metamind contains the consumer-side Mastra workflow, flowsafe approval, breakwater connector wiring, automated coverage, and attended runbook. The remaining proof is an attended run with its Chromium extension connected. - **M5 — Substrate hardening. LARGELY LANDED (2026-07-17, the deferred-items sweep):** diff --git a/packages/connector/README.md b/packages/connector/README.md index ed79ccb..9166c6d 100644 --- a/packages/connector/README.md +++ b/packages/connector/README.md @@ -17,7 +17,7 @@ limits, audit). | connector | id | class | protocol commands | |---|---|---|---| -| `observe` | `browser.observe` | read — no approval | `snapshot` (a11y/dom/screenshot), `get_tabs`, `wait` | +| `observe` | `browser.observe` | read — no approval | `snapshot` (a11y/dom/screenshot), `get_tabs`, `get_dialogs`, `wait` | | `act` | `browser.act` | write — approval-gated | `click`, `type`, `navigate`, `key`, `scroll`, `switch_tab` | | `fillCredential` | `browser.fill_credential` | vaulted write — approval-gated | `fill_secret` | @@ -31,6 +31,18 @@ The protocol's `resolve_ref` command is deliberately unreachable from here — it is an internal service↔extension probe. Dry-run intent is expressed via the service API's `dryRun` flag, which the `dryRunExecute` paths set. +Snapshot reads return `target: { tabId, url }` beside the tree or screenshot. +That target is captured from the exact attached CDP session with the artifact; +consumers should validate it instead of inferring session identity from the +global `get_tabs` list, where each browser window can have an active tab. +When the driver cannot produce a trustworthy snapshot, including an expected +tab mismatch or a page change during capture, `observe` returns structured +`{ ok: false, error }` output with the driver's reason. + +Accessibility refs are opaque and bound to the extension session, CDP +attachment, and snapshot generation that minted them. Do not parse their +encoding or reuse a ref after another attachment replaces the target. + ## Install ```sh @@ -50,8 +62,11 @@ import { BROWSER_WRITE_CONNECTOR_IDS, callBrowserDryRun, callBrowserWrite, + callConnector, createBrowserConnectors, durableStores, + type ObserveInput, + type ObserveOutput, } from "@understudy/connector"; // D1-backed stores are load-bearing on Cloudflare: breakwater's in-memory @@ -68,6 +83,29 @@ Register the three connectors as tools on your Mastra agent (they *are* Mastra tools — `createConnector()` wraps `createTool()`), or call them from workflow steps via the helpers: +```ts +// 1. Verify that the snapshot came from the intended browser target. +const snapshot = await callConnector( + observe, + { + sessionId, + read: { type: "snapshot", mode: "a11y", tabId: expectedTabId }, + }, + requestContext, +); +const target = snapshot.target; +if ( + snapshot.ok === false || + target === undefined || + target.tabId !== expectedTabId || + target.url !== expectedUrl +) { + throw new Error(snapshot.error ?? "browser snapshot target mismatch"); +} +``` + +After target validation, use the normal simulation and approval flow: + ```ts // 1. Simulate before asking for approval — no side effect, no grant needed. const preview = await callBrowserDryRun(act, { @@ -180,3 +218,12 @@ yours to enforce at the agent. - It does not mint sessions. Create one with `POST {UNDERSTUDY_URL}/v1/sessions` (bearer `UNDERSTUDY_TOKEN`) during case setup and pass the returned `sessionId` into every connector input. + +## Versioning + +- **0.4.0** — returns the exact snapshot target through `observe`, preserves + snapshot driver failures as structured output, and requires + `@understudy/protocol@^0.6.0`. Upgrade the service and extension in the same + rollout because protocol 0.6 makes snapshot target fields mandatory. +- **0.3.0** — adds `observe`'s `get_dialogs` read for recent dialogs handled by + the extension and requires `@understudy/protocol@^0.5.0`. diff --git a/packages/connector/src/index.test.ts b/packages/connector/src/index.test.ts index 09caf50..cd2fa82 100644 --- a/packages/connector/src/index.test.ts +++ b/packages/connector/src/index.test.ts @@ -57,7 +57,7 @@ function sentRequest(spy: ReturnType, call = 0): { url: string; bo return { url, body: JSON.parse(init.body), headers: init.headers }; } -const CLICK = { sessionId: "s-1", action: { type: "click", ref: "s1e2" } } as const; +const CLICK = { sessionId: "s-1", action: { type: "click", ref: "opaque-ref" } } as const; afterEach(() => { vi.unstubAllGlobals(); @@ -90,7 +90,7 @@ describe("grant gate (fail closed)", () => { const fetchSpy = vi.fn(); vi.stubGlobal("fetch", fetchSpy); const { act } = createBrowserConnectors(ENV, stores()); - const missingSession = { action: { type: "click", ref: "s1e2" } } as unknown as typeof CLICK; + const missingSession = { action: { type: "click", ref: "opaque-ref" } } as unknown as typeof CLICK; await expect( callBrowserWrite(act, missingSession, grantFor(BROWSER_ACT_CONNECTOR), "k1"), @@ -130,7 +130,7 @@ describe("act", () => { // random: a retry after a lost/unparseable response re-runs execute() // under the same key, and only a stable id lets the service replay the // recorded Event instead of executing the write twice. - command: { type: "click", ref: "s1e2", commandId: "ik_case1:step1:click" }, + command: { type: "click", ref: "opaque-ref", commandId: "ik_case1:step1:click" }, }); }); @@ -162,20 +162,52 @@ describe("observe (read - no grant, no idempotency)", () => { } it("snapshot returns the a11y tree", async () => { - const tree = [{ ref: "s1e1", role: "button", name: "Go" }]; + const tree = [{ ref: "opaque-tree-ref", role: "button", name: "Go" }]; const out = await observeRead( { type: "snapshot", mode: "a11y" }, - { type: "snapshot_result", commandId: "c1", tree }, + { + type: "snapshot_result", + commandId: "c1", + tree, + tabId: 7, + url: "https://x/", + }, ); - expect(out).toEqual({ tree }); + expect(out).toEqual({ tree, target: { tabId: 7, url: "https://x/" } }); }); it("snapshot in screenshot mode returns the image artifact", async () => { const out = await observeRead( { type: "snapshot", mode: "screenshot" }, - { type: "screenshot_result", commandId: "c1", mime: "image/png", b64: "aGk=" }, + { + type: "screenshot_result", + commandId: "c1", + mime: "image/png", + b64: "aGk=", + tabId: 7, + url: "https://x/", + }, + ); + expect(out).toEqual({ + screenshot: { mime: "image/png", b64: "aGk=" }, + target: { tabId: 7, url: "https://x/" }, + }); + }); + + it("snapshot preserves a target-mismatch failure as structured output", async () => { + const out = await observeRead( + { type: "snapshot", mode: "a11y", tabId: 8 }, + { + type: "action_result", + commandId: "c1", + ok: false, + error: "attached CDP session is tab 7, not requested tab 8", + }, ); - expect(out).toEqual({ screenshot: { mime: "image/png", b64: "aGk=" } }); + expect(out).toEqual({ + ok: false, + error: "attached CDP session is tab 7, not requested tab 8", + }); }); it("get_tabs returns the tab list", async () => { @@ -245,7 +277,11 @@ describe("observe (read - no grant, no idempotency)", () => { }); describe("fill_credential (vaulted write)", () => { - const INPUT = { sessionId: "s-1", ref: "s1e9", secretRef: "vault://acme/portal/password" }; + const INPUT = { + sessionId: "s-1", + ref: "opaque-ref", + secretRef: "vault://acme/portal/password", + }; it("passes the opaque secretRef through - the wire carries no plaintext field", async () => { const fetchSpy = vi.fn().mockResolvedValue( @@ -264,7 +300,11 @@ describe("fill_credential (vaulted write)", () => { expect(out).toEqual({ ok: true, filled: true, error: undefined }); const { body } = sentRequest(fetchSpy); expect(body).toMatchObject({ - command: { type: "fill_secret", ref: "s1e9", secretRef: "vault://acme/portal/password" }, + command: { + type: "fill_secret", + ref: "opaque-ref", + secretRef: "vault://acme/portal/password", + }, }); expect(Object.keys((body as { command: Record }).command).sort()).toEqual([ "commandId", @@ -468,7 +508,7 @@ describe("write-classification sync with @understudy/protocol", () => { // #when it executes await callBrowserWrite( fillCredential, - { sessionId: "s-1", ref: "s1e9", secretRef: "vault://x" }, + { sessionId: "s-1", ref: "opaque-ref", secretRef: "vault://x" }, grantFor(BROWSER_FILL_CREDENTIAL_CONNECTOR), "case1:login:fill", ); diff --git a/packages/connector/src/index.ts b/packages/connector/src/index.ts index 484bef1..4422f86 100644 --- a/packages/connector/src/index.ts +++ b/packages/connector/src/index.ts @@ -41,6 +41,7 @@ import { parseCommand, parseEvent, SnapshotModeSchema, + SnapshotTargetSchema, TabInfoSchema, } from "@understudy/protocol"; import { z } from "zod"; @@ -137,10 +138,12 @@ export const observeOutput = z.object({ tree: z.array(A11yNodeSchema).optional(), /** Vision fallback (canvas / visual layout) - an evidence artifact. */ screenshot: z.object({ mime: z.string(), b64: z.string() }).optional(), + /** Exact attached CDP target captured with tree/screenshot snapshots. */ + target: SnapshotTargetSchema.optional(), tabs: z.array(TabInfoSchema).optional(), /** Recent page dialogs (get_dialogs), oldest first. */ dialogs: z.array(DialogRecordSchema).optional(), - /** wait outcome. */ + /** Wait outcome, or `false` when the driver rejects a snapshot. */ ok: z.boolean().optional(), error: z.string().optional(), }); @@ -333,9 +336,17 @@ export function createBrowserConnectors( const ev = await callUnderstudy(runtime, env, input.sessionId, toCommand(read)); switch (read.type) { case "snapshot": - if (ev.type === "snapshot_result") return { tree: ev.tree }; + if (ev.type === "snapshot_result") { + return { tree: ev.tree, target: { tabId: ev.tabId, url: ev.url } }; + } if (ev.type === "screenshot_result") { - return { screenshot: { mime: ev.mime, b64: ev.b64 } }; + return { + screenshot: { mime: ev.mime, b64: ev.b64 }, + target: { tabId: ev.tabId, url: ev.url }, + }; + } + if (ev.type === "action_result" && !ev.ok) { + return { ok: false, error: ev.error }; } break; case "get_tabs": diff --git a/packages/protocol/README.md b/packages/protocol/README.md index 8c1fd93..e0f2932 100644 --- a/packages/protocol/README.md +++ b/packages/protocol/README.md @@ -10,10 +10,10 @@ that speak it: - **consumer connectors** (e.g. [`@understudy/connector`](https://github.com/ProofOfTechOrg/understudy/tree/master/packages/connector)) that drive the service over `POST /v1/sessions/:sessionId/commands`. -Every message is a zod-4 discriminated union tagged by `type`, carries a -`commandId` for request/response correlation, and is validated at every -boundary. Consumers nest these schemas inside their own zod-4 objects, so the -package requires zod 4. +Every message is a zod-4 discriminated union tagged by `type` and is validated +at every boundary. Commands and their correlated results carry a `commandId`; +connection, page, and heartbeat events do not. Consumers nest these schemas +inside their own zod-4 objects, so the package requires zod 4. ## Install @@ -27,13 +27,16 @@ pnpm add @understudy/protocol zod import { // schemas CommandSchema, EventSchema, A11yNodeSchema, TabInfoSchema, SnapshotModeSchema, + SnapshotTargetSchema, DialogTypeSchema, DialogDispositionSchema, + DialogRecordSchema, // parsers parseCommand, parseEvent, safeParseCommand, safeParseEvent, // classification isWriteCommand, WRITE_COMMAND_TYPES, // types type Command, type CommandType, type Event, type WriteCommandType, - type A11yNode, type TabInfo, type SnapshotMode, + type A11yNode, type TabInfo, type SnapshotMode, type SnapshotTarget, + type DialogType, type DialogDisposition, type DialogRecord, } from "@understudy/protocol"; ``` @@ -62,6 +65,14 @@ there, where it is real). | `switch_tab` | `tabId` | **write** | | `fill_secret` | `ref`, `secretRef`, `submit?` | **write** (vaulted) | +When `snapshot.tabId` or `navigate.tabId` is present, the extension treats it +as the expected attached CDP tab and fails closed if it differs. It never +silently executes the command against another attached tab. + +Accessibility refs are opaque capabilities bound to the extension session, +the exact CDP attachment, and the snapshot generation that minted them. A ref +from one attached tab cannot resolve after another attachment replaces it. + `isWriteCommand` / `WRITE_COMMAND_TYPES` classify the write class in the **operational** sense this system enforces: a command with a user-visible side effect, which must be gated on approval (D8), simulated (never performed) on a @@ -87,11 +98,12 @@ commands deserve a warning: | type | fields | |---|---| | `hello` | `browser`, `extVersion`, `tabs` | -| `snapshot_result` | `commandId`, `tree: A11yNode[]` | -| `screenshot_result` | `commandId`, `mime`, `b64` | +| `snapshot_result` | `commandId`, `tree: A11yNode[]`, `tabId`, `url` | +| `screenshot_result` | `commandId`, `mime`, `b64`, `tabId`, `url` | | `tabs_result` | `commandId`, `tabs: TabInfo[]` | | `action_result` | `commandId`, `ok`, `error?`, `url?`, `simulated?` | | `page_event` | `kind: "navigated" \| "load"`, `tabId`, `url` | +| `dialog` | `tabId`, `dialogType`, `message`, `url`, `defaultPrompt?`, `disposition` | | `pong` | — | `action_result.simulated` is set only on dry-run responses. A simulated @@ -99,16 +111,29 @@ commands deserve a warning: current snapshot generation), not *executability*; ref-less commands simulate `ok: true` without touching the browser at all. +Every snapshot result carries the exact CDP-attached `tabId` and main-frame +`url` captured with the artifact. Consumers must validate this target before +using page content; `TabInfo.active` is browser-window state and is not a +session-attachment signal. + ## Element targeting -`A11yNode.ref` is the only element address a consumer's agent ever uses — -opaque, generation-namespaced (`s{gen}e{seq}`), resolved by the extension -against its CDP `backendNodeId` map. Refs are valid only for the snapshot -generation that produced them; a stale ref returns -`action_result{ ok: false }` and the consumer re-snapshots. +`A11yNode.ref` is the only element address a consumer's agent ever uses. Its +encoding is deliberately unspecified. The extension resolves it against the +current CDP attachment's `backendNodeId` map and rejects refs minted by another +extension session, attachment, or snapshot generation. A stale ref returns +`action_result{ ok: false }`; the consumer re-snapshots before retrying. ## Versioning +- **0.6.0** — requires `tabId` and `url` on every `snapshot_result` and + `screenshot_result`, exports `SnapshotTargetSchema` / `SnapshotTarget`, and + binds accessibility refs to their extension session, CDP attachment, and + snapshot generation. This is a breaking wire change: upgrade the protocol, + service, extension, and `@understudy/connector@0.4.0` together. +- **0.5.0** — adds `dialog` events and the `DialogTypeSchema`, + `DialogDispositionSchema`, and `DialogRecordSchema` exports. The connector + exposes recent handled dialogs through `observe`'s `get_dialogs` read. - **0.4.0** — exports `WRITE_COMMAND_TYPES` / `WriteCommandType` (the single write-classification source downstream layers derive from) and reclassifies `scroll` / `switch_tab` as writes, so `isWriteCommand` now returns `true` for diff --git a/packages/protocol/src/index.test.ts b/packages/protocol/src/index.test.ts index 8708b02..de85856 100644 --- a/packages/protocol/src/index.test.ts +++ b/packages/protocol/src/index.test.ts @@ -41,7 +41,7 @@ describe("CommandSchema", () => { const cmd = { type: "fill_secret", commandId: "c1", - ref: "s1e2", + ref: "opaque-ref", secretRef: "vault://x", submit: true, }; @@ -50,12 +50,12 @@ describe("CommandSchema", () => { it("rejects fill_secret missing secretRef", () => { expect( - safeParseCommand({ type: "fill_secret", commandId: "c1", ref: "s1e2" }).success, + safeParseCommand({ type: "fill_secret", commandId: "c1", ref: "opaque-ref" }).success, ).toBe(false); }); it("parses a valid resolve_ref command", () => { - const cmd = { type: "resolve_ref", commandId: "c1", ref: "s1e2" }; + const cmd = { type: "resolve_ref", commandId: "c1", ref: "opaque-ref" }; expect(parseCommand(cmd)).toEqual(cmd); }); @@ -76,14 +76,14 @@ describe("isWriteCommand", () => { const fillSecret: Command = { type: "fill_secret", commandId: "c1", - ref: "s1e2", + ref: "opaque-ref", secretRef: "vault://x", }; expect(isWriteCommand(fillSecret)).toBe(true); }); it("classifies resolve_ref as a read - the dry-run probe must run freely", () => { - const probe: Command = { type: "resolve_ref", commandId: "c1", ref: "s1e2" }; + const probe: Command = { type: "resolve_ref", commandId: "c1", ref: "opaque-ref" }; expect(isWriteCommand(probe)).toBe(false); }); @@ -125,6 +125,35 @@ describe("isWriteCommand", () => { }); describe("EventSchema", () => { + it("round-trips an accessibility snapshot with its attached target identity", () => { + const ev = { + type: "snapshot_result", + commandId: "c1", + tree: [], + tabId: 7, + url: "https://example.com/", + }; + expect(EventSchema.parse(ev)).toEqual(ev); + }); + + it("rejects a snapshot result without its attached target identity", () => { + expect( + safeParseEvent({ type: "snapshot_result", commandId: "c1", tree: [] }).success, + ).toBe(false); + }); + + it("rejects a snapshot result with an invalid attached tab id", () => { + expect( + safeParseEvent({ + type: "snapshot_result", + commandId: "c1", + tree: [], + tabId: -1, + url: "https://example.com/", + }).success, + ).toBe(false); + }); + it("round-trips an action_result", () => { const ev = { type: "action_result", commandId: "c1", ok: true, url: "https://example.com/" }; expect(EventSchema.parse(ev)).toEqual(ev); diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 6408b9f..f8600c6 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -37,6 +37,15 @@ export type TabInfo = z.infer; export const SnapshotModeSchema = z.enum(["a11y", "dom", "screenshot"]); export type SnapshotMode = z.infer; +// Identity of the exact CDP target a snapshot came from. This travels on the +// snapshot result itself so consumers never have to infer the driven target +// from chrome.tabs' global active flags (one active tab exists per window). +export const SnapshotTargetSchema = z.object({ + tabId: z.number().int().nonnegative(), + url: z.string().min(1), +}); +export type SnapshotTarget = z.infer; + // A JavaScript dialog the page raised (alert/confirm/prompt) or a // navigation-guard prompt (beforeunload). The extension handles each locally // and synchronously - an open dialog blocks the single CDP channel, so there @@ -168,12 +177,14 @@ export const EventSchema = z.discriminatedUnion("type", [ type: z.literal("snapshot_result"), commandId: z.string(), tree: z.array(A11yNodeSchema), + ...SnapshotTargetSchema.shape, }), z.object({ type: z.literal("screenshot_result"), commandId: z.string(), mime: z.string(), b64: z.string(), + ...SnapshotTargetSchema.shape, }), z.object({ type: z.literal("tabs_result"),