From b1b778ccb33e531e3c1ab49fd94c149015470d9f Mon Sep 17 00:00:00 2001 From: gcharang <21151592+gcharang@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:36:51 +0400 Subject: [PATCH] fix(backend): fail fast on disconnected sessions + honest error taxonomy Closes the three QA follow-ups from the attended M3 e2e, hardened further by this round's review lanes: - No-extension fail-fast: CfSessionCoordinator.send() rejects immediately unless the host reports a live, onConnect-authorized extension socket (CoordinatorHost.hasAuthorizedConnection). The gate reads that delivery predicate - the exact precondition sendToExtension relies on - NOT the persisted SessionState.status scalar: a late onClose from a replaced socket stamps "detached" over a healthy reconnect, which would wedge a live session into permanent 503 (architect-lane catch). onClose likewise only detaches when no other authorized socket remains. - Error taxonomy at the commands route, mapped by message-prefix constants (src/coordinator.ts - the only failure signal that survives the DO RPC boundary): 503 {extension not connected} (non-2xx on purpose: a 200 ok:false Event would be cached by consumers' idempotency stores and replayed after reconnect), 504 {command timed out}, 400 {invalid body} for unparseable JSON, uniform JSON 500 via app.onError for the rest. Ref-less dryRun writes keep their documented zero-wire simulated-ok semantic (never a liveness signal) - regression-tested. - fill_secret gates on the same predicate BEFORE resolveSecret: no vault read and no plaintext materialization for a command that cannot dispatch (DL-004). - Dry-run probe failures surface the extension's own reason ("dry-run: stale or unknown ref: s1e9"; "unexpected probe response ''") instead of one collapsed string. - Env.CALLER_TOKENS/EXTENSION_TOKENS are required types - they were always in wrangler.jsonc secrets.required; the optional typing lied. auth.ts keeps the empty-string runtime guards. Tests 53 -> 64: fail-fast unit + adversarial liveness (full pending->connected->detached->reconnected lifecycle; a replaced socket's close is not a detach), vault-untouched fill_secret gate, probe-reason fidelity, 400/500 route tests, and the old 35s DO-timeout test converted into the route-level 504 proof (same wall-clock, full route->RPC->DO path, marker-clear re-asserted). Workspace: 142 tests green. Known noise: fail-fast rejections log "Uncaught (in promise)" via workerd's DO RPC instrumentation in vitest/wrangler tail - these are handled 503s, not real uncaught errors. Co-Authored-By: Claude Fable 5 --- apps/backend/README.md | 23 ++- apps/backend/src/coordinator-cf.ts | 22 +- apps/backend/src/coordinator.ts | 10 + apps/backend/src/index.ts | 44 +++- apps/backend/src/session.ts | 75 +++++-- apps/backend/src/types.ts | 14 +- apps/backend/test/coordinator.test.ts | 23 ++- apps/backend/test/service.test.ts | 284 +++++++++++++++++++++++++- apps/backend/test/session.test.ts | 49 ++--- 9 files changed, 471 insertions(+), 73 deletions(-) diff --git a/apps/backend/README.md b/apps/backend/README.md index 0a6c368..58a4148 100644 --- a/apps/backend/README.md +++ b/apps/backend/README.md @@ -23,10 +23,25 @@ from the `SessionAgent` class name); the Hono app claims everything else, includ Command flow: `POST /v1/sessions/:sessionId/commands` → `authenticate` (bearer caller token → `{actor, tenantId}`, 401 on failure) → `scopeSession` (verifies the sessionId's embedded tenant matches the caller's, 404 on mismatch) → `safeParseCommand` -(400 on schema failure) → `stub.dispatch()` or, for `fill_secret`, `stub.fillSecret()` +(400 on an unparseable body or schema failure) → `stub.dispatch()` or, for `fill_secret`, `stub.fillSecret()` → the correlated Event is returned as JSON. This order is load-bearing: an unauthenticated or cross-tenant request never reaches parsing or dispatch. +Dispatch failures map to status codes by error-message prefix (the only signal +that survives the DO RPC boundary; the constants live in `src/coordinator.ts`): +**503** `{error: "extension not connected"}` when the session has no live, +onConnect-authorized extension socket — the gate consults that delivery +predicate directly (not the persisted `status` scalar, which a late close from +a replaced socket can leave stale), 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 reconnect; +**504** `{error: "command timed out"}` when a connected extension never +answered; anything else is a uniform JSON **500** via `app.onError`. A real +`fill_secret` checks the same predicate *before* touching the vault, so no +plaintext is ever resolved for a command that cannot dispatch. Exception: +a ref-less dryRun write (e.g. navigate) still short-circuits to simulated +`ok:true` without touching the wire — it was never a liveness signal. + Inside the DO, `SessionAgent` holds a `CfSessionCoordinator` (the Cloudflare implementation of the portable `SessionCoordinator` interface). `send(cmd)` writes the command to the extension WebSocket, parks a `{resolve, reject, timer}` in an @@ -143,8 +158,10 @@ the connection set during that window. Four things close this gap: ## Invariants -- Every `Command` produces exactly one `Event` bearing its `commandId`; the - command route returns exactly that Event. +- Every *successfully dispatched* `Command` produces exactly one `Event` + bearing its `commandId`, and the command route returns exactly that Event; + a command that cannot dispatch or never resolves maps to a non-2xx JSON + error instead (503/504/500 — see the failure mapping above). - `fill_secret` plaintext never enters `setState`, logs, the Event response, or an error string; the coordinator logs only `{commandId, type}`. - One Durable Object per `sessionId`; a sessionId whose embedded tenant disagrees diff --git a/apps/backend/src/coordinator-cf.ts b/apps/backend/src/coordinator-cf.ts index f4b67ea..e20cbda 100644 --- a/apps/backend/src/coordinator-cf.ts +++ b/apps/backend/src/coordinator-cf.ts @@ -9,6 +9,7 @@ */ import type { Command, Event } from "@understudy/protocol"; +import { COMMAND_TIMED_OUT, SESSION_NOT_CONNECTED } from "./coordinator"; import type { PendingCommand, PendingMap, SessionCoordinator } from "./coordinator"; import type { SessionStatus } from "./types"; @@ -22,6 +23,14 @@ const DEFAULT_TIMEOUT_MS = 30_000; export interface CoordinatorHost { /** Writes a JSON frame to the live 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 + * lossy last-writer-wins echo of it (a late onClose from a replaced + * socket can stamp "detached" over a healthy reconnected session). + */ + hasAuthorizedConnection(): boolean; /** Reads the persisted awaiting-commandId marker (SessionState.awaitingCommandIds). */ getAwaitingCommandIds(): string[]; /** Persists the awaiting-commandId marker via the DO's setState. */ @@ -64,10 +73,21 @@ export class CfSessionCoordinator implements SessionCoordinator { * always settles even if the WebSocket is gone. */ send(cmd: Command): Promise { + // Fail fast instead of parking a promise that can only time out: with no + // deliverable socket, sendToExtension writes to nobody, so the 30s + // timeout (surfacing as an opaque 500) would be the guaranteed outcome - + // the M3 e2e's QA finding. The Worker maps this prefix to a 503. A + // socket that dies between this check and the write still ends in the + // timeout: fail-fast is best-effort, the timer stays the backstop. + if (!this.host.hasAuthorizedConnection()) { + return Promise.reject( + new Error(`${SESSION_NOT_CONNECTED}: no authorized extension connection`), + ); + } return new Promise((resolve, reject) => { const timer = setTimeout(() => { reject( - new Error(`command ${cmd.commandId} (${cmd.type}) timed out after ${this.timeoutMs}ms`), + new Error(`${COMMAND_TIMED_OUT}: ${cmd.commandId} (${cmd.type}) after ${this.timeoutMs}ms`), ); this.pending.delete(cmd.commandId); this.dropAwaiting(cmd.commandId); diff --git a/apps/backend/src/coordinator.ts b/apps/backend/src/coordinator.ts index 155d39b..f0ec1d0 100644 --- a/apps/backend/src/coordinator.ts +++ b/apps/backend/src/coordinator.ts @@ -12,6 +12,16 @@ import type { Command, Event } from "@understudy/protocol"; import type { SessionStatus } from "./types"; +/** + * send()'s delivery-failure vocabulary. A message prefix is the only failure + * signal that survives an RPC boundary (custom Error subclasses do not), so + * every throw site and every host mapping failures onto its transport (the + * Cloudflare Worker maps these to 503/504 in index.ts) shares these + * constants instead of free-typing the strings. + */ +export const SESSION_NOT_CONNECTED = "session not connected"; +export const COMMAND_TIMED_OUT = "command timed out"; + /** One outstanding `send(cmd)` call, awaiting its correlated Event. */ export interface PendingCommand { resolve: (ev: Event) => void; diff --git a/apps/backend/src/index.ts b/apps/backend/src/index.ts index 5274aae..4ce6454 100644 --- a/apps/backend/src/index.ts +++ b/apps/backend/src/index.ts @@ -14,6 +14,7 @@ import { Hono } from "hono"; import { getAgentByName, routeAgentRequest } from "agents"; import { safeParseCommand } from "@understudy/protocol"; import { authenticate, mintSessionId, scopeSession } from "./auth"; +import { COMMAND_TIMED_OUT, SESSION_NOT_CONNECTED } from "./coordinator"; import type { Env } from "./types"; import type { SessionAgent } from "./session"; @@ -60,7 +61,14 @@ app.post("/v1/sessions/:sessionId/commands", async (c) => { const scope = await scopeSession(sessionId, actor.tenantId, c.env); if (scope === "not-found") return c.json({ error: "not found" }, 404); - const body = await c.req.json(); + // Unparseable JSON is the client's fault: 400, not a rethrow into the + // uniform 500 (which would dress a client error as a server error). + let body: { command?: unknown; dryRun?: unknown }; + try { + body = await c.req.json(); + } catch { + return c.json({ error: "invalid body" }, 400); + } const parsed = safeParseCommand(body?.command); if (!parsed.success) return c.json({ error: "invalid command" }, 400); @@ -70,12 +78,36 @@ app.post("/v1/sessions/:sessionId/commands", async (c) => { // carrying a secret through this route (DL-004). const dryRun = body?.dryRun === true; const stub = await getSessionStub(c.env, sessionId); - const event = - parsed.data.type === "fill_secret" - ? await stub.fillSecret(parsed.data, dryRun) - : await stub.dispatch(parsed.data, dryRun); + try { + const event = + parsed.data.type === "fill_secret" + ? await stub.fillSecret(parsed.data, dryRun) + : await stub.dispatch(parsed.data, dryRun); + return c.json(event); + } catch (err) { + // The DO's throw crosses the RPC boundary as a plain Error; its message + // prefix is the contract (coordinator.ts). "Not connected" is 503 - a + // retryable infrastructure state - deliberately NOT a 200 ok:false + // Event, which a consumer's idempotency store would cache and replay + // even after the extension reconnects. + const message = err instanceof Error ? err.message : String(err); + if (message.startsWith(SESSION_NOT_CONNECTED)) { + return c.json({ error: "extension not connected" }, 503); + } + if (message.startsWith(COMMAND_TIMED_OUT)) { + return c.json({ error: "command timed out" }, 504); + } + throw err; + } +}); - return c.json(event); +// Anything a route throws (or rethrows above) becomes a uniform JSON 500 +// instead of workerd's opaque non-JSON error page. Command payloads never +// enter error messages by construction (DL-004), so logging the message +// leaks nothing; the response body stays generic regardless. +app.onError((err, c) => { + console.error("unhandled route error", err.message); + return c.json({ error: "internal error" }, 500); }); export default { diff --git a/apps/backend/src/session.ts b/apps/backend/src/session.ts index 4287c27..91715bb 100644 --- a/apps/backend/src/session.ts +++ b/apps/backend/src/session.ts @@ -3,6 +3,7 @@ import type { AgentContext, Connection, ConnectionContext, WSMessage } from "age import { isWriteCommand, safeParseEvent } from "@understudy/protocol"; import type { Command, Event } from "@understudy/protocol"; import { scopeSession, verifyExtensionToken } from "./auth"; +import { SESSION_NOT_CONNECTED } from "./coordinator"; import { CfSessionCoordinator } from "./coordinator-cf"; import { resolveSecret } from "./secrets"; import type { Env, SessionState, SessionStatus } from "./types"; @@ -35,6 +36,7 @@ export class SessionAgent extends Agent { if (this.isAuthorizedConnection(connection)) connection.send(payload); } }, + hasAuthorizedConnection: () => this.hasAuthorizedConnection(), getAwaitingCommandIds: () => this.state.awaitingCommandIds, persistAwaitingCommandIds: (ids) => this.setState({ ...this.state, awaitingCommandIds: ids }), persistStatus: (status) => this.setState({ ...this.state, status }), @@ -125,7 +127,14 @@ export class SessionAgent extends Agent { } async onClose(connection: Connection, code: number, reason: string, wasClean: boolean): Promise { - this.setState({ ...this.state, status: "detached" }); + // 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" }); } async dispatch(command: Command, dryRun?: boolean): Promise { @@ -136,26 +145,20 @@ export class SessionAgent extends Agent { return this.coordinator.send(command); } - const ok = await this.checkRefResolves(this.commandRef(command)); - return { - type: "action_result", - commandId: command.commandId, - ok, - ...(ok ? {} : { error: "dry-run: ref did not resolve" }), - simulated: true, - }; + const probe = await this.checkRefResolves(this.commandRef(command)); + return this.simulatedResult(command.commandId, probe); } async fillSecret(cmd: FillSecretCommand, dryRun?: boolean): Promise { if (dryRun) { - const ok = await this.checkRefResolves(cmd.ref); - return { - type: "action_result", - commandId: cmd.commandId, - ok, - ...(ok ? {} : { error: "dry-run: ref did not resolve" }), - simulated: true, - }; + return this.simulatedResult(cmd.commandId, await this.checkRefResolves(cmd.ref)); + } + + // Gate BEFORE the vault: resolving a secret for a command that cannot + // dispatch would materialize plaintext (and emit a vault access) for + // nothing - fail-fast matters most exactly here (DL-004). + if (!this.hasAuthorizedConnection()) { + throw new Error(`${SESSION_NOT_CONNECTED}: no authorized extension connection`); } let secret: string; @@ -198,15 +201,37 @@ export class SessionAgent extends Agent { // (generation bump), so it can never contain the consumer's ref AND it // invalidates the consumer's outstanding refs, breaking the approved // command that follows the dry-run. - private async checkRefResolves(ref: string | undefined): Promise { - if (ref === undefined) return true; + private async checkRefResolves( + ref: string | undefined, + ): Promise<{ ok: true } | { ok: false; reason: string }> { + if (ref === undefined) return { ok: true }; const ev = await this.coordinator.send({ type: "resolve_ref", commandId: crypto.randomUUID(), ref, }); - return ev.type === "action_result" && ev.ok; + if (ev.type !== "action_result") { + return { ok: false, reason: `unexpected probe response '${ev.type}'` }; + } + if (ev.ok) return { ok: true }; + // Surface the extension's own reason (e.g. "stale or unknown ref: s1e2") + // instead of collapsing every probe failure into one generic string. + return { ok: false, reason: ev.error ?? "ref did not resolve" }; + } + + /** The simulated action_result a dry-run returns in place of dispatching. */ + private simulatedResult( + commandId: string, + probe: { ok: true } | { ok: false; reason: string }, + ): Event { + return { + type: "action_result", + commandId, + ok: probe.ok, + ...(probe.ok ? {} : { error: `dry-run: ${probe.reason}` }), + simulated: true, + }; } private commandRef(command: Command): string | undefined { @@ -224,4 +249,14 @@ export class SessionAgent extends Agent { private isAuthorizedConnection(connection: Connection): boolean { 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 (which a late onClose from a replaced socket can leave stale). + private hasAuthorizedConnection(): boolean { + for (const connection of this.getConnections()) { + if (this.isAuthorizedConnection(connection)) return true; + } + return false; + } } diff --git a/apps/backend/src/types.ts b/apps/backend/src/types.ts index f88fa02..43ee451 100644 --- a/apps/backend/src/types.ts +++ b/apps/backend/src/types.ts @@ -38,10 +38,16 @@ export interface Env { VAULT: VaultBinding; /** Signs/verifies server-minted sessionIds so scopeSession can verify tenant ownership statelessly (M-006, DL-008). */ AUTH_HMAC_SECRET: string; - /** Static caller-token -> tenantId map for the dev auth verifier (M-006). */ - CALLER_TOKENS?: string; - /** Extension per-user token(s), verified independently of caller auth. */ - EXTENSION_TOKENS?: string; + /** + * Static caller-token -> tenantId map (JSON) for the dev auth verifier + * (M-006). Required, not optional: wrangler.jsonc lists it in + * `secrets.required`, which is also the .dev.vars allowlist, so a + * deployment without it cannot start. auth.ts still guards the empty + * string at runtime. + */ + CALLER_TOKENS: string; + /** Extension per-user token(s) (JSON), verified independently of caller auth. Required via `secrets.required`, like CALLER_TOKENS. */ + EXTENSION_TOKENS: string; } /** diff --git a/apps/backend/test/coordinator.test.ts b/apps/backend/test/coordinator.test.ts index f7e6833..a132ad9 100644 --- a/apps/backend/test/coordinator.test.ts +++ b/apps/backend/test/coordinator.test.ts @@ -2,13 +2,14 @@ import { describe, it, expect, vi } from "vitest"; import { CfSessionCoordinator, type CoordinatorHost } from "../src/coordinator-cf"; import type { Command, Event } from "@understudy/protocol"; -function createFakeHost(): CoordinatorHost & { sent: string[] } { +function createFakeHost(connected = true): CoordinatorHost & { sent: string[] } { let awaiting: string[] = []; const sent: string[] = []; return { sendToExtension: (payload: string) => { sent.push(payload); }, + hasAuthorizedConnection: () => connected, getAwaitingCommandIds: () => awaiting, persistAwaitingCommandIds: (ids: string[]) => { awaiting = ids; @@ -53,7 +54,7 @@ describe("CfSessionCoordinator", () => { // #then it rejects with a payload-free error and the marker is cleared expect(err).toBeInstanceOf(Error); - expect((err as Error).message).toBe("command c2 (click) timed out after 1000ms"); + expect((err as Error).message).toBe("command timed out: c2 (click) after 1000ms"); expect((err as Error).message).not.toContain("r1"); expect(host.getAwaitingCommandIds()).toEqual([]); } finally { @@ -61,6 +62,24 @@ describe("CfSessionCoordinator", () => { } }); + it("rejects immediately when no authorized socket exists - no marker, no frame, no timer burn", async () => { + // #given a host whose session has no live authorized extension socket + const host = createFakeHost(false); + const coordinator = new CfSessionCoordinator(host); + const cmd: Command = { type: "get_tabs", commandId: "c-fast" }; + + // #when send() is called + const err = await coordinator.send(cmd).catch((e: unknown) => e); + + // #then it rejects with the route-mappable prefix before parking anything + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe( + "session not connected: no authorized extension connection", + ); + expect(host.getAwaitingCommandIds()).toEqual([]); + expect(host.sent).toEqual([]); + }); + 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 c60883d..8acdaea 100644 --- a/apps/backend/test/service.test.ts +++ b/apps/backend/test/service.test.ts @@ -4,6 +4,7 @@ import { runInDurableObject } from "cloudflare:test"; import { safeParseCommand, safeParseEvent } from "@understudy/protocol"; import type { Command } from "@understudy/protocol"; import type { SessionAgent } from "../src/session"; +import type { SessionStatus } from "../src/types"; import { CALLER_TOKEN_A, CALLER_TOKEN_B, EXTENSION_TOKEN_A } from "./tokens"; import { BASE, getSessionStub, getWebSocket } from "./helpers"; @@ -440,13 +441,14 @@ describe("dryRun (DL-011: fail-safe, never dispatches a mutation or resolves a s true, ); - // #then it returns exactly a simulated ok:false result (the ref does not resolve) + // #then it returns exactly a simulated ok:false result carrying the + // extension's OWN failure reason, not a collapsed generic string const event = await res.json(); expect(event).toEqual({ type: "action_result", commandId: "c5", ok: false, - error: "dry-run: ref did not resolve", + error: "dry-run: stale or unknown ref: s1e1", simulated: true, }); @@ -524,4 +526,282 @@ describe("dryRun (DL-011: fail-safe, never dispatches a mutation or resolves a s socket.close(1000, "done"); } }); + + it("surfaces the extension's probe-failure reason on a dryRun write", async () => { + // #given a connected fake extension whose ref map resolves nothing + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + answerResolveRefsWith(socket, []); + + try { + // #when a consumer posts a dryRun click on a stale ref + const res = await postCommand( + sessionId, + CALLER_TOKEN_A, + { type: "click", commandId: "c-probe-reason", ref: "s1e9" }, + true, + ); + + // #then the simulated failure carries the extension's own reason + const event = await res.json(); + expect(event).toEqual({ + type: "action_result", + commandId: "c-probe-reason", + ok: false, + error: "dry-run: stale or unknown ref: s1e9", + simulated: true, + }); + } finally { + socket.close(1000, "done"); + } + }); +}); + +describe("extension liveness fail-fast", () => { + /** Bounded poll for an async status transition (onClose runs after the socket close). */ + async function waitForStatus(sessionId: string, want: SessionStatus): Promise { + const stub = await getSessionStub(sessionId); + for (let i = 0; i < 100; i++) { + const status = await stub.getStatus(); + if (status.status === want) return; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + throw new Error(`session never reached status '${want}'`); + } + + it("answers 503 immediately when no extension has ever connected - no timeout burn", async () => { + // #given a session with no extension attached (status stays "pending") + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when a consumer posts a command + const startedAt = Date.now(); + const res = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "get_tabs", + commandId: "c-no-ext", + }); + + // #then it fails fast as a retryable 503 (never the old 30s-timeout 500) + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "extension not connected" }); + expect(Date.now() - startedAt).toBeLessThan(5_000); + }); + + it("answers 503 after the extension detaches", async () => { + // #given a session whose extension connected and then dropped + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + socket.close(1000, "gone"); + await waitForStatus(sessionId, "detached"); + + // #when a consumer posts a command + const res = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "get_tabs", + commandId: "c-detached", + }); + + // #then it is refused as a retryable 503, not parked until the timeout + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "extension not connected" }); + }); + + it("still simulates a ref-less dryRun write with no extension connected", async () => { + // #given a session with no extension attached + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when a consumer posts a dryRun navigate (no ref, so no probe hits the wire) + const res = await postCommand( + sessionId, + CALLER_TOKEN_A, + { type: "navigate", commandId: "c-dry-no-ext", url: "https://example.com/" }, + true, + ); + + // #then the documented zero-wire semantic is preserved: simulated ok:true, + // NOT a 503 - a ref-less dry-run was never a liveness signal + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ + type: "action_result", + commandId: "c-dry-no-ext", + ok: true, + simulated: true, + }); + }); + + it("fails a ref-bearing dryRun fast when no extension is connected (the probe needs the wire)", async () => { + // #given a session with no extension attached + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when a consumer posts a dryRun click (its resolve_ref probe must round-trip) + const res = await postCommand( + sessionId, + CALLER_TOKEN_A, + { type: "click", commandId: "c-dry-probe-no-ext", ref: "s1e1" }, + true, + ); + + // #then the probe fails fast as 503 rather than burning the timeout + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "extension not connected" }); + }); + + it("refuses a real fill_secret on a disconnected session WITHOUT touching the vault", async () => { + // #given a seeded secret and a session with no extension attached + await seedVault("vault://gated-pw", "must-stay-unread"); + const sessionId = await openSession(CALLER_TOKEN_A); + const vaultGetSpy = vi.spyOn(env.VAULT, "get"); + + try { + // #when a consumer posts a real (non-dry) fill_secret + const res = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "fill_secret", + commandId: "c-fill-no-ext", + ref: "s1e1", + secretRef: "vault://gated-pw", + }); + + // #then it is refused as 503 and the secret was NEVER resolved - no + // plaintext materialized, no vault access emitted, for a command that + // could not dispatch (DL-004) + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "extension not connected" }); + expect(vaultGetSpy).not.toHaveBeenCalled(); + } finally { + vaultGetSpy.mockRestore(); + } + }); + + /** One full command round-trip over `socket` (get_tabs in, tabs_result out). */ + async function roundTrip(socket: WebSocket, sessionId: string, commandId: string): Promise { + const incoming = waitForCommand(socket); + const resPromise = postCommand(sessionId, CALLER_TOKEN_A, { type: "get_tabs", commandId }); + const received = await incoming; + socket.send(JSON.stringify({ type: "tabs_result", commandId: received.commandId, tabs: [] })); + return resPromise; + } + + it("recovers through the full lifecycle: pending 503 -> connected ok -> detached 503 -> reconnected ok", async () => { + // #given a fresh session (pending) + const sessionId = await openSession(CALLER_TOKEN_A); + expect((await postCommand(sessionId, CALLER_TOKEN_A, { type: "get_tabs", commandId: "l1" })).status).toBe(503); + + // #when an extension connects + const first = await connectFakeExtension(sessionId); + expect((await roundTrip(first, sessionId, "l2")).status).toBe(200); + + // #when it drops + first.close(1000, "gone"); + await waitForStatus(sessionId, "detached"); + expect((await postCommand(sessionId, CALLER_TOKEN_A, { type: "get_tabs", commandId: "l3" })).status).toBe(503); + + // #when it reconnects + const second = await connectFakeExtension(sessionId); + try { + // #then commands flow again - no wedged 503 after a reconnect + expect((await roundTrip(second, sessionId, "l4")).status).toBe(200); + } finally { + second.close(1000, "done"); + } + }); + + 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 + const sessionId = await openSession(CALLER_TOKEN_A); + const old = await connectFakeExtension(sessionId); + 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)); + + // #then the session is NOT stamped detached and commands still flow + 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"); + } + }); +}); + +describe("error taxonomy (route mapping)", () => { + it("rejects an unparseable JSON body as 400, not a masked 500", async () => { + // #given an open session + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when the body is not JSON at all + const res = await exports.default.fetch( + authedRequest(`/v1/sessions/${sessionId}/commands`, CALLER_TOKEN_A, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: "not json {", + }), + ); + + // #then it is the client's fault: 400 + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ error: "invalid body" }); + }); + + it("maps an in-flight command abandoned by a hello resync to the uniform JSON 500", async () => { + // #given a connected extension holding a command in flight + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + + try { + const incoming = waitForCommand(socket); + const resPromise = postCommand(sessionId, CALLER_TOKEN_A, { + type: "get_tabs", + commandId: "c-abandoned", + }); + await incoming; // the command is now parked in the coordinator + + // #when the extension resyncs (hello) instead of answering + socket.send( + JSON.stringify({ type: "hello", browser: "chrome", extVersion: "1.0.0", tabs: [] }), + ); + + // #then the abandoned dispatch surfaces as the uniform JSON 500 - + // app.onError's shape, with no stack or internals in the body + const res = await resPromise; + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ error: "internal error" }); + } finally { + socket.close(1000, "done"); + } + }); + + it( + "maps a timed-out command to 504 - the DO-integration proof of the real per-command timeout", + async () => { + // #given a connected extension that never answers (real ~30s timeout: + // fake timers cannot reach into the DO's I/O context - see the note in + // session.test.ts's dispatch suite) + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + + try { + // #when a consumer posts a command that will never be answered + const res = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "get_tabs", + commandId: "c-silent", + }); + + // #then the per-command timeout surfaces as 504, not an opaque 500 + expect(res.status).toBe(504); + expect(await res.json()).toEqual({ error: "command timed out" }); + + // #then the awaiting marker was cleared by the REAL DO-level timeout + // (the fake-timer coordinator test covers this deterministically; + // this re-asserts it through the integrated path, post-settlement) + const stub = await getSessionStub(sessionId); + await runInDurableObject(stub, (instance: SessionAgent) => { + expect(instance.state.awaitingCommandIds).toEqual([]); + }); + } finally { + socket.close(1000, "done"); + } + }, + 40_000, + ); }); diff --git a/apps/backend/test/session.test.ts b/apps/backend/test/session.test.ts index c2106aa..540cbae 100644 --- a/apps/backend/test/session.test.ts +++ b/apps/backend/test/session.test.ts @@ -102,6 +102,11 @@ 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 }); const dispatchPromise = instance.dispatch(cmd); // The marker is parked synchronously before dispatch() suspends. expect(instance.state.awaitingCommandIds).toContain("s1"); @@ -121,41 +126,11 @@ describe("dispatch / resolvePending", () => { }); }); - it( - "rejects a dispatched command with a payload-free error when its timeout fires", - async () => { - // #given a command sent via dispatch with no reply ever arriving, - // driven from a single runInDurableObject callback (see the comment - // above) and using the DO's real ~30s per-command timeout rather - // than vi.useFakeTimers(): a faked setTimeout firing outside a - // runInDurableObject call reaches into the Durable Object's - // SQLite-backed setState from the wrong I/O context and throws - // ("Cannot perform I/O on behalf of a different Durable Object") - - // verified empirically. coordinator.test.ts already covers this - // exact timeout/marker-clearing behavior deterministically with fake - // timers at the coordinator level (no DO involved); this test adds - // the DO-integration proof, so it pays the real ~30s cost once. - const sessionId = crypto.randomUUID(); - const stub = await getSessionStub(sessionId); - const cmd: Command = { type: "click", commandId: "s2", ref: "secret-ref" }; - - const err = await runInDurableObject(stub, async (instance: SessionAgent) => { - const dispatchPromise = instance.dispatch(cmd); - expect(instance.state.awaitingCommandIds).toContain("s2"); - - // #when the per-command timeout elapses - return dispatchPromise.catch((e: unknown) => e); - }); - - // #then it rejects with a payload-free error and clears the marker - expect(err).toBeInstanceOf(Error); - expect((err as Error).message).not.toContain("secret-ref"); - await runInDurableObject(stub, (instance: SessionAgent) => { - expect(instance.state.awaitingCommandIds).toEqual([]); - }); - }, - 35_000, - ); + // The real ~30s timeout's DO-integration proof lives in service.test.ts + // ("maps a timed-out command to 504"), which drives it through the full + // route -> RPC -> DO -> coordinator path against a connected-but-silent + // extension; coordinator.test.ts covers the timeout/marker-clearing + // machinery deterministically with fake timers. }); describe("DO eviction resilience (DL-007)", () => { @@ -219,6 +194,10 @@ describe("hello resync", () => { let outcome!: Promise<{ settled: "resolved" | "rejected"; value: unknown }>; 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 rejects before the + // marker is ever parked and there is nothing for the resync to abandon. + Object.assign(instance, { hasAuthorizedConnection: () => true }); outcome = instance.dispatch(cmd).then( (value) => ({ settled: "resolved", value }) as const, (error: unknown) => ({ settled: "rejected", value: error }) as const,