MCP session cold init resilience

apps/cloud/src/auth/context.ts

@@ -1,7 +1,7 @@
 import { Context, Effect, Layer } from "effect";
 import { makeUserStore } from "../auth/user-store";
 import { DbService } from "../db/db";
-import { UserStoreError, tryPromiseService, withServiceLogging } from "./errors";
+import { tryPromiseService, userStoreErrorFromFailure, withServiceLogging } from "./errors";
 
 // ---------------------------------------------------------------------------
 // UserStoreService — wraps the Drizzle-backed user store with Effect
@@ -10,13 +10,15 @@ import { UserStoreError, tryPromiseService, withServiceLogging } from "./errors"
 type RawStore = ReturnType<typeof makeUserStore>;
 
 // `op` names the store call so every span reads `user_store.<operation>`
-// instead of one undifferentiated "user_store" bucket, and failures log which
-// query actually failed.
+// instead of one undifferentiated "user_store" bucket, failures log which
+// query actually failed, and — because the same `op` is threaded onto the
+// public error alongside the classified driver reason — an error report is
+// diagnosable without the trace.
 const makeService = (store: RawStore) => ({
   use: <A>(op: string, fn: (s: RawStore) => Promise<A>) =>
     withServiceLogging(
       `user_store.${op}`,
-      () => new UserStoreError(),
+      (failure) => userStoreErrorFromFailure(op, failure),
       tryPromiseService(() => fn(store)),
     ),
 });

apps/cloud/src/auth/errors.ts

@@ -1,10 +1,103 @@
 import { Data, Effect, Option, Predicate, Schema } from "effect";
 
