Classify transient WorkOS failures as retryable during MCP auth (#1352)

* Keep MCP sessions alive through transient WorkOS membership-check failures

The /mcp auth chain re-verifies WorkOS org membership on every request. A
transient WorkOS 429/5xx/timeout collapsed to organizationId=null, which
returned Forbidden — and a Forbidden carrying a session id schedules the
session Durable Object for destruction (in-flight executions, paused
approvals, undelivered results). A single WorkOS blip could mass-condemn
every session in a shared-API-key org.

Distinguish a failed membership lookup (retryable Unavailable -> 503 with
Retry-After, session left intact) from a lookup that succeeds with no
membership (genuine Forbidden). Session destruction now requires a positive
determination that access is gone, mirroring the existing reason system
classification on the JWKS/api-key verify paths.

No membership cache is added: a positive TTL cache would re-open the
revocation-liveness gap this check exists to close, and negative caching
would pin a blip as no access; the classification fix removes the
amplification risk without that tradeoff.

* Fail closed on definitive WorkOS denials in the MCP membership check

The previous commit classified every failed membership lookup as transient,
but WorkOS also answers with deterministic 4xx denials (401 revoked/invalid
API key, 403, 404 deleted org) that the SDK throws as typed exceptions and
the service boundary flattened into a status-less WorkOSError. Those were
being mapped to a retryable 503, preserving sessions indefinitely for a
revoked customer.

Thread the SDK exception's HTTP status onto WorkOSError at the service
boundary (same extraction as the workos-vault plugin) and split the failure
branch: 401/403/404 fail closed as Forbidden (session condemned), while
429/5xx/timeout/network stay retryable 503s.

Also: render Retry-After on the shared envelope's Unavailable branch (the
cloud agent-handler and envelope renderers had drifted), and document that
the transient 503 and terminated-session 404 intentionally share JSON-RPC
code -32001 with HTTP status as the discriminator.

Author: RhysSullivan

.changeset/workos-blip-session-survival.md

@@ -0,0 +1,5 @@
+---
+"executor": patch
+---
+
+Treat a transient WorkOS outage during the MCP live-membership check as a retryable 503 instead of a Forbidden that destroys the session.

apps/cloud/src/auth/errors.test.ts

@@ -0,0 +1,68 @@
+// The WorkOS failure-classification boundary: the service adapter must thread
+// the SDK exception's HTTP status onto the public `WorkOSError`, and the
+// definitive-denial predicate must split 401/403/404 (WorkOS answered "no" —
+// fail closed) from 429/5xx/no-status (transient — retryable). This split is
+// what keeps a WorkOS blip from condemning MCP sessions AND a revoked key from
+// being mistaken for a blip.
+import { describe, expect, it } from "@effect/vitest";
+import {
+  GenericServerException,
+  NotFoundException,
+  RateLimitExceededException,
+  UnauthorizedException,
+} from "@workos-inc/node/worker";
+
+import {
+  ServiceAdapterError,
+  isDefinitiveWorkOSDenial,
+  isDefinitiveWorkOSDenialStatus,
+  workosErrorFromFailure,
+} from "./errors";
+
+describe("workosErrorFromFailure", () => {
+  it("reads the status off the real SDK exceptions through the adapter wrapper", () => {
+    const cases: ReadonlyArray<readonly [unknown, number]> = [
+      [new UnauthorizedException("req_1"), 401],
+      [
+        new NotFoundException({ code: "not_found", message: "gone", path: "/x", requestID: "r" }),
+        404,
+      ],
+      [new RateLimitExceededException("slow down", "req_2", 1), 429],
+      [new GenericServerException(503, "upstream", {}, "req_3"), 503],
+    ];
+    for (const [sdkError, status] of cases) {
+      const wrapped = workosErrorFromFailure(new ServiceAdapterError({ cause: sdkError }));
+      expect(wrapped.status, `status for ${String(sdkError)}`).toBe(status);
+    }
+  });
+
+  it("leaves status unset for non-HTTP failures (network error, timeout)", () => {
+    // The shape fetch/undici raise on a network failure: an error value with a
+    // message and no HTTP status (statusless by construction, unlike the SDK's
+    // typed HTTP exceptions).
+    const networkFailure: unknown = { name: "TypeError", message: "fetch failed" };
+    const wrapped = workosErrorFromFailure(new ServiceAdapterError({ cause: networkFailure }));
+    expect(wrapped.status).toBeUndefined();
+  });
+});
+
+describe("isDefinitiveWorkOSDenial", () => {
+  it("treats 401/403/404 as definitive and everything else as transient", () => {
+    expect(isDefinitiveWorkOSDenialStatus(401)).toBe(true);
+    expect(isDefinitiveWorkOSDenialStatus(403)).toBe(true);
+    expect(isDefinitiveWorkOSDenialStatus(404)).toBe(true);
+    expect(isDefinitiveWorkOSDenialStatus(429)).toBe(false);
+    expect(isDefinitiveWorkOSDenialStatus(500)).toBe(false);
+    expect(isDefinitiveWorkOSDenialStatus(503)).toBe(false);
+    expect(isDefinitiveWorkOSDenialStatus(undefined)).toBe(false);
+  });
+
+  it("only claims WorkOSError instances, never arbitrary errors with a status", () => {
+    expect(isDefinitiveWorkOSDenial({ status: 403 })).toBe(false);
+    expect(
+      isDefinitiveWorkOSDenial(
+        workosErrorFromFailure(new ServiceAdapterError({ cause: new UnauthorizedException("r") })),
+      ),
+    ).toBe(true);
+  });
+});

