fix(backend): fail fast on disconnected sessions + honest error taxonomy

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

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<Event> {
+    // 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<Event>((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);

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;

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 {

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<Env, SessionState> {
           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<Env, SessionState> {
   }
 
   async onClose(connection: Connection, code: number, reason: string, wasClean: boolean): Promise<void> {
-    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<Event> {
@@ -136,26 +145,20 @@ export class SessionAgent extends Agent<Env, SessionState> {
       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<Event> {
     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<Env, SessionState> {
   // (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<boolean> {
-    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<Env, SessionState> {
   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;
+  }
 }

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;
 }
 
 /**

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,14 +54,32 @@ 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 {
       vi.useRealTimers();
     }
   });
 
+  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();