+// How a user-store call failed, classified from the driver cause. Safe to put
+// on the wire and on a Sentry tag: it names a failure MODE, never a query, a
+// value, or a customer.
+export const USER_STORE_FAILURE_REASONS = [
+  "connect_timeout",
+  "connection_closed",
+  "query",
+  "unknown",
+] as const;
+
+export type UserStoreFailureReason = (typeof USER_STORE_FAILURE_REASONS)[number];
+
+/**
+ * The public failure of every cloud user-store call.
+ *
+ * It carries the two fields that make an issue diagnosable from the error
+ * alone: which store call failed, and how. Before those existed the error had
+ * an empty field set, so Sentry showed a titleless, messageless issue and the
+ * only cause detail (the pretty-printed Effect cause in a Sentry `extra`) is
+ * scrubbed server-side — the failing operation and the driver reason existed
+ * only in the trace store. Same shape as `WorkOSError.status`: a small, safe
+ * classification field threaded at the service boundary.
+ */
 export class UserStoreError extends Schema.TaggedErrorClass<UserStoreError>()(
   "UserStoreError",
-  {},
+  {
+    /** The store call that failed, e.g. `getOrganization`. */
+    operation: Schema.String,
+    /** How it failed, classified from the driver cause chain. */
+    reason: Schema.Literals(USER_STORE_FAILURE_REASONS),
+  },
   { httpApiStatus: 500 },
-) {}
+) {
+  override get message(): string {
+    return `user store ${this.operation} failed: ${this.reason}`;
+  }
+}
+
+/** Reasons a retry can plausibly clear: the query never reached a healthy
+ *  server. A `query` failure is deterministic and must not be retried. */
+export const isTransientUserStoreReason = (reason: UserStoreFailureReason): boolean =>
+  reason === "connect_timeout" || reason === "connection_closed";
+
+// postgres.js tags its connection failures with a string `code`
+// (`CONNECT_TIMEOUT`, `CONNECTION_CLOSED`, …) and its query failures with the
+// SQLSTATE. Drizzle re-throws both wrapped in its own "Failed query" error with
+// the driver error in `.cause`, and the service adapter wraps that again, so
+// the classification walks the chain rather than inspecting one level.
+const MAX_CAUSE_DEPTH = 8;
+
+const REASON_BY_DRIVER_CODE: Readonly<Record<string, UserStoreFailureReason>> = {
+  CONNECT_TIMEOUT: "connect_timeout",
+  ETIMEDOUT: "connect_timeout",
+  CONNECTION_CLOSED: "connection_closed",
+  CONNECTION_ENDED: "connection_closed",
+  CONNECTION_DESTROYED: "connection_closed",
+  ECONNREFUSED: "connection_closed",
+  ECONNRESET: "connection_closed",
+};
+
+const stringCodeOf = (value: unknown): string | undefined => {
+  if (typeof value !== "object" || value === null) return undefined;
+  const code = (value as { readonly code?: unknown }).code;
+  return typeof code === "string" ? code : undefined;
+};
+
+const driverCodesOf = (failure: unknown): readonly string[] => {
+  const codes: string[] = [];
+  let current: unknown = isServiceAdapterError(failure) ? failure.cause : failure;
+  for (
+    let depth = 0;
+    depth < MAX_CAUSE_DEPTH && current !== undefined && current !== null;
+    depth++
+  ) {
+    const code = stringCodeOf(current);
+    if (code !== undefined) codes.push(code);
+    current =
+      typeof current === "object" ? (current as { readonly cause?: unknown }).cause : undefined;
+  }
+  return codes;
+};
+
+/** Classify a raw store failure. A recognised connection code wins; any other
+ *  driver code (a SQLSTATE) is a deterministic query failure; nothing at all is
+ *  `unknown`. */
+export const userStoreReasonFromCause = (failure: unknown): UserStoreFailureReason => {
+  const codes = driverCodesOf(failure);
+  for (const code of codes) {
+    const reason = REASON_BY_DRIVER_CODE[code];
+    if (reason !== undefined) return reason;
+  }
+  return codes.length > 0 ? "query" : "unknown";
+};
+
+/** Build the public `UserStoreError` for a store-adapter failure, naming the
+ *  operation the call site already knows and classifying the driver cause. */
+export const userStoreErrorFromFailure = (operation: string, failure: unknown): UserStoreError =>
+  new UserStoreError({ operation, reason: userStoreReasonFromCause(failure) });
 
 export class WorkOSError extends Schema.TaggedErrorClass<WorkOSError>()(
   "WorkOSError",

apps/cloud/src/auth/user-store-error.node.test.ts

@@ -0,0 +1,128 @@
+// `UserStoreError` is the public failure of every cloud user-store call, and it
+// used to carry NOTHING: no operation, no reason, no message. Sentry grouped
+// every store failure — a connect timeout, a missing table, a constraint
+// violation — into one titleless issue, and the only cause detail (the
+// pretty-printed Effect cause stuffed into a Sentry `extra`) is scrubbed
+// server-side, so the issue was undiagnosable from Sentry alone.
+//
+// This pins the two safe classification fields it must carry instead:
+// `operation` (already in hand at the call site) and `reason` (classified from
+// the driver cause the way `statusFromWorkOSCause` classifies WorkOS causes).
+import { createServer, type Server, type Socket } from "node:net";
+
+import { describe, expect, it } from "@effect/vitest";
+import { Effect, Layer, Result } from "effect";
+import postgres from "postgres";
+
+import { UserStoreService } from "./context";
+import { ServiceAdapterError, userStoreReasonFromCause, type UserStoreError } from "./errors";
+import { DbService } from "../db/db";
+
+// A socket that completes the TCP handshake and then says nothing — exactly
+// what a wedged Hyperdrive/Postgres endpoint looks like to postgres.js, and the
+// only way to obtain the driver's REAL connect-timeout error object rather than
+// a hand-written imitation of it.
+const blackHolePort = async (): Promise<{
+  readonly port: number;
+  readonly close: () => Promise<void>;
+}> => {
+  const sockets = new Set<Socket>();
+  const server: Server = createServer((socket) => {
+    sockets.add(socket);
+    socket.on("close", () => sockets.delete(socket));
+  });
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
+  const address = server.address();
+  // oxlint-disable-next-line executor/no-error-constructor, executor/no-try-catch-or-throw -- boundary: the fixture cannot run without a bound port
+  if (address === null || typeof address === "string") throw new Error("no port");
+  return {
+    port: address.port,
+    close: () =>
+      new Promise<void>((resolve) => {
+        // The timed-out client leaves its half-open socket attached; without
+        // dropping it first, `close` waits for a peer that will never speak.
+        for (const socket of sockets) socket.destroy();
+        server.close(() => resolve());
+      }),
+  };
+};
+
+const realConnectTimeoutError = async (): Promise<unknown> => {
+  const hole = await blackHolePort();
+  const sql = postgres(`postgresql://postgres:postgres@127.0.0.1:${hole.port}/postgres`, {
+    max: 1,
+    connect_timeout: 1,
+    fetch_types: false,
+    onnotice: () => undefined,
+  });
+  // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: capturing the driver's own thrown error IS the fixture
+  try {
+    await sql`select 1`;
+    // oxlint-disable-next-line executor/no-error-constructor, executor/no-try-catch-or-throw -- boundary: the fixture is unusable if the socket answered
+    throw new Error("expected the connect to time out");
+  } catch (error) {
+    return error;
+  } finally {
+    // oxlint-disable-next-line executor/no-promise-catch -- boundary: best-effort teardown of a connection that never opened
+    await sql.end({ timeout: 0 }).catch(() => undefined);
+    await hole.close();
+  }
+};
+
+// Drizzle re-throws driver failures wrapped in its own error with the failing
+// SQL in the message and the driver error in `.cause` — the shape production
+// actually reports (`Failed query: select … -> write CONNECT_TIMEOUT …`).
+const wrappedLikeDrizzle = (cause: unknown): Error =>
+  // oxlint-disable-next-line executor/no-error-constructor -- boundary: reproducing the driver wrapper shape the store really fails with
+  Object.assign(new Error('Failed query: select "id" from "organizations" where "id" = $1'), {
+    cause,
+  });
+
+const stubDb = Layer.succeed(DbService)({ db: {} as never });
+
+const failingStoreCall = (
+  operation: string,
+  failure: unknown,
+): Effect.Effect<Result.Result<never, UserStoreError>> =>
+  Effect.gen(function* () {
+    const users = yield* UserStoreService;
+    // oxlint-disable-next-line executor/no-promise-reject -- boundary: the store adapter lifts a REJECTING promise; rejecting is the fixture
+    return yield* users.use(operation, () => Promise.reject(failure));
+  }).pipe(
+    Effect.provide(UserStoreService.Live.pipe(Layer.provide(stubDb))),
+    Effect.result,
+  ) as Effect.Effect<Result.Result<never, UserStoreError>>;
+
+describe("UserStoreError classification", () => {
+  it("classifies the driver cause chain", async () => {
+    const driverError = await realConnectTimeoutError();
+
+    expect(userStoreReasonFromCause(wrappedLikeDrizzle(driverError))).toBe("connect_timeout");
+    expect(
+      userStoreReasonFromCause(new ServiceAdapterError({ cause: wrappedLikeDrizzle(driverError) })),
+    ).toBe("connect_timeout");
+    expect(
+      userStoreReasonFromCause(
+        // oxlint-disable-next-line executor/no-error-constructor -- boundary: a SQLSTATE failure shape
+        Object.assign(new Error('relation "organizations" does not exist'), { code: "42P01" }),
+      ),
+    ).toBe("query");
+    // oxlint-disable-next-line executor/no-error-constructor -- boundary: a failure with nothing to classify
+    expect(userStoreReasonFromCause(new Error("something else"))).toBe("unknown");
+  }, 20_000);
+
+  it("carries operation, reason and a stable message on the public error", async () => {
+    const driverError = await realConnectTimeoutError();
+
+    const result = await Effect.runPromise(
+      failingStoreCall("getOrganization", wrappedLikeDrizzle(driverError)),
+    );
+
+    expect(Result.isFailure(result)).toBe(true);
+    if (!Result.isFailure(result)) return;
+    expect(result.failure.operation).toBe("getOrganization");
+    expect(result.failure.reason).toBe("connect_timeout");
+    expect(result.failure.message).toContain("getOrganization");
+    expect(result.failure.message).toContain("connect_timeout");
+  }, 20_000);
+});

apps/cloud/src/mcp/agent-handler.ts

@@ -27,9 +27,12 @@ import { mcpSessionStub } from "@executor-js/cloudflare/mcp/session-stub";
 import { wrapMcpSseResponse } from "../observability/memory-metrics";
 import { WorkerTelemetryLive } from "../observability/telemetry";
 import { cloudMcpAuth } from "./auth-provider";
+import { isMcpSessionMetaUnavailable } from "./session-meta";
 import { McpSessionDOSqlite } from "./session-durable-object";
 import { parseTraceparent } from "./traceparent";
 
+const MCP_SESSION_UNAVAILABLE_MESSAGE = "Session storage temporarily unavailable - please retry";
+
 const corsPreflightResponse = (): Response =>
   new Response(null, {
     status: 204,
@@ -171,6 +174,13 @@ const propsForPrincipal = (
     return {
       session: {
         organizationId: principal.organizationId,
+        // The org record the live membership check resolved microseconds ago,
+        // handed to the session DO so it never opens a connection of its own to
+        // re-read it. An unnamed org (no auth plane could resolve one) is
+        // omitted rather than sent empty, so the DO can tell "not carried" from
+        // "carried, and blank".
+        ...(principal.organizationName ? { organizationName: principal.organizationName } : {}),
+        ...(principal.organizationSlug ? { organizationSlug: principal.organizationSlug } : {}),
         userId: principal.accountId,
         elicitationMode: readElicitationMode(request),
         artifactsEnabled: readArtifactsEnabled(request),
@@ -306,6 +316,23 @@ export const makeCloudMcpAgentHandler = () => {
       // vocabulary — a deploy, a storage timeout, a cancelled
       // blockConcurrencyWhile — which reaches here through the agents SDK's own
       // `getServerByName` retry and used to 500 identically.
+      // The session DO could not reach the organization directory to name the
+      // org (after its own bounded retry). Transient by construction, so it
+      // gets the same retryable envelope a WorkOS blip gets on the auth path —
+      // not an unclassified 500 the agents SDK then retries the whole DO
+      // operation over, which is what turned a 10s connect timeout into a
+      // half-minute client hang.
+      //
+      // Checked BEFORE the platform classifier: this is an application failure
+      // that merely escapes through the same seam, and it names its own cause.
+      // The classifier only recognizes the runtime's own reset vocabulary, so
+      // the two never contend — the order just keeps it that way if either
+      // vocabulary grows.
+      if (isMcpSessionMetaUnavailable(error)) {
+        return jsonRpcErrorBody(503, -32001, MCP_SESSION_UNAVAILABLE_MESSAGE, {
+          retryAfterSeconds: UNAVAILABLE_RETRY_AFTER_SECONDS,
+        });
+      }
       const failure = classifyDurableObjectError(error);
       if (!failure) {
         // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't a recognized platform failure to the Workers runtime unchanged

apps/cloud/src/mcp/auth-provider.test.ts

@@ -70,9 +70,11 @@ const stubOrgAuthNoMembership = Layer.succeed(McpOrganizationAuth)({
   authorize: () => Effect.succeed(null),
 });
 
-// `authorize` SUCCEEDS with an org id — active membership.
+// `authorize` SUCCEEDS with the resolved org record — active membership. The
+// record, not just the id: the session props carry the org's name and slug so
+// the session DO never re-reads the row.
 const stubOrgAuthActive = Layer.succeed(McpOrganizationAuth)({
-  authorize: () => Effect.succeed(ORG_ID),
+  authorize: () => Effect.succeed({ id: ORG_ID, name: "Stub Org", slug: "stub-org" }),
 });
 
 // A failure that is not a WorkOSError at all (e.g. the per-request DB layer

apps/cloud/src/mcp/auth-provider.ts

@@ -55,6 +55,7 @@ import {
   McpAuthLive,
   McpOrganizationAuth,
   McpOrganizationAuthLive,
+  type AuthorizedMcpOrganization,
   type McpAuthResult,
   type VerifiedToken,
 } from "./auth";
@@ -88,16 +89,24 @@ const ORGANIZATION_AUTHORIZE_UNAVAILABLE =
 
 /**
  * Enrich a cloud {@link VerifiedToken} (which carries only accountId +
- * organizationId) into the full {@link Principal} the seam validates. The
- * envelope only uses `accountId` + `organizationId` for ownership; cloud
- * resolves org name/email inside the DO, so the cosmetic identity fields carry
- * empty placeholders. `organizationId` is guaranteed non-null here because the
- * Forbidden branch already rejected the no-org case before Authenticated.
+ * organizationId) into the full {@link Principal} the seam validates.
+ *
+ * The org name and slug come from the record the live membership check just
+ * resolved — this is the whole point of `authorize` returning the record rather
+ * than an id. They used to be dropped here (`organizationName: ""`), which left
+ * the session Durable Object to re-read the same row over a fresh database
+ * connection on every cold init. `email` stays a placeholder: the envelope only
+ * uses `accountId` + `organizationId` for ownership, and nothing downstream
+ * reads it.
  */
-const principalFromToken = (token: VerifiedToken, organizationId: string): Principal => ({
+const principalFromToken = (
+  token: VerifiedToken,
+  organization: AuthorizedMcpOrganization,
+): Principal => ({
   accountId: token.accountId,
-  organizationId,
-  organizationName: "",
+  organizationId: organization.id,
+  organizationName: organization.name,
+  ...(organization.slug === undefined ? {} : { organizationSlug: organization.slug }),
   email: "",
   name: null,
   avatarUrl: null,
@@ -223,9 +232,9 @@ export const cloudMcpAuthProviderLayer: Layer.Layer<
         // caller genuinely holds no active membership (revoked / never a member)
         // — a real Forbidden, which the handler may act on by condemning the
         // session.
-        const organizationId = authorizeResult.success;
-        if (!organizationId) return forbidden(NO_ORGANIZATION_MESSAGE, -32001);
-        return authenticated(principalFromToken(token, organizationId));
+        const organization = authorizeResult.success;
+        if (!organization) return forbidden(NO_ORGANIZATION_MESSAGE, -32001);
+        return authenticated(principalFromToken(token, organization));
       });
 
     const toOutcome = (request: Request, result: McpAuthResult): Effect.Effect<AuthOutcome> => {