apps/cloud/src/auth/errors.ts

@@ -1,4 +1,4 @@
-import { Data, Effect, Schema } from "effect";
+import { Data, Effect, Option, Predicate, Schema } from "effect";
 
 export class UserStoreError extends Schema.TaggedErrorClass<UserStoreError>()(
   "UserStoreError",
@@ -8,10 +8,69 @@ export class UserStoreError extends Schema.TaggedErrorClass<UserStoreError>()(
 
 export class WorkOSError extends Schema.TaggedErrorClass<WorkOSError>()(
   "WorkOSError",
-  {},
+  {
+    /**
+     * The upstream HTTP status WorkOS answered with, when the failure WAS an
+     * HTTP answer (the SDK sets `.status` on all its typed exceptions).
+     * Absent for network errors / timeouts. Consumers that must distinguish
+     * "WorkOS said no" (definitive 4xx) from "WorkOS was unreachable"
+     * (transient) branch on this — see `isDefinitiveWorkOSDenialStatus`.
+     */
+    status: Schema.optional(Schema.Number),
+  },
   { httpApiStatus: 500 },
 ) {}
 
+// Statuses that are a DEFINITIVE denial from WorkOS — a deterministic "this
+// request cannot be authorized" answer (invalid/revoked API key, forbidden,
+// resource gone), not a blip. Retrying does not help, and auth decisions built
+// on them must fail CLOSED. Everything else (429, 5xx, or no status at all —
+// network error/timeout) is transient and must stay retryable.
+const DEFINITIVE_DENIAL_STATUSES: ReadonlySet<number> = new Set([401, 403, 404]);
+
+export const isDefinitiveWorkOSDenialStatus = (status: number | undefined): boolean =>
+  status !== undefined && DEFINITIVE_DENIAL_STATUSES.has(status);
+
+// Tag-based guards (same pattern as `isOAuth2Error` in core/sdk/oauth-helpers):
+// the untyped failure channels these run against carry values that crossed
+// layer boundaries, so the discriminant is the `_tag`, not the prototype chain.
+const isWorkOSError = Predicate.isTagged("WorkOSError") as (error: unknown) => error is WorkOSError;
+
+/** A `WorkOSError` that is a definitive denial (see above), typed on `unknown`
+ *  so auth code holding an untyped failure channel can branch safely. */
+export const isDefinitiveWorkOSDenial = (error: unknown): boolean =>
+  isWorkOSError(error) && isDefinitiveWorkOSDenialStatus(error.status);
+
+// Every typed exception the WorkOS node SDK throws for an HTTP answer carries a
+// numeric `.status` (UnauthorizedException 401, NotFoundException 404,
+// RateLimitExceededException 429, GenericServerException/OauthException with the
+// live status, ...). Network errors and timeouts throw non-SDK errors with no
+// status. Same extraction pattern as the workos-vault plugin's
+// `statusFromWorkOSCause` (packages/plugins/workos-vault/src/sdk/client.ts).
+const CauseWithStatusSchema = Schema.Struct({ status: Schema.Number });
+const decodeCauseWithStatusOption = Schema.decodeUnknownOption(CauseWithStatusSchema);
+
+export const statusFromWorkOSCause = (cause: unknown): number | undefined =>
+  Option.match(decodeCauseWithStatusOption(cause), {
+    onNone: () => undefined,
+    onSome: (decoded) => decoded.status,
+  });
+
+/**
+ * Build the public `WorkOSError` for a WorkOS service-adapter failure,
+ * threading the upstream HTTP status through when the underlying SDK exception
+ * carried one. The failure is normally the `ServiceAdapterError` wrapper from
+ * `tryPromiseService` (SDK exception in `.cause`); a bare cause also works.
+ */
+const isServiceAdapterError = Predicate.isTagged("ServiceAdapterError") as (
+  failure: unknown,
+) => failure is ServiceAdapterError;
+
+export const workosErrorFromFailure = (failure: unknown): WorkOSError =>
+  new WorkOSError({
+    status: statusFromWorkOSCause(isServiceAdapterError(failure) ? failure.cause : failure),
+  });
+
 export class ApiKeyManagementError extends Schema.TaggedErrorClass<ApiKeyManagementError>()(
   "ApiKeyManagementError",
   { cause: Schema.Unknown },
@@ -47,7 +106,11 @@ export const tryPromiseService = <A>(fn: () => Promise<A>): Effect.Effect<A, Ser
  */
 export const withServiceLogging = <A, E, R>(
   name: string,
-  publicError: () => E,
+  // Receives the raw failure (for service adapters: the `ServiceAdapterError`
+  // whose `.cause` is the SDK exception) so the public error can carry safe
+  // classification fields (e.g. `WorkOSError.status`). Zero-arg callers that
+  // ignore the failure keep working unchanged.
+  publicError: (failure: unknown) => E,
   effect: Effect.Effect<A, unknown, R>,
 ): Effect.Effect<A, E, R> =>
   effect.pipe(

apps/cloud/src/auth/organization.ts

@@ -57,9 +57,19 @@ export const resolveOrganization = (organizationId: string) =>
 // until their access token naturally expires (~10 min).
 //
 // To close that gap we verify membership live on every protected request.
-// `listUserMemberships` is one WorkOS call per request. If this becomes a
-// hot path we can layer a short per-(user, org) TTL cache underneath, or
-// swap it for a local memberships table fed by the WorkOS Events API.
+// `listUserMemberships` is one WorkOS call per request.
+//
+// Caching decision (2026-07): we deliberately do NOT add a positive TTL cache
+// here. A positive cache is exactly what would re-open the revocation gap this
+// live check exists to close — a revoked member would keep access for the cache
+// TTL. Negative caching is worse still (a transient WorkOS blip would get
+// pinned as "no access"), so it is out too. The rate-limit amplification a
+// shared-API-key org can cause under a WorkOS slowdown is mitigated instead by
+// the classification fix at the MCP call site (a blip now yields a retryable
+// 503, so it no longer condemns sessions or triggers reconnect storms). If per-
+// request WorkOS load later proves to be the bottleneck, the right structural
+// fix is a local memberships table fed by the WorkOS Events API (authoritative,
+// no staleness window), not a TTL cache over this call — tracked as follow-up.
 //
 // Returns the resolved organization (via resolveOrganization) if the user
 // currently holds an *active* membership in it, otherwise null. Callers

apps/cloud/src/auth/workos.ts

@@ -6,7 +6,7 @@ import { env } from "cloudflare:workers";
 import { Context, Data, Effect, Layer, Option, Schema } from "effect";
 import { GeneratePortalLinkIntent, WorkOS } from "@workos-inc/node/worker";
 import { parseCookie } from "./cookies";
-import { WorkOSError, tryPromiseService, withServiceLogging } from "./errors";
+import { tryPromiseService, withServiceLogging, workosErrorFromFailure } from "./errors";
 
 const COOKIE_NAME = "wos-session";
 const INVALID_COOKIE_PASSWORD_MESSAGE = "WORKOS_COOKIE_PASSWORD must be at least 32 characters";
@@ -147,10 +147,14 @@ const make = Effect.gen(function* () {
 
   const workos = new WorkOS({ apiKey, clientId, ...workosApiUrlOptions(env.WORKOS_API_URL) });
 
+  // The public `WorkOSError` carries the upstream HTTP status when the SDK
+  // exception had one (all its typed exceptions do), so consumers can tell a
+  // definitive WorkOS denial (401/403/404 — fail closed) from a transient
+  // failure (429/5xx/network — retryable).
   const use = <A>(fn: (wos: WorkOS) => Promise<A>) =>
     withServiceLogging(
       "workos",
-      () => new WorkOSError(),
+      workosErrorFromFailure,
       tryPromiseService(() => fn(workos)),
     );
 

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

@@ -4,6 +4,7 @@ import {
   McpAuthProvider,
   jsonRpcErrorBody,
   defaultMcpResource,
+  UNAVAILABLE_RETRY_AFTER_SECONDS,
   type AuthOutcome,
   type McpResource,
 } from "@executor-js/host-mcp";
@@ -57,7 +58,21 @@ const renderAuthError = (
   if (Predicate.isTagged(outcome, "Forbidden")) {
     return jsonRpcResponse(403, outcome.code ?? -32001, outcome.message);
   }
-  return jsonRpcResponse(503, -32001, outcome.message);
+  // Unavailable: a transient auth-infra failure (JWKS blip OR a WorkOS
+  // membership-lookup 429/5xx/timeout). Both are retryable, so advertise a
+  // Retry-After so the client (and any polite retry layer) backs off instead of
+  // hammering (same rendering as the shared envelope's Unavailable branch).
+  // Crucially, this path NEVER reaches the session-destroy branch below — a
+  // transient failure must not condemn a live session.
+  //
+  // Note this 503 shares JSON-RPC code -32001 with the terminated-session 404
+  // ("Session timed out, please reconnect"); that is intentional — -32001 is
+  // the generic auth/session envelope code, and the HTTP STATUS is the
+  // discriminator clients act on: 503 = retry the SAME session id, 404 = the
+  // id is dead, reconnect.
+  return jsonRpcErrorBody(503, -32001, outcome.message, {
+    retryAfterSeconds: UNAVAILABLE_RETRY_AFTER_SECONDS,
+  });
 };
 
 const authenticate = (request: Request) =>
@@ -99,7 +114,10 @@ const propsForPrincipal = (
   });
 
 export const makeCloudMcpAgentHandler = () => {
-  const serveOptions = { binding: "MCP_SESSION", transport: "streamable-http" } as const;
+  const serveOptions = {
+    binding: "MCP_SESSION",
+    transport: "streamable-http",
+  } as const;
   // The agents SDK builds an exact-match `URLPattern` from the path handed to
   // `serve` (see `createStreamingHttpHandler` in `agents/dist/mcp/index.js`) —
   // a single `/mcp` handler never matches `/mcp/toolkits/<slug>` and falls
@@ -126,6 +144,11 @@ export const makeCloudMcpAgentHandler = () => {
 
     const { auth, outcome } = await Effect.runPromise(authenticate(request));
     if (!Predicate.isTagged(outcome, "Authenticated")) {
+      // Destroying a live session on auth grounds requires a POSITIVE
+      // determination that access is genuinely gone — only `Forbidden` carries
+      // that (valid bearer, org absent/revoked). `Unavailable` (transient WorkOS
+      // / JWKS failure) and `Unauthorized` (retry with a fresh token) must leave
+      // the session intact, so the condemn path is gated on `Forbidden` alone.
       if (Predicate.isTagged(outcome, "Forbidden") && sessionId) {
         await Effect.runPromise(
           Effect.ignore(
@@ -142,7 +165,10 @@ export const makeCloudMcpAgentHandler = () => {
       // Matches the old envelope's contract (@modelcontextprotocol/sdk's
       // `WebStandardStreamableHTTPServerTransport.handleDeleteRequest`): 200,
       // not 204 — see e2e/cloud/mcp-protocol.test.ts.
-      return new Response(null, { status: 200, headers: { "access-control-allow-origin": "*" } });
+      return new Response(null, {
+        status: 200,
+        headers: { "access-control-allow-origin": "*" },
+      });
     }
 
     if (sessionId) {