diff --git a/.changeset/mirror-readiness-removal.md b/.changeset/mirror-readiness-removal.md new file mode 100644 index 000000000..e6ad82e6a --- /dev/null +++ b/.changeset/mirror-readiness-removal.md @@ -0,0 +1,5 @@ +--- +"@executor-js/cloud": patch +--- + +`authorizeOrganization` now reads the local membership mirror unconditionally: the per-request readiness check (`MirrorReadiness`) and its live WorkOS `listUserMemberships` fallback are gone from the request path entirely. The backfill is complete and permanent, and an organization that predates the mirror is still covered by the existing on-demand scan (`ensureOrganizationBackfilled`). A stalled reconciler is now an operational alert instead of a per-request fallback: after each run, the cron checks the mirror's `drained_at` heartbeat and, if it has fallen behind the lag budget, logs a structured error and reports it to Sentry. The deploy gate (`scripts/ensure-workos-mirror-ready.ts`) is unchanged — it still refuses to ship while the mirror is unready — and `drained_at` keeps being written by every reconciler run. diff --git a/.claude/skills/prod-telemetry/SKILL.md b/.claude/skills/prod-telemetry/SKILL.md index 9af858ca0..cfd07d452 100644 --- a/.claude/skills/prod-telemetry/SKILL.md +++ b/.claude/skills/prod-telemetry/SKILL.md @@ -57,22 +57,33 @@ join the same traces via traceparent). `execute`/`execute-action` calls `mcp.execute.code` (the script itself, capped at 10k chars — cloud-only content capture; local/self-host telemetry never records content). -- `auth.authorize_organization` — every membership authorization. - `mirror.ready` (bool: the local membership mirror answered; `false` = - the request fell back to a live WorkOS read) and `mirror.readiness` - (why: `ready`, `backfill pending: …`, `reconciler stale: …`). The - mirror's write spans are `workos_mirror.`; the reconciler run is +- `auth.authorize_organization` — every membership authorization. Reads the + local membership mirror unconditionally; there is no per-request readiness + check and no WorkOS fallback, so this span carries no readiness attribute. + The mirror's write spans are `workos_mirror.`; the reconciler run is `workos_events.sync`. `workos_sync.drained_at` in the prod DB is the - reconciler heartbeat. + reconciler heartbeat, and a stalled reconciler now raises its own error + from the cron (see below) rather than showing up as a fallback here. -**Recipe — membership-mirror fallback rate (should be ~0 after cutover):** +**Recipe — reconciler heartbeat (ticks should land roughly every minute; a +gap wider than the 10-minute lag budget means the cron alert should already +have fired — see `workos_events: reconciler stale` below):** ```apl ['executor-cloud'] -| where _time > ago(1h) and name == "auth.authorize_organization" -| extend ready = tobool(['attributes.custom']['mirror.ready']) -| extend why = tostring(['attributes.custom']['mirror.readiness']) -| summarize n = count() by ready, why +| where _time > ago(1h) and name == "workos_events.sync" +| summarize n = count() by bin(_time, 1m) +| sort by _time desc +``` + +**Recipe — stale-reconciler alerts (should be empty; each row is one paging +event):** + +```apl +['executor-cloud'] +| where _time > ago(1d) and ['status.message'] contains "workos_events: reconciler stale" +| project _time, trace_id, msg = tostring(['status.message']) +| sort by _time desc ``` **Recipe — error signatures by class (the daily-digest query):** diff --git a/apps/cloud/src/account/account-api.ts b/apps/cloud/src/account/account-api.ts index 37b4cb26e..d9aaf70d2 100644 --- a/apps/cloud/src/account/account-api.ts +++ b/apps/cloud/src/account/account-api.ts @@ -10,7 +10,6 @@ import { import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { sessionFromSealed, type Session } from "../auth/middleware"; import { WorkOSClient } from "../auth/workos"; @@ -51,7 +50,9 @@ import { AccountCaller, workosAccountProvider } from "./workos-account-service"; // `UserStoreService` / `WorkOsMirror` / `MemberDirectory` are supplied by the // combined `rsLive` layer. // `ApiKeyService.WorkOS` is built here on top of the boot `WorkOSClient`. -const AccountProviderMiddleware = HttpRouter.middleware<{ provides: AccountProvider }>()( +const AccountProviderMiddleware = HttpRouter.middleware<{ + provides: AccountProvider; +}>()( Effect.gen(function* () { // Long-lived services only (built once at boot). `UserStoreService` and // `DbService` are NOT grabbed here — they come per request from the combined @@ -100,15 +101,11 @@ const AccountProviderMiddleware = HttpRouter.middleware<{ provides: AccountProvi * (the seat-gate) stays a residual requirement, satisfied by the app `boot`. */ export const workosAccountMiddleware = ( - rsLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness - >, + rsLive: Layer.Layer, ) => AccountProviderMiddleware.combine(requestScopedMiddleware(rsLive)).layer; export const makeAccountApiLive = ( - rsLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness - >, + rsLive: Layer.Layer, ) => { // Cloud builds the WorkOS `AccountProvider` INSIDE the request body (so it // closes over the per-request postgres socket), so it can't be a self- diff --git a/apps/cloud/src/account/org-api-key-revoke.node.test.ts b/apps/cloud/src/account/org-api-key-revoke.node.test.ts index 02228f370..48db9c2df 100644 --- a/apps/cloud/src/account/org-api-key-revoke.node.test.ts +++ b/apps/cloud/src/account/org-api-key-revoke.node.test.ts @@ -6,7 +6,6 @@ import { AccountError, AccountForbidden } from "@executor-js/api"; import { ApiKeyService, OrgApiKeyNotFound } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../auth/mirror-readiness"; import { ORG_SELECTOR_HEADER } from "../auth/organization"; import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { WorkOsMirror } from "../auth/workos-mirror"; @@ -132,12 +131,6 @@ const stubMirror = Layer.succeed(WorkOsMirror)({ // The mirror as the directory reads it: both are active members of ORG, and // only ADMIN carries the `admin` role. Revoke reads the caller's membership // (the org check and the admin gate) and nothing else. -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed( @@ -201,7 +194,6 @@ const providerWith = (accountId: string) => { stubUsers, stubMirror, stubDirectory, - stubReadiness, stubApiKeys, stubAutumn, Layer.succeed(AccountCaller)({ session: session(accountId) }), diff --git a/apps/cloud/src/account/workos-account-service.ts b/apps/cloud/src/account/workos-account-service.ts index e7abd95aa..94f5aed51 100644 --- a/apps/cloud/src/account/workos-account-service.ts +++ b/apps/cloud/src/account/workos-account-service.ts @@ -11,7 +11,6 @@ import { import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; import type { Session } from "../auth/middleware"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOSClient } from "../auth/workos"; import { ensureOrganizationBackfilled, mirrorInvitedMember } from "../auth/mirror-feeders"; import { WorkOsMirror, mirrorMembershipFromWorkOs } from "../auth/workos-mirror"; @@ -72,7 +71,6 @@ export const workosAccountProvider: Layer.Layer< | UserStoreService | WorkOsMirror | MemberDirectory - | MirrorReadiness | ApiKeyService | AutumnService | AccountCaller @@ -103,12 +101,7 @@ export const workosAccountProvider: Layer.Layer< // erased to `R = never`, as the neutral AccountProvider shape requires. // Provided per method below. const ctx = yield* Effect.context< - | WorkOSClient - | UserStoreService - | AutumnService - | MemberDirectory - | MirrorReadiness - | WorkOsMirror + WorkOSClient | UserStoreService | AutumnService | MemberDirectory | WorkOsMirror >(); // Unauthenticated (missing/invalid session) => AccountUnauthorized, exactly diff --git a/apps/cloud/src/admin/admin-users-api.node.test.ts b/apps/cloud/src/admin/admin-users-api.node.test.ts index b29390be1..68f796bea 100644 --- a/apps/cloud/src/admin/admin-users-api.node.test.ts +++ b/apps/cloud/src/admin/admin-users-api.node.test.ts @@ -6,7 +6,6 @@ import { MemberDirectory, type DirectoryMember } from "@executor-js/api/server"; import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../auth/mirror-readiness"; import { ORG_SELECTOR_HEADER } from "../auth/organization"; import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "../auth/workos-mirror"; @@ -49,12 +48,6 @@ const memberships = new Map([ ["user_invited_admin", mirrored("user_invited_admin", { role: "admin", status: "pending" })], ]); -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed(organizationId === ORG ? (memberships.get(accountId) ?? null) : null), @@ -149,7 +142,11 @@ const stubWorkOS = (userId: string) => get: (_target, prop) => { if (prop === "authenticateRequest") { return () => - Effect.succeed({ userId, email: `${userId}@placeholder.test`, organizationId: null }); + Effect.succeed({ + userId, + email: `${userId}@placeholder.test`, + organizationId: null, + }); } return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); }, @@ -163,14 +160,7 @@ const authorizeAs = (userId: string) => }), ).pipe( Effect.provide( - Layer.mergeAll( - stubDirectory, - stubApiKeys, - stubUsers, - stubWorkOS(userId), - stubMirror, - stubReadiness, - ), + Layer.mergeAll(stubDirectory, stubApiKeys, stubUsers, stubWorkOS(userId), stubMirror), ), ); diff --git a/apps/cloud/src/admin/admin-users-api.ts b/apps/cloud/src/admin/admin-users-api.ts index a6ade66a0..0c77c03a9 100644 --- a/apps/cloud/src/admin/admin-users-api.ts +++ b/apps/cloud/src/admin/admin-users-api.ts @@ -56,7 +56,6 @@ import type { Executor } from "@executor-js/sdk"; import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { isPlatformAuth, resolveBearerAuth } from "../auth/workos-auth-provider"; import { orgSelectorFromRequest, authorizeOrganizationSelector } from "../auth/organization"; @@ -77,7 +76,7 @@ export const authorizeTenant = ( ): Effect.Effect< string, AdminUsersUnauthorized | AdminUsersForbidden, - WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror > => Effect.gen(function* () { // (1) The bearer path. `resolveBearerAuth` (not `resolveApiKeyPrincipal`, @@ -138,7 +137,6 @@ const withPlatformView = ()( +const AdminUsersProviderMiddleware = HttpRouter.middleware<{ + provides: AdminUsersProvider; +}>()( Effect.gen(function* () { const longLived = yield* Effect.context(); return (httpEffect) => @@ -265,9 +263,7 @@ const AdminUsersProviderMiddleware = HttpRouter.middleware<{ provides: AdminUser * `/api` prefix as the rest of the cloud router. */ export const makeCloudAdminUsersRoutes = ( - rsLive: Layer.Layer< - DbService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror - >, + rsLive: Layer.Layer, options: Parameters[1] = {}, ) => makeAdminUsersApiLayer( diff --git a/apps/cloud/src/api/layers.ts b/apps/cloud/src/api/layers.ts index c69342599..c64452e6d 100644 --- a/apps/cloud/src/api/layers.ts +++ b/apps/cloud/src/api/layers.ts @@ -11,7 +11,6 @@ import { import { SessionAuthLive } from "../auth/middleware-live"; import { UserStoreService } from "../auth/context"; import { cloudMemberDirectoryLayer } from "../auth/member-directory"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { CloudAuthPublicHandlers, @@ -36,23 +35,14 @@ const WorkOsMirrorLive = WorkOsMirror.Live.pipe(Layer.provide(DbLive)); // The shared `MemberDirectory` read seam over the membership mirror — the // same per-request socket the mirror writes through. const MemberDirectoryLive = cloudMemberDirectoryLayer.pipe(Layer.provide(DbLive)); -// Whether the mirror may authorize this request at all (backfill complete, -// reconciler caught up) — read on the same socket before the membership row. -const MirrorReadinessLive = MirrorReadiness.Live.pipe(Layer.provide(DbLive)); // Per-request layer. Anything that opens an I/O object (postgres.js socket, // fetch stream readers, anything backed by a `Writable`) MUST live here — // `provideRequestScoped` rebuilds it per request so Cloudflare Workers' // I/O isolation is satisfied. See `api.request-scope.test.ts`. export const RequestScopedServicesLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness -> = Layer.mergeAll( - DbLive, - UserStoreLive, - WorkOsMirrorLive, - MemberDirectoryLive, - MirrorReadinessLive, -); + DbService | UserStoreService | WorkOsMirror | MemberDirectory +> = Layer.mergeAll(DbLive, UserStoreLive, WorkOsMirrorLive, MemberDirectoryLive); // Boot-scoped layer. Built once at worker boot, reused across requests. // Safe for config, in-memory caches, the global tracer provider, and @@ -77,9 +67,7 @@ export const BootSharedServices = Layer.mergeAll( // handler reads it for the free-organizations-per-user limit gate — one of the // few app-only billing touchpoints. (It is NOT on the neutral boot core.) export const makeNonProtectedApiLive = ( - rsLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness - >, + rsLive: Layer.Layer, ) => HttpApiBuilder.layer(NonProtectedApi).pipe( Layer.provide(Layer.mergeAll(CloudAuthPublicHandlers, CloudSessionAuthHandlers)), @@ -95,9 +83,7 @@ export const makeNonProtectedApiLive = ( // gates on billing, so `AutumnService.Default` is provided here, not on the // neutral boot core. export const makeOrgApiLive = ( - rsLive: Layer.Layer< - DbService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror - >, + rsLive: Layer.Layer, ) => HttpApiBuilder.layer(OrgHttpApi).pipe( Layer.provide(OrgHandlers), @@ -143,7 +129,9 @@ export const OrgApiLive = makeOrgApiLive(RequestScopedServicesLive); // folded into `.layer` here; the rest of the router (`makeApiLive` in // `./router.ts`, `./protected.ts`, the test harness) re-provides the same // shared `RouterConfigLive` directly. -const protectedApi = makeProtectedApiLayer(cloudPlugins, { errorCapture: ErrorCaptureLive }); +const protectedApi = makeProtectedApiLayer(cloudPlugins, { + errorCapture: ErrorCaptureLive, +}); export const ProtectedCloudApi = protectedApi.api; export const ProtectedCloudApiHandlers = protectedApi.handlers; diff --git a/apps/cloud/src/api/protected-api-key-auth.node.test.ts b/apps/cloud/src/api/protected-api-key-auth.node.test.ts index 4aaf7cc1f..aac518b0f 100644 --- a/apps/cloud/src/api/protected-api-key-auth.node.test.ts +++ b/apps/cloud/src/api/protected-api-key-auth.node.test.ts @@ -5,7 +5,6 @@ import { MemberDirectory } from "@executor-js/api/server"; import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../auth/mirror-readiness"; import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "../auth/workos-mirror"; import { resolveProtectedPrincipal } from "./protected"; @@ -55,13 +54,8 @@ const stubWorkOS = Layer.succeed( ); // The mirror as the directory reads it: user_123 holds an active membership in -// org_123 and nothing else. Membership is never read from WorkOS. -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - +// org_123 and nothing else. Membership is always read from the mirror, never +// from WorkOS. const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed( @@ -136,9 +130,7 @@ const stubMirror = Layer.succeed( const run = (request: Request) => resolveProtectedPrincipal(request).pipe( - Effect.provide( - Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror, stubReadiness), - ), + Effect.provide(Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror)), ); describe("protected API key auth", () => { diff --git a/apps/cloud/src/api/protected-jwt-auth.node.test.ts b/apps/cloud/src/api/protected-jwt-auth.node.test.ts index 02abb0b53..edf2a0796 100644 --- a/apps/cloud/src/api/protected-jwt-auth.node.test.ts +++ b/apps/cloud/src/api/protected-jwt-auth.node.test.ts @@ -6,7 +6,6 @@ import { MemberDirectory } from "@executor-js/api/server"; import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../auth/mirror-readiness"; import type { JwtBearerConfig } from "../auth/workos-auth-provider"; import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "../auth/workos-mirror"; @@ -72,13 +71,8 @@ const stubWorkOS = Layer.succeed( ); // The mirror as the directory reads it: user_123 holds an active membership in -// org_123 and nothing else. Membership is never read from WorkOS. -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - +// org_123 and nothing else. Membership is always read from the mirror, never +// from WorkOS. const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed( @@ -153,9 +147,7 @@ const stubMirror = Layer.succeed( const run = (request: Request, jwt: JwtBearerConfig) => resolveProtectedPrincipal(request, jwt).pipe( - Effect.provide( - Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror, stubReadiness), - ), + Effect.provide(Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror)), ); const request = (token: string) => diff --git a/apps/cloud/src/api/protected.ts b/apps/cloud/src/api/protected.ts index a9308025e..ac97dd585 100644 --- a/apps/cloud/src/api/protected.ts +++ b/apps/cloud/src/api/protected.ts @@ -16,7 +16,6 @@ import { import { cloudPlugins, type CloudPlugins } from "../plugins"; import { ApiKeyService } from "../auth/api-keys"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { cloudIdentityFailureStrategy, workosIdentityLayer } from "../auth/workos-auth-provider"; import { AutumnService } from "../extensions/billing/service"; @@ -97,9 +96,7 @@ const ExecutionStackMiddleware = makeExecutionStackMiddleware< // account seat-gate, and the createOrganization free-limit gate each provide it // where they run.) export const makeProtectedApiLive = ( - rsLive: Layer.Layer< - DbService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror - >, + rsLive: Layer.Layer, ) => { // The neutral `IdentityProvider`, built per request: it reads `UserStoreService` // + `MemberDirectory` from `rsLive` and the WorkOS control plane (`WorkOSClient` + `ApiKeyService`, diff --git a/apps/cloud/src/api/router.ts b/apps/cloud/src/api/router.ts index 7bb7da247..8c80825ef 100644 --- a/apps/cloud/src/api/router.ts +++ b/apps/cloud/src/api/router.ts @@ -8,7 +8,6 @@ import { } from "@executor-js/api/server"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { DbService } from "../db/db"; import { makeAccountApiLive } from "../account/account-api"; @@ -36,9 +35,7 @@ import { makeProtectedApiLive } from "./protected"; // assert per-request semantics — see // `apps/cloud/src/api.request-scope.node.test.ts`. export const makeApiLive = ( - requestScopedLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness - >, + requestScopedLive: Layer.Layer, ) => { const BillingRoutesLive = AutumnRoutesLive.pipe( Layer.provide(requestScopedMiddleware(requestScopedLive).layer), diff --git a/apps/cloud/src/auth/doc-gate.ts b/apps/cloud/src/auth/doc-gate.ts index cc33730c4..7fcd3b074 100644 --- a/apps/cloud/src/auth/doc-gate.ts +++ b/apps/cloud/src/auth/doc-gate.ts @@ -42,7 +42,6 @@ import { makeUserStoreLayer, UserStoreService } from "./context"; import { parseCookie } from "./cookies"; import { LAST_ORG_COOKIE } from "./last-org-cookie"; import { makeMemberDirectoryLayer } from "./member-directory"; -import { makeMirrorReadinessLayer } from "./mirror-readiness"; import { makeWorkOsMirrorLayer } from "./workos-mirror"; import { sealedSessionDisplayName } from "./middleware"; import { authorizeOrganizationSelector } from "./organization"; @@ -184,7 +183,6 @@ const authorizeLastOrgSlug = async ( Layer.mergeAll( makeUserStoreLayer(), makeMemberDirectoryLayer(), - makeMirrorReadinessLayer(), makeWorkOsMirrorLayer(), ).pipe(Layer.provide(dbLive)), ), @@ -258,7 +256,9 @@ export const authGateMiddleware = createMiddleware({ type: "request" }).server( // the client AuthGate makes mid-session, made here before the document // exists so the app shell is never painted for an org-less session. if (!session.organizationId && !ONBOARDING_PATHS.has(pathname)) { - return redirect("/create-org", { refreshedSession: session.refreshedSession }); + return redirect("/create-org", { + refreshedSession: session.refreshedSession, + }); } // A BARE console path (no org slug in the URL) canonicalizes onto the org diff --git a/apps/cloud/src/auth/mirror-feeders.node.test.ts b/apps/cloud/src/auth/mirror-feeders.node.test.ts index 5834d156a..b1cb9ba1d 100644 --- a/apps/cloud/src/auth/mirror-feeders.node.test.ts +++ b/apps/cloud/src/auth/mirror-feeders.node.test.ts @@ -87,7 +87,6 @@ import { encodeLoginState } from "./login-state"; import { cloudMemberDirectoryLayer } from "./member-directory"; import { SessionAuthLive } from "./middleware-live"; import { mirrorSignIn } from "./mirror-feeders"; -import { MirrorReadiness, MirrorReadinessState } from "./mirror-readiness"; import { ORG_SELECTOR_HEADER, authorizeOrganization, @@ -172,14 +171,6 @@ const seedOrganization = (id: string) => ), ); -// The mirror is READY throughout (backfill complete, reconciler caught up): -// every membership read below is against the mirror, never WorkOS. The -// readiness rule itself is pinned in workos-mirror.node.test.ts and the -// fallback in org-selector-auth.node.test.ts. -const readyMirror = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - const stubAutumn = Layer.succeed(AutumnService)({ use: () => Effect.die("feeders do not read billing"), ensureCustomer: () => Effect.void, @@ -500,7 +491,12 @@ describe("a delayed sign-in feeder", () => { const rejoinedAt = "2026-01-03T00:00:00.000Z"; yield* mirrorSignIn( workosUser(userId), - [workosMembership(userId, org, { id: `om_${userId}_${org}_2`, updatedAt: rejoinedAt })], + [ + workosMembership(userId, org, { + id: `om_${userId}_${org}_2`, + updatedAt: rejoinedAt, + }), + ], new Date(rejoinedAt), ); return { membership, rejoined: yield* readMembership(userId, org) }; @@ -581,20 +577,14 @@ describe("session handlers read membership from the mirror", () => { DbService | UserStoreService | WorkOsMirror | MemberDirectory >; readonly autumn?: Layer.Layer; - /** The mirror's readiness for this request; ready unless a test says otherwise. */ - readonly readiness?: Layer.Layer; } = {}, ) => HttpRouter.toWebHandler( HttpApiBuilder.layer(NonProtectedApi).pipe( Layer.provide(Layer.mergeAll(CloudAuthPublicHandlers, CloudSessionAuthHandlers)), Layer.provide( - requestScopedMiddleware( - Layer.mergeAll( - options.services ?? RequestScopedServicesLive, - options.readiness ?? readyMirror, - ), - ).layer, + requestScopedMiddleware(Layer.mergeAll(options.services ?? RequestScopedServicesLive)) + .layer, ), Layer.provideMerge(SessionAuthLive), Layer.provideMerge(options.autumn ?? stubAutumn), @@ -627,9 +617,10 @@ describe("session handlers read membership from the mirror", () => { ); /** - * `authorizeOrganization` over the live stores and a READY mirror, as every - * protected request runs it; `workos` serves whatever the check may read - * from WorkOS (nothing, by default: any read dies). + * `authorizeOrganization` over the live stores, as every protected request + * runs it: membership is read from the mirror unconditionally; `workos` + * serves whatever the check may read from WorkOS (nothing, by default: any + * read dies). */ const authorize = ( userId: string, @@ -639,12 +630,9 @@ describe("session handlers read membership from the mirror", () => { Effect.runPromise( authorizeOrganization(userId, org).pipe( Effect.provide( - Layer.mergeAll( - UserStoreService.Live, - WorkOsMirror.Live, - cloudMemberDirectoryLayer, - readyMirror, - ).pipe(Layer.provideMerge(DbService.Live)), + Layer.mergeAll(UserStoreService.Live, WorkOsMirror.Live, cloudMemberDirectoryLayer).pipe( + Layer.provideMerge(DbService.Live), + ), ), Effect.provide(workos), Effect.scoped, @@ -665,7 +653,12 @@ describe("session handlers read membership from the mirror", () => { purges.push(op); }).pipe( Effect.flatMap(() => - Effect.fail(new UserStoreError({ operation: op, reason: "connection_closed" })), + Effect.fail( + new UserStoreError({ + operation: op, + reason: "connection_closed", + }), + ), ), ) : live.use(op, fn), @@ -914,7 +907,7 @@ describe("session handlers read membership from the mirror", () => { expect(await authorized(admin, org)).toBe(false); }); - it("finishes on a retry while the mirror is not ready, after WorkOS already deleted the org", async () => { + it("finishes on a retry after WorkOS already deleted the org", async () => { const admin = freshId("user"); const member = freshId("user"); const org = freshId("org"); @@ -931,19 +924,17 @@ describe("session handlers read membership from the mirror", () => { expect(first.status).toBe(500); expect(purges).toEqual(["deleteOrganizationCascade"]); - // The reconciler stalls before the retry. WorkOS no longer lists the org - // or the admin's membership in it — and the fallback must not ask it: - // the stub dies on `listUserMemberships`. The admin's own mirror row, - // which the failed purge left behind, is what admits the retry. + // WorkOS no longer has the org to delete a second time. The admin's own + // mirror row, which the failed purge left behind, is what admits the + // retry — membership is read from the mirror unconditionally. const retry = sessionHandler(admin, { autumn: deletingAutumn, - readiness: Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.ReconcilerStale({ drainedAt: null })), - }), - workos: { deleteOrganization: () => Effect.fail(new WorkOSError({ status: 404 })) }, + workos: { + deleteOrganization: () => Effect.fail(new WorkOSError({ status: 404 })), + }, }); const second = await retry(deleteOrganizationRequest(org)); - expect(second.status, "the retry is admitted from the mirror, not WorkOS").toBe(200); + expect(second.status, "the retry is admitted from the mirror").toBe(200); expect(await second.json()).toEqual({ success: true }); expect(await readMembers(org), "and the purge ran").toEqual([]); expect((await readOrganization(org))?.deletedAt).not.toBeNull(); @@ -961,7 +952,9 @@ describe("session handlers read membership from the mirror", () => { calls.push(`getUserOrgMembership:${userId}`); return Effect.succeed( userId === memberId - ? (workosMembership(userId, organizationId, { role: { slug: "admin" } }) as never) + ? (workosMembership(userId, organizationId, { + role: { slug: "admin" }, + }) as never) : null, ); }, @@ -1089,7 +1082,9 @@ describe("session handlers read membership from the mirror", () => { ); expect(response.status).toBe(200); - const body = (await response.json()) as { organizations: { id: string; slug: string }[] }; + const body = (await response.json()) as { + organizations: { id: string; slug: string }[]; + }; expect(body.organizations.map((o) => [o.id, o.slug])).toEqual([[live, liveSlug]]); }); }); @@ -1156,7 +1151,6 @@ describe("account service writes through to the mirror", () => { UserStoreService.Live, WorkOsMirror.Live, cloudMemberDirectoryLayer, - readyMirror, ); return workosAccountProvider.pipe( Layer.provide( @@ -1778,7 +1772,17 @@ describe("backfill", () => { ); const counts = await runBackfill( - new Map([[org, [workosMembership(paused, org, { status: "inactive", updatedAt: T2 })]]]), + new Map([ + [ + org, + [ + workosMembership(paused, org, { + status: "inactive", + updatedAt: T2, + }), + ], + ], + ]), false, ); expect(counts, "the deactivated membership is written, not tombstoned").toMatchObject({ @@ -1904,7 +1908,10 @@ describe("backfill", () => { [orgB, [workosMembership(staying, orgB)]], ]); const retried = await runBackfill(attemptB, false); - expect(retried).toMatchObject({ organizations: 2, membershipsTombstoned: 1 }); + expect(retried).toMatchObject({ + organizations: 2, + membershipsTombstoned: 1, + }); expect( await syncState(), "the retry keeps the first attempt's boundary instead of taking a later one", diff --git a/apps/cloud/src/auth/mirror-readiness-store.ts b/apps/cloud/src/auth/mirror-readiness-store.ts index c2e65524c..ccab8939a 100644 --- a/apps/cloud/src/auth/mirror-readiness-store.ts +++ b/apps/cloud/src/auth/mirror-readiness-store.ts @@ -1,33 +1,30 @@ // --------------------------------------------------------------------------- -// Mirror READINESS: whether the local membership mirror may be trusted as -// the membership authority for a request, or WorkOS must still be asked. +// Mirror READINESS: whether the local membership mirror has ever been fit to +// authorize from, per the ORIGINAL cutover rule. The request path +// (`organization.ts`) no longer consults this — it authorizes from the +// mirror unconditionally, because the one-off backfill is complete and +// permanent and a pre-mirror organization is covered by the on-demand scan +// (`ensureOrganizationBackfilled`). What remains is the deploy gate +// (`scripts/ensure-workos-mirror-ready.ts`), which still refuses to ship a +// build that trusts the mirror while it is unready, and the reconciler's own +// staleness alert (`workos-events-runner.ts`), which reads `drainedAt` after +// each run and raises a Sentry error when the drain has fallen behind the lag +// budget below — a stalled reconciler is now an operational page, not a +// per-request fallback. // -// The mirror is fed by login, write-through, and the Events API reconciler -// (`workos-mirror.ts`), and is complete only once the one-off backfill has -// written every organization and the reconciler has caught up to the -// present. Before that, two things go wrong if it is trusted anyway: -// - a member who has not signed in since the mirror shipped has no row -// yet, and every protected request of theirs is refused — the backfill -// is what writes them; -// - a member revoked in the WorkOS dashboard while the reconciler was not -// running still holds an active row, and keeps their access until the -// stream is replayed — the reconciler is what tombstones them. -// So readiness is BOTH: the backfill's completion mark +// Readiness is BOTH: the backfill's completion mark // (`workos_sync.backfill_completed_at`, written once by a run that covered // every live organization) AND a recent drain of the events stream // (`workos_sync.drained_at`, moved forward by every reconciler run that read // the stream to its end). The lag budget bounds how far behind the reconciler // may be: it runs every minute, so a mark older than the budget means it has // stalled (WorkOS unreachable, the cron not deployed, a backlog draining over -// many runs) and the mirror may be missing revocations. While either half is -// missing the authorization path reads membership from WorkOS instead -// (`organization.ts`), exactly as it did before the cutover; nothing is -// denied or granted on the mirror's word. +// many runs) and the mirror may be missing revocations. // // The rule and the row read live here, free of `cloudflare:workers`, so the -// deploy gate (`scripts/ensure-workos-mirror-ready.ts`) applies the SAME rule -// over a plain postgres.js connection under bun before the build that trusts -// the mirror goes live. The request-scoped service is `mirror-readiness.ts`. +// deploy gate applies the SAME rule over a plain postgres.js connection under +// bun before a build goes live, and the reconciler's alert applies the SAME +// `drainedAt` age check the gate does. // --------------------------------------------------------------------------- import { eq } from "drizzle-orm"; diff --git a/apps/cloud/src/auth/mirror-readiness.ts b/apps/cloud/src/auth/mirror-readiness.ts deleted file mode 100644 index 1bf8ac05d..000000000 --- a/apps/cloud/src/auth/mirror-readiness.ts +++ /dev/null @@ -1,69 +0,0 @@ -// --------------------------------------------------------------------------- -// MirrorReadiness — the request-scoped service that answers whether the -// membership mirror may authorize this request (see -// `mirror-readiness-store.ts` for the rule and why it exists). -// -// Per-request layer shape, like `UserStoreService` and `WorkOsMirror`: it -// reads the request's postgres socket, so it is rebuilt per request -// (`RequestScopedServicesLive`) and never shared across Workers requests. One -// indexed point read per authorization, on the same socket the membership -// read uses next. -// --------------------------------------------------------------------------- - -import { Clock, Context, Effect, Layer } from "effect"; - -import { DbService, type DrizzleDb } from "../db/db"; -import { - WorkOsMirrorError, - tryPromiseService, - userStoreReasonFromCause, - withServiceLogging, -} from "./errors"; -import { readMirrorReadiness, type MirrorReadinessState } from "./mirror-readiness-store"; - -export { - MIRROR_RECONCILER_LAG_BUDGET, - MirrorReadinessState, - describeMirrorReadiness, - mirrorReadinessFrom, - type MirrorReadinessRow, -} from "./mirror-readiness-store"; - -export interface MirrorReadinessShape { - /** - * The mirror's readiness as of now. Fails with `WorkOsMirrorError` when the - * row cannot be read — the caller must not treat that as either ready or - * not; it is the same infra failure as any other mirror read. - */ - readonly state: () => Effect.Effect; -} - -const makeService = (db: DrizzleDb): MirrorReadinessShape => ({ - state: () => - Effect.flatMap(Clock.currentTimeMillis, (millis) => - withServiceLogging( - "workos_mirror.readiness", - (failure) => - new WorkOsMirrorError({ - operation: "readiness", - reason: userStoreReasonFromCause(failure), - }), - tryPromiseService(() => readMirrorReadiness(db, new Date(millis))), - ), - ), -}); - -export class MirrorReadiness extends Context.Service()( - "@executor-js/cloud/MirrorReadiness", -) { - static Live = Layer.effect(this)(Effect.map(DbService.asEffect(), ({ db }) => makeService(db))); -} - -/** - * A FRESH `MirrorReadiness` layer (new layer value per call), for a service - * built once but invoked across many Workers requests — the MCP - * org-authorization seam and the document gate — for the same reason - * `makeUserStoreLayer` exists. See [[makeDbLayer]]. - */ -export const makeMirrorReadinessLayer = (): Layer.Layer => - Layer.effect(MirrorReadiness)(Effect.map(DbService.asEffect(), ({ db }) => makeService(db))); diff --git a/apps/cloud/src/auth/org-api-key-auth.node.test.ts b/apps/cloud/src/auth/org-api-key-auth.node.test.ts index 5358e7599..5b19386a2 100644 --- a/apps/cloud/src/auth/org-api-key-auth.node.test.ts +++ b/apps/cloud/src/auth/org-api-key-auth.node.test.ts @@ -5,7 +5,6 @@ import { MemberDirectory, NoOrganization } from "@executor-js/api/server"; import { ApiKeyService } from "./api-keys"; import { UserStoreService } from "./context"; -import { MirrorReadiness, MirrorReadinessState } from "./mirror-readiness"; import { WorkOSClient, type WorkOSClientService } from "./workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "./workos-mirror"; import { isPlatformAuth, resolveApiKeyPrincipal, resolveBearerAuth } from "./workos-auth-provider"; @@ -70,13 +69,8 @@ const stubWorkOS = Layer.succeed( ); // The mirror as the directory reads it: user_123 holds an active membership in -// org_123 and nothing else. Membership is never read from WorkOS. -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - +// org_123 and nothing else. Membership is always read from the mirror, never +// from WorkOS. const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed( @@ -149,14 +143,7 @@ const stubMirror = Layer.succeed( }), ); -const layers = Layer.mergeAll( - stubApiKeys, - stubWorkOS, - stubUsers, - stubDirectory, - stubMirror, - stubReadiness, -); +const layers = Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror); const bearer = (token: string) => new Request("https://executor.test/api/tools", { @@ -224,14 +211,7 @@ describe("org-level API keys", () => { const exit = yield* Effect.exit( resolveBearerAuth(bearer("valid_org_key")).pipe( Effect.provide( - Layer.mergeAll( - stubApiKeys, - stubWorkOS, - deletedOrgUsers, - stubDirectory, - stubMirror, - stubReadiness, - ), + Layer.mergeAll(stubApiKeys, stubWorkOS, deletedOrgUsers, stubDirectory, stubMirror), ), ), ); @@ -292,14 +272,7 @@ describe("org-level API keys", () => { }); const auth = yield* resolveBearerAuth(bearer("valid_org_key")).pipe( Effect.provide( - Layer.mergeAll( - stubApiKeys, - stubWorkOS, - stubUsers, - noMembershipReads, - stubMirror, - stubReadiness, - ), + Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, noMembershipReads, stubMirror), ), ); diff --git a/apps/cloud/src/auth/org-selector-auth.node.test.ts b/apps/cloud/src/auth/org-selector-auth.node.test.ts index 9cdbd0950..f972a2e75 100644 --- a/apps/cloud/src/auth/org-selector-auth.node.test.ts +++ b/apps/cloud/src/auth/org-selector-auth.node.test.ts @@ -6,7 +6,6 @@ import { MemberDirectory, type DirectoryMember } from "@executor-js/api/server"; import { ApiKeyService } from "./api-keys"; import { UserStoreService } from "./context"; -import { MirrorReadiness, MirrorReadinessState } from "./mirror-readiness"; import { AUTHORIZE_ORGANIZATION_SPAN } from "./organization"; import { resolveSessionPrincipal } from "./workos-auth-provider"; import { WorkOSClient, type WorkOSClientService } from "./workos"; @@ -68,12 +67,6 @@ const memberships = new Map([ [PENDING_ORG, mirrored(PENDING_ORG, { status: "pending" })], ]); -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed(accountId === MEMBER ? (memberships.get(organizationId) ?? null) : null), @@ -167,85 +160,28 @@ const stubMirror = Layer.succeed( }), ); -const run = ( - headers: Record, - readiness: Layer.Layer = stubReadiness, -) => +const run = (headers: Record) => resolveSessionPrincipal(new Request("https://executor.test/api/tools", { headers })).pipe( - Effect.provide( - Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror, readiness), - ), + Effect.provide(Layer.mergeAll(stubApiKeys, stubWorkOS, stubUsers, stubDirectory, stubMirror)), ); -// The mirror before the cutover has landed: the backfill has not covered -// every organization, or the reconciler has not drained recently. -const unreadyMirror = (state: MirrorReadinessState) => - Layer.succeed(MirrorReadiness)({ state: () => Effect.succeed(state) }); - /** - * A WorkOS that answers the pre-cutover membership list for MEMBER — active - * in SESSION_ORG only, as a member — and records the calls, so the fallback - * is assertable: with the mirror unready the list is read from WorkOS and - * the directory (which says MEMBER is active in URL_ORG too) is never asked. - */ -const workosMemberships = (calls: string[]) => - Layer.succeed( - WorkOSClient, - new Proxy({} as WorkOSClientService, { - get: (_t, prop) => { - if (prop === "authenticateRequest") { - return () => - Effect.succeed({ - userId: MEMBER, - email: "u@e2e.test", - organizationId: SESSION_ORG, - }); - } - if (prop === "listUserMemberships") { - return (userId: string) => - Effect.sync(() => { - calls.push(`listUserMemberships:${userId}`); - return { - object: "list" as const, - data: [ - { - id: `om_${MEMBER}_${SESSION_ORG}`, - userId: MEMBER, - organizationId: SESSION_ORG, - status: "active", - role: { slug: "member" }, - }, - ] as never[], - listMetadata: { before: null, after: null }, - }; - }); - } - return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); - }, - }), - ); - -const unreadDirectory = Layer.succeed(MemberDirectory)({ - membership: () => Effect.die("an unready mirror must not be asked for membership"), - membershipById: () => Effect.die("an unready mirror must not be asked for membership"), - membershipsOf: () => Effect.die("an unready mirror must not be asked for membership"), - members: () => Effect.die("an unready mirror must not be asked for membership"), - membersById: () => Effect.die("an unready mirror must not be asked for membership"), - findByEmail: () => Effect.die("an unready mirror must not be asked for membership"), -}); - -/** - * A tracer that keeps every span's attributes, so the readiness decision the - * authorization stamps on its span (`mirror.ready`, `mirror.readiness`) is - * assertable: that attribute is what production counts the fallback by. + * A tracer that keeps every span's attributes, so the authorization span + * itself — {@link AUTHORIZE_ORGANIZATION_SPAN} — is assertable. */ const makeRecordingTracer = () => { - const spans: { readonly name: string; readonly attributes: Map }[] = []; + const spans: { + readonly name: string; + readonly attributes: Map; + }[] = []; const tracer: Tracer.Tracer = { span: (options) => { const attributes = new Map(); spans.push({ name: options.name, attributes }); - let status: Tracer.SpanStatus = { _tag: "Started", startTime: options.startTime }; + let status: Tracer.SpanStatus = { + _tag: "Started", + startTime: options.startTime, + }; return { _tag: "Span", name: options.name, @@ -261,7 +197,12 @@ const makeRecordingTracer = () => { sampled: options.sampled, kind: options.kind, end: (endTime, exit) => { - status = { _tag: "Ended", startTime: options.startTime, endTime, exit }; + status = { + _tag: "Ended", + startTime: options.startTime, + endTime, + exit, + }; }, attribute: (key, value) => { attributes.set(key, value); @@ -275,24 +216,6 @@ const makeRecordingTracer = () => { return { tracer, attributesOf }; }; -const runAgainstWorkOs = (headers: Record, state: MirrorReadinessState) => { - const calls: string[] = []; - return resolveSessionPrincipal(new Request("https://executor.test/api/tools", { headers })) - .pipe( - Effect.provide( - Layer.mergeAll( - stubApiKeys, - workosMemberships(calls), - stubUsers, - unreadDirectory, - stubMirror, - unreadyMirror(state), - ), - ), - ) - .pipe(Effect.map((principal) => ({ principal, calls }))); -}; - describe("resolveSessionPrincipal · URL org selector", () => { it.effect("fails closed when no selector header is sent", () => Effect.gen(function* () { @@ -358,79 +281,21 @@ describe("resolveSessionPrincipal · URL org selector", () => { ); }); -// The mirror authorizes only while it is READY (`mirror-readiness.ts`): -// the backfill has written every organization and the reconciler has drained -// the stream within its lag budget. Until then the membership check reads -// WorkOS, as it did before the cutover — so a member the backfill has not -// written yet is not locked out, and a stale mirror row cannot grant access -// WorkOS has revoked. -describe("resolveSessionPrincipal · mirror readiness", () => { - const unready: readonly [string, MirrorReadinessState][] = [ - ["the backfill has not completed", MirrorReadinessState.BackfillPending()], - ["the reconciler has never drained", MirrorReadinessState.ReconcilerStale({ drainedAt: null })], - [ - "the reconciler's last drain is older than the budget", - MirrorReadinessState.ReconcilerStale({ drainedAt: createdAt }), - ], - ]; - - for (const [why, state] of unready) { - it.effect(`reads membership from WorkOS, not the mirror, while ${why}`, () => - Effect.gen(function* () { - // WorkOS says MEMBER is active in SESSION_ORG: authorized there... - const granted = yield* runAgainstWorkOs( - { cookie: "wos-session=x", "x-executor-organization": SESSION_ORG }, - state, - ); - expect(granted.principal.organizationId).toBe(SESSION_ORG); - expect(granted.principal.orgRole, "the role comes from WorkOS's list too").toBe("member"); - expect(granted.calls).toEqual([`listUserMemberships:${MEMBER}`]); - - // ...and NOT in URL_ORG, even though the (unready) mirror holds an - // active admin membership there: the mirror's word is not taken. - const refused = yield* Effect.flip( - runAgainstWorkOs({ cookie: "wos-session=x", "x-executor-organization": URL_SLUG }, state), - ); - expect(refused).toMatchObject({ _tag: "NoOrganization" }); - }), - ); - } - - it.effect("reads the mirror once it is ready, without any WorkOS membership call", () => +// Membership is read from the local mirror unconditionally — there is no +// readiness gate and no per-request WorkOS fallback (`auth/organization.ts`). +// The span every authorization runs under is still pinned here; a stalled +// reconciler is now an operational alert (`workos-events-runner.ts`), not a +// request-path branch, so it has no span attribute left to assert on. +describe("resolveSessionPrincipal · authorization span", () => { + it.effect("runs the authorization under its span", () => Effect.gen(function* () { - // `stubWorkOS` dies on any call past session authentication, so a - // resolved principal here proves the list was never requested. - const principal = yield* run( - { cookie: "wos-session=x", "x-executor-organization": URL_SLUG }, - stubReadiness, - ); + const recorder = makeRecordingTracer(); + const principal = yield* run({ + cookie: "wos-session=x", + "x-executor-organization": URL_SLUG, + }).pipe(Effect.withTracer(recorder.tracer)); expect(principal.organizationId).toBe(URL_ORG); - expect(principal.orgRole).toBe("admin"); - }), - ); - - it.effect("stamps which source answered on the authorization span", () => - Effect.gen(function* () { - // The fallback is counted in production by this attribute, not by - // grepping the warning it logs beside it — so both branches must set it. - const ready = makeRecordingTracer(); - yield* run({ cookie: "wos-session=x", "x-executor-organization": URL_SLUG }).pipe( - Effect.withTracer(ready.tracer), - ); - const readyAttrs = ready.attributesOf(AUTHORIZE_ORGANIZATION_SPAN); - expect(readyAttrs?.get("mirror.ready"), "the mirror answered").toBe(true); - expect(readyAttrs?.get("mirror.readiness")).toBe("ready"); - - const stale = makeRecordingTracer(); - yield* runAgainstWorkOs( - { cookie: "wos-session=x", "x-executor-organization": SESSION_ORG }, - MirrorReadinessState.ReconcilerStale({ drainedAt: null }), - ).pipe(Effect.withTracer(stale.tracer)); - const staleAttrs = stale.attributesOf(AUTHORIZE_ORGANIZATION_SPAN); - expect(staleAttrs?.get("mirror.ready"), "WorkOS answered").toBe(false); - expect(String(staleAttrs?.get("mirror.readiness")), "and the span says why").toContain( - "reconciler", - ); + expect(recorder.attributesOf(AUTHORIZE_ORGANIZATION_SPAN)).toBeDefined(); }), ); }); diff --git a/apps/cloud/src/auth/organization.ts b/apps/cloud/src/auth/organization.ts index f1d558932..e46560c52 100644 --- a/apps/cloud/src/auth/organization.ts +++ b/apps/cloud/src/auth/organization.ts @@ -18,7 +18,6 @@ import { EXECUTOR_ORG_SELECTOR_HEADER } from "@executor-js/sdk/shared"; import { UserStoreService } from "./context"; import { ensureOrganizationBackfilled } from "./mirror-feeders"; -import { MirrorReadiness, MirrorReadinessState, describeMirrorReadiness } from "./mirror-readiness"; import type { Organization } from "./user-store"; import { WorkOSClient } from "./workos"; @@ -94,12 +93,11 @@ export const markOrganizationDeleted = (organizationId: string) => // until their access token naturally expired (~10 min) if the session were // trusted on its own. // -// To close that gap, membership is verified on every protected request — but +// To close that gap, membership is verified on every protected request // against the LOCAL mirror of WorkOS memberships (`memberships` join -// `accounts`, read through the shared `MemberDirectory`), never against WorkOS -// itself. This used to be one `listUserMemberships` call per request (2026-07: -// deliberately NOT cached, because a positive TTL cache is exactly what would -// re-open the revocation gap). The mirror is not a cache with a TTL; it is a +// `accounts`, read through the shared `MemberDirectory`) — never against +// WorkOS itself, and unconditionally: there is no readiness gate and no +// per-request WorkOS fallback. The mirror is not a cache with a TTL; it is a // replica whose freshness is defined by its feeders: // - login (`auth/handlers.ts` callback): the user and every membership WorkOS // lists for them, from the list the callback already fetches; @@ -117,45 +115,39 @@ export const markOrganizationDeleted = (organizationId: string) => // not finish refuses every session at once — its membership rows are still // there, live, until the purge removes them, and must not authorize anyone. // -// The mirror is trusted only while it is READY (`mirror-readiness.ts`): the -// one-off backfill has written every organization, and the events reconciler -// has drained the stream within its lag budget. Until both hold, membership is -// read from WorkOS (`listUserMemberships`, one call per request) exactly as -// before the cutover — a member the backfill has not written yet must not be -// locked out, and a member revoked in the dashboard while the reconciler was -// down must not be let in on a stale row. The readiness row is one indexed -// point read on the same socket; the deploy gate -// (`scripts/ensure-workos-mirror-ready.ts`) applies the same rule before this -// build goes live, so in steady state the fallback is never taken. A -// readiness or mirror read failure fails the request (500), never a silent -// fallback in either direction. The one org the fallback never asks WorkOS -// about is one the mirror holds as DELETED: WorkOS no longer has it (or is -// about to not), so its answer is "no member" for everyone — including the -// admin whose deletion failed part-way and must retry it (below). That -// membership is read from the mirror, whose rows are exactly what the purge -// has not removed yet, ready or not; a refused caller gets null either way. +// The one-off backfill is complete and permanent, and an organization that +// predates it is covered on demand (below), so there is nothing left for a +// per-request readiness check to gate. What can still go wrong is the events +// reconciler falling behind — a member revoked in the WorkOS dashboard would +// keep a stale active row until it catches up. That is now an OPERATIONAL +// concern, not a request-path fallback: the reconciler itself +// (`workos-events-runner.ts`) checks its own drain lag after every run and +// raises a Sentry error when it has stalled, so it is fixed by paging someone, +// not by asking WorkOS on every request. The deploy gate +// (`scripts/ensure-workos-mirror-ready.ts`) separately refuses to ship a build +// that trusts the mirror while it is unready, using the same rule +// (`mirror-readiness-store.ts`). // -// Readiness is database-wide; completeness is PER ORGANIZATION. An -// organization whose row was minted after the backfill ran — lazily by a -// request (`resolveOrganization`), or by a first login — carries no -// `backfilled_at`, and the mirror holds only the memberships login and -// write-through happened to record for it: a member who has not signed in -// since would be refused on a row that was never written. So the org row is -// read FIRST, and an unmarked live organization is scanned from WorkOS -// (`ensureOrganizationBackfilled`: one membership listing plus one `getUser` -// per member, then the mark) BEFORE its mirror is read — the same on-demand -// scan the seat gates run. One-time per organization: the scan marks the -// row, and this branch is never taken for it again. An organization the -// mirror does not hold at all — one that predates the mirror and that nobody -// has signed in to since (a CLI or MCP token names it, and the JWT path has -// no login feeder), or one created in the WorkOS dashboard — is reachable by -// neither the backfill (which lists the mirror's organizations) nor the -// reconciler (which starts at the replay boundary), so it is resolved on -// demand HERE: WorkOS is asked for the caller's own membership in it first -// (`getUserOrgMembership`, a read scoped to this caller — never a listing -// of the org), and only a member's answer mints the row -// (`resolveOrganization`) and scans it as above. A non-member mints nothing: -// a signed-in caller cannot create the row of an arbitrary WorkOS +// Completeness is PER ORGANIZATION. An organization whose row was minted +// after the backfill ran — lazily by a request (`resolveOrganization`), or by +// a first login — carries no `backfilled_at`, and the mirror holds only the +// memberships login and write-through happened to record for it: a member +// who has not signed in since would be refused on a row that was never +// written. So the org row is read FIRST, and an unmarked live organization is +// scanned from WorkOS (`ensureOrganizationBackfilled`: one membership listing +// plus one `getUser` per member, then the mark) BEFORE its mirror is read — +// the same on-demand scan the seat gates run. One-time per organization: the +// scan marks the row, and this branch is never taken for it again. An +// organization the mirror does not hold at all — one that predates the +// mirror and that nobody has signed in to since (a CLI or MCP token names it, +// and the JWT path has no login feeder), or one created in the WorkOS +// dashboard — is reachable by neither the backfill (which lists the mirror's +// organizations) nor the reconciler (which starts at the replay boundary), so +// it is resolved on demand HERE: WorkOS is asked for the caller's own +// membership in it first (`getUserOrgMembership`, a read scoped to this +// caller — never a listing of the org), and only a member's answer mints the +// row (`resolveOrganization`) and scans it as above. A non-member mints +// nothing: a signed-in caller cannot create the row of an arbitrary WorkOS // organization by naming its id. An organization marked deleted is never // scanned: WorkOS no longer has it, and its rows are the purge's to remove, // not a listing's to refresh. @@ -189,20 +181,6 @@ const activeMembershipFromMirror = (userId: string, organizationId: string) => return active; }); -// The pre-cutover read, kept for the window in which the mirror is not yet -// ready: WorkOS's own membership list for the user, one call per request. -const activeMembershipFromWorkOs = (userId: string, organizationId: string) => - Effect.gen(function* () { - const workos = yield* WorkOSClient; - const memberships = yield* workos.listUserMemberships(userId); - const membership = memberships.data.find( - (m) => m.organizationId === organizationId && m.status === "active", - ); - if (!membership) return null; - const active: ActiveMembership = { role: membership.role.slug }; - return active; - }); - // The authorized organization, or null for one marked deleted (unless the // caller is the deletion flow). The membership already names the caller's // role — surfaced normalized so identity resolution can bind the executor's @@ -236,14 +214,7 @@ const heldOrResolvedForMember = (userId: string, organizationId: string) => return yield* resolveOrganization(organizationId); }); -/** - * The span every membership authorization runs under, with the readiness - * decision stamped on it so the fallback can be counted rather than grepped: - * `mirror.ready` (boolean, which source answered) and `mirror.readiness` (the - * state's description, why). Query Axiom for `name == "auth.authorize_organization"` - * and `mirror.ready == false` to see how many requests are on the WorkOS - * fallback and for which reason. - */ +/** The span every membership authorization runs under. */ export const AUTHORIZE_ORGANIZATION_SPAN = "auth.authorize_organization"; export const authorizeOrganization = ( @@ -252,43 +223,14 @@ export const authorizeOrganization = ( options: AuthorizeOrganizationOptions = {}, ) => Effect.gen(function* () { - const readiness = yield* MirrorReadiness; - const state = yield* readiness.state(); - const ready = MirrorReadinessState.$is("Ready")(state); - yield* Effect.annotateCurrentSpan({ - "mirror.ready": ready, - "mirror.readiness": describeMirrorReadiness(state), - }); - if (!ready) { - yield* Effect.logWarning( - "authorizeOrganization: membership mirror not ready; membership read from WorkOS", - { readiness: describeMirrorReadiness(state) }, - ); - // A marked organization is the mirror's to answer for (see above): - // WorkOS lists no member of it, and the deletion retry must still - // get in. - const users = yield* UserStoreService; - const held = yield* users.use("getOrganization", (s) => s.getOrganization(organizationId)); - if (held?.deletedAt != null) { - if (options.deleted !== "allow") return null; - const membership = yield* activeMembershipFromMirror(userId, organizationId); - if (!membership) return null; - return authorized(held, membership, options); - } - const membership = yield* activeMembershipFromWorkOs(userId, organizationId); - if (!membership) return null; - // The row read above is reused: `resolveOrganization` would read it a - // second time, and a request reads the organization row ONCE (the MCP - // session DO relies on that — see e2e `mcp-session-cold-init`). - const org = held ?? (yield* resolveOrganization(organizationId)); - return authorized(org, membership, options); - } - const org = yield* heldOrResolvedForMember(userId, organizationId); if (!org) return null; // An unmarked live organization is scanned before its mirror is read // (see above). The row returned below still shows the mark as it was - // read; nothing past this point reads it. + // read; nothing past this point reads it. A marked-deleted organization + // is never scanned, so the deletion retry (`deleted: "allow"`) reaches + // `authorized()` below with the membership row the purge has not removed + // yet. if (org.deletedAt === null && org.backfilledAt === null) { yield* ensureOrganizationBackfilled(organizationId); } diff --git a/apps/cloud/src/auth/workos-auth-provider.ts b/apps/cloud/src/auth/workos-auth-provider.ts index 25ce48a54..7165a9304 100644 --- a/apps/cloud/src/auth/workos-auth-provider.ts +++ b/apps/cloud/src/auth/workos-auth-provider.ts @@ -20,16 +20,16 @@ // - session org not authorized -> NoOrganization 403 no_organization // - no auth header -> falls through to the sealed-session path // The org-resolution infra errors (`UserStoreError` / `WorkOSError` / -// `MemberDirectoryError` / `WorkOsMirrorError`, the last from the mirror -// readiness read) are `Effect.die`d so they surface as 500 defects — the -// same status the old inline resolver produced when those bubbled up. +// `MemberDirectoryError` / `WorkOsMirrorError`) are `Effect.die`d so they +// surface as 500 defects — the same status the old inline resolver produced +// when those bubbled up. // -// The per-request `UserStoreService` + `MemberDirectory` + `MirrorReadiness` -// + `WorkOsMirror` (read by the org-resolution path: the org row, whether -// the mirror may be trusted, the caller's mirrored membership, and the -// on-demand scan of an organization the backfill never covered) stay -// REQUIREMENTS OF THE LAYER, satisfied by the facade's per-request DB combine — -// NOT function-level requirements (that is what forced a forked tag before). +// The per-request `UserStoreService` + `MemberDirectory` + `WorkOsMirror` +// (read by the org-resolution path: the org row, the caller's mirrored +// membership, and the on-demand scan of an organization the backfill never +// covered) stay REQUIREMENTS OF THE LAYER, satisfied by the facade's +// per-request DB combine — NOT function-level requirements (that is what +// forced a forked tag before). // --------------------------------------------------------------------------- import { Effect, Layer } from "effect"; @@ -54,7 +54,6 @@ import type { import { ApiKeyService } from "./api-keys"; import { workosApiJwtBearerConfig } from "./api-jwt-bearer"; -import { MirrorReadiness } from "./mirror-readiness"; import { WorkOsMirror } from "./workos-mirror"; import { BEARER_PREFIX } from "./bearer"; import { @@ -215,7 +214,7 @@ export const resolveBearerAuth = ( | WorkOSError | WorkOsMirrorError | MemberDirectoryError, - WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror > => Effect.gen(function* () { const authHeader = request.headers.get("authorization"); @@ -301,7 +300,7 @@ export const resolveApiKeyPrincipal = ( | WorkOSError | WorkOsMirrorError | MemberDirectoryError, - WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror > => Effect.gen(function* () { const auth = yield* resolveBearerAuth(request, jwt); @@ -383,7 +382,7 @@ export const resolveProtectedPrincipal = ( | WorkOSError | WorkOsMirrorError | MemberDirectoryError, - WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror > => Effect.gen(function* () { const bearerPrincipal = yield* resolveApiKeyPrincipal(request, jwt); @@ -404,17 +403,12 @@ export const resolveProtectedPrincipal = ( export const workosIdentityLayer: Layer.Layer< IdentityProvider, never, - WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror > = Layer.effect( IdentityProvider, Effect.gen(function* () { const context = yield* Effect.context< - | WorkOSClient - | ApiKeyService - | UserStoreService - | MemberDirectory - | MirrorReadiness - | WorkOsMirror + WorkOSClient | ApiKeyService | UserStoreService | MemberDirectory | WorkOsMirror >(); return IdentityProvider.of({ authenticate: (request) => @@ -430,7 +424,7 @@ export const workosIdentityLayer: Layer.Layer< WorkOSError: (error) => Effect.die(error), // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: membership-mirror read failure -> 500 defect, same class as the store failure above MemberDirectoryError: (error) => Effect.die(error), - // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: mirror-readiness read failure -> 500 defect, same class as the store failure above + // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: membership-mirror read failure -> 500 defect, same class as the store failure above WorkOsMirrorError: (error) => Effect.die(error), }), Effect.provide(context), diff --git a/apps/cloud/src/auth/workos-events-runner.node.test.ts b/apps/cloud/src/auth/workos-events-runner.node.test.ts new file mode 100644 index 000000000..66d53d334 --- /dev/null +++ b/apps/cloud/src/auth/workos-events-runner.node.test.ts @@ -0,0 +1,81 @@ +// --------------------------------------------------------------------------- +// A reconciler run that does not end `"drained"` can still leave the mirror +// fresh (another run drained it moments ago) or leave it stale (nothing has +// drained inside the lag budget). `alertOnStaleReconciler` is the ONLY place +// that distinction is now reported — the request path no longer reads +// readiness at all — so this pins both branches directly against it: a +// fresh `drainedAt` logs nothing, and a stale or absent `drainedAt` logs a +// structured error. `captureCauseEffect` is not swapped out: it calls +// `Sentry.captureException` directly, is a no-op in this uninitialized test +// environment, and its call path is exercised for real rather than mocked. +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import { Cause, Duration, Effect, Layer, Logger } from "effect"; + +import { WorkOsMirror, type WorkOsMirrorShape } from "./workos-mirror"; +import { alertOnStaleReconciler } from "./workos-events-runner"; +import type { WorkOsEventsSyncReport } from "./workos-events-sync"; + +const capturingLogger = (sink: Array) => + Logger.make((options) => { + sink.push(String(options.message)); + sink.push(Cause.pretty(options.cause)); + }); + +const reportEndingWith = (stopped: WorkOsEventsSyncReport["stopped"]): WorkOsEventsSyncReport => ({ + pages: 1, + events: 0, + applied: 0, + stale: 0, + absent: 0, + stopped, + cursor: null, +}); + +const stubMirror = (drainedAt: Date | null) => + Layer.succeed( + WorkOsMirror, + new Proxy({} as WorkOsMirrorShape, { + get: (_target, prop) => { + if (prop === "drainedAt") return () => Effect.succeed(drainedAt); + return () => Effect.die(`unexpected WorkOsMirror.${String(prop)} call`); + }, + }), + ); + +const run = (report: WorkOsEventsSyncReport, drainedAt: Date | null) => { + const logged: string[] = []; + return Effect.runPromise( + alertOnStaleReconciler(report).pipe( + Effect.provide(stubMirror(drainedAt)), + Effect.provide(Logger.layer([capturingLogger(logged)])), + ), + ).then(() => logged); +}; + +describe("alertOnStaleReconciler", () => { + it("does not alert when the run itself drained the stream", async () => { + // A `"drained"` run just called `markDrained`, so it is healthy by + // definition and `drainedAt` is never read. + const logged = await run(reportEndingWith("drained"), null); + expect(logged).toEqual([]); + }); + + it("does not alert when the mirror drained inside the lag budget", async () => { + const fresh = new Date(Date.now() - Duration.toMillis(Duration.minutes(1))); + const logged = await run(reportEndingWith("page_budget"), fresh); + expect(logged).toEqual([]); + }); + + it("alerts when the mirror has not drained inside the lag budget", async () => { + const stale = new Date(Date.now() - Duration.toMillis(Duration.minutes(11))); + const logged = await run(reportEndingWith("page_budget"), stale); + expect(logged.some((line) => line.includes("workos_events: reconciler stale"))).toBe(true); + }); + + it("alerts when the mirror has never drained", () => + run(reportEndingWith("awaiting_backfill"), null).then((logged) => { + expect(logged.some((line) => line.includes("workos_events: reconciler stale"))).toBe(true); + })); +}); diff --git a/apps/cloud/src/auth/workos-events-runner.ts b/apps/cloud/src/auth/workos-events-runner.ts index 1e1c860e7..e5979e48b 100644 --- a/apps/cloud/src/auth/workos-events-runner.ts +++ b/apps/cloud/src/auth/workos-events-runner.ts @@ -13,17 +13,28 @@ // A failing run is captured (Sentry + structured log) and swallowed here: // neither entry has a caller to report to, and the run is retried by the // next cron tick from the last committed cursor. +// +// A run that does not fail outright can still leave the mirror stale: one +// that stops short of `"drained"` (page budget, cursor contention) means the +// reconciler did not catch up this tick, and one stuck on +// `"awaiting_backfill"` past the budget means the boundary row the backfill +// was supposed to hand off was lost — both are read from `drainedAt` after +// the run and, past `MIRROR_RECONCILER_LAG_BUDGET`, reported the same way a +// thrown failure is: a structured error log plus a Sentry capture. This is +// the ONLY place a stale reconciler surfaces now; the request path +// (`auth/organization.ts`) no longer checks readiness or falls back. // --------------------------------------------------------------------------- -import { Effect, Layer } from "effect"; +import { Clock, Data, Duration, Effect, Layer } from "effect"; import { captureCauseEffect } from "../observability"; import { WorkerTelemetryLive } from "../observability/telemetry"; import { makeDbLayer } from "../db/db"; import { makeUserStoreLayer } from "./context"; +import { MIRROR_RECONCILER_LAG_BUDGET } from "./mirror-readiness-store"; import { CoreSharedServices } from "./workos"; -import { syncWorkOsEvents } from "./workos-events-sync"; -import { makeWorkOsMirrorLayer } from "./workos-mirror"; +import { syncWorkOsEvents, type WorkOsEventsSyncReport } from "./workos-events-sync"; +import { makeWorkOsMirrorLayer, WorkOsMirror } from "./workos-mirror"; const makeSyncServices = () => { const dbLive = makeDbLayer(); @@ -34,14 +45,59 @@ const makeSyncServices = () => { ); }; +const LAG_BUDGET_MS = Duration.toMillis(MIRROR_RECONCILER_LAG_BUDGET); + +/** + * The mirror has not drained the WorkOS events stream inside its lag budget. + * Its own tagged error so Sentry groups every occurrence under one issue, + * with the run's outcome and the heartbeat's age as the fields to read. + */ +export class WorkOsReconcilerStale extends Data.TaggedError("WorkOsReconcilerStale")<{ + readonly stopped: WorkOsEventsSyncReport["stopped"]; + readonly drainedAt: string | null; + readonly ageMs: number | null; +}> {} + +/** + * After a run that did not end `"drained"`, check whether the mirror has + * fallen behind its lag budget and, if so, report it the way a failed run is + * reported: a structured error log plus a Sentry capture. A run ending + * `"drained"` just wrote `markDrained` and is healthy by definition, so it is + * never checked. Cheap by design: one `drainedAt` read per run (about once a + * minute). + */ +export const alertOnStaleReconciler = Effect.fn("workos_events.alert_on_stale_reconciler")( + function* (report: WorkOsEventsSyncReport) { + if (report.stopped === "drained") return; + + const mirror = yield* WorkOsMirror; + const drainedAt = yield* mirror.drainedAt(); + const now = yield* Clock.currentTimeMillis; + const ageMs = drainedAt === null ? null : now - drainedAt.getTime(); + if (ageMs !== null && ageMs <= LAG_BUDGET_MS) return; + + const stale = new WorkOsReconcilerStale({ + stopped: report.stopped, + drainedAt: drainedAt === null ? null : drainedAt.toISOString(), + ageMs, + }); + yield* Effect.logError("workos_events: reconciler stale", stale); + yield* captureCauseEffect(stale); + }, +); + /** * One reconciler pass over fresh request-scoped services. Resolves when the * pass ends, whether it drained the stream, stopped at the page budget, * yielded to another run, or failed (a failure is reported, never thrown). + * A run that ends short of `"drained"` and leaves `drainedAt` past the lag + * budget is ALSO reported (see {@link alertOnStaleReconciler}) — a stalled + * reconciler is now an operational alert, not a per-request fallback. */ export const runWorkOsEventsSync = (): Promise => Effect.runPromise( syncWorkOsEvents().pipe( + Effect.tap((report) => alertOnStaleReconciler(report)), Effect.asVoid, Effect.provide(makeSyncServices()), Effect.scoped, diff --git a/apps/cloud/src/auth/workos-events-sync.node.test.ts b/apps/cloud/src/auth/workos-events-sync.node.test.ts index 6e5318963..aff07942d 100644 --- a/apps/cloud/src/auth/workos-events-sync.node.test.ts +++ b/apps/cloud/src/auth/workos-events-sync.node.test.ts @@ -62,7 +62,6 @@ import { UserStoreService } from "./context"; import { WorkOSError } from "./errors"; import { cloudMemberDirectoryLayer } from "./member-directory"; import { mirrorSignIn } from "./mirror-feeders"; -import { MirrorReadiness, MirrorReadinessState } from "./mirror-readiness"; import { authorizeOrganization } from "./organization"; import { WorkOSClient, type WorkOSClientService, type WorkOSListEventsOptions } from "./workos"; import { @@ -202,26 +201,14 @@ const profiles = (reads: string[] = []): Partial => ({ }); const DbLive = DbService.Live; -// The mirror is READY here (the authorization checks below read the mirror, -// not WorkOS); the readiness rule is pinned in workos-mirror.node.test.ts. -const readyMirror = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - +// The authorization checks below always read the mirror, never WorkOS. const MirrorServices = Layer.mergeAll( WorkOsMirror.Live, UserStoreService.Live, cloudMemberDirectoryLayer, - readyMirror, ).pipe(Layer.provideMerge(DbLive)); -type Services = - | WorkOsMirror - | UserStoreService - | MemberDirectory - | MirrorReadiness - | DbService - | WorkOSClient; +type Services = WorkOsMirror | UserStoreService | MemberDirectory | DbService | WorkOSClient; const run = ( body: Effect.Effect, @@ -549,7 +536,10 @@ describe("applyEvent", () => { const promoted = yield* applyEvent( membershipEvent( "organization_membership.updated", - workosMembership(joiner, org, { role: { slug: "admin" }, updatedAt: T2 }), + workosMembership(joiner, org, { + role: { slug: "admin" }, + updatedAt: T2, + }), ), ); // A member WorkOS no longer has (their `user.deleted` is further down @@ -558,7 +548,15 @@ describe("applyEvent", () => { membershipEvent("organization_membership.created", workosMembership(gone, org)), ); const goneRow = yield* readMembership(gone, org); - return { knownJoins, knownRow, joins, joinerRow, promoted, goneJoins, goneRow }; + return { + knownJoins, + knownRow, + joins, + joinerRow, + promoted, + goneJoins, + goneRow, + }; }), stubWorkOS({ getUser: (userId) => @@ -581,7 +579,10 @@ describe("applyEvent", () => { expect(result.joinerRow?.email).toBe(`${joiner}@placeholder.test`); expect(result.promoted).toBe("applied"); expect(result.goneJoins, "a member WorkOS no longer has is still mirrored").toBe("applied"); - expect(result.goneRow).toMatchObject({ membershipId: `om_${gone}_${org}`, name: null }); + expect(result.goneRow).toMatchObject({ + membershipId: `om_${gone}_${org}`, + name: null, + }); expect(reads, "one read per unprofiled member, none for a profiled one").toEqual([ joiner, gone, @@ -601,7 +602,9 @@ describe("applyEvent", () => { Effect.provide( Layer.mergeAll( MirrorServices, - stubWorkOS({ getUser: () => Effect.fail(new WorkOSError({ status: 503 })) }), + stubWorkOS({ + getUser: () => Effect.fail(new WorkOSError({ status: 503 })), + }), ), ), Effect.scoped, @@ -731,7 +734,11 @@ describe("applyEvent", () => { // deletion, and stalled past it now writes what it holds. yield* mirrorSignIn( workosUser(userId), - [workosMembership(userId, org, { organizationName: "Never Mirrored" })], + [ + workosMembership(userId, org, { + organizationName: "Never Mirrored", + }), + ], new Date(T1), ); const afterLogin = yield* readOrganization(org); @@ -1108,7 +1115,14 @@ describe("syncWorkOsEvents", () => { const cursor = yield* mirror.getCursor(); const membership = yield* readMembership(userId, org); const drainedAfter = yield* mirror.drainedAt(); - return { report, cursor, intruder, membership, drainedBefore, drainedAfter }; + return { + report, + cursor, + intruder, + membership, + drainedBefore, + drainedAfter, + }; }), ); expect(requests, "the second page is never read").toHaveLength(1); diff --git a/apps/cloud/src/auth/workos-mirror.node.test.ts b/apps/cloud/src/auth/workos-mirror.node.test.ts index 8a07eab01..2ac33a933 100644 --- a/apps/cloud/src/auth/workos-mirror.node.test.ts +++ b/apps/cloud/src/auth/workos-mirror.node.test.ts @@ -63,11 +63,10 @@ import { import { makeWorkOsMirrorStore } from "./workos-mirror-store"; import { MIRROR_RECONCILER_LAG_BUDGET, - MirrorReadiness, MirrorReadinessState, - makeMirrorReadinessLayer, mirrorReadinessFrom, -} from "./mirror-readiness"; + readMirrorReadiness, +} from "./mirror-readiness-store"; const DbLive = DbService.Live; const Services = Layer.mergeAll( @@ -243,7 +242,11 @@ describe("WorkOsMirror upserts", () => { // the row holds — the payload a timestamp guard would let through. // Same id: the membership is deleted, it never returns. const newerSameId = yield* mirror.upsertMembership( - membership(org, id, { id: membershipId, role: "admin", updatedAt: T3 }), + membership(org, id, { + id: membershipId, + role: "admin", + updatedAt: T3, + }), ); const afterNewerSameId = yield* directory.membership(id, org, ["inactive"]); // The member re-added in WorkOS: a payload newer than the deletion, @@ -299,7 +302,10 @@ describe("WorkOsMirror upserts", () => { result.newerSameId, "a payload of the deleted id stamped AFTER the removal is refused: identity, not time", ).toBe(false); - expect(result.afterNewerSameId).toMatchObject({ status: "inactive", role: "member" }); + expect(result.afterNewerSameId).toMatchObject({ + status: "inactive", + role: "member", + }); expect(result.readded, "a replacement under a new id reactivates").toBe(true); expect(result.afterReadd?.status).toBe("active"); expect(result.lateDelete, "a replayed deletion of the OLD id is refused").toBe(false); @@ -516,7 +522,11 @@ describe("WorkOsMirror upserts", () => { // with status inactive): the row is inactive but still WorkOS's, with // no `deleted_at` to protect it. const deactivated = yield* mirror.upsertMembership( - membership(org, id, { id: membershipId, status: "inactive", updatedAt: T2 }), + membership(org, id, { + id: membershipId, + status: "inactive", + updatedAt: T2, + }), ); const whileInactive = yield* directory.membership(id, org); // A payload older than the deactivation cannot undo it, nor one @@ -529,7 +539,11 @@ describe("WorkOsMirror upserts", () => { ); // WorkOS reactivates it, same id, newer stamp: live again. const reactivated = yield* mirror.upsertMembership( - membership(org, id, { id: membershipId, role: "admin", updatedAt: T3 }), + membership(org, id, { + id: membershipId, + role: "admin", + updatedAt: T3, + }), ); const afterReactivate = yield* directory.membership(id, org); return { @@ -634,7 +648,10 @@ describe("WorkOsMirror upserts", () => { result.rejoined, "a membership of a deleted user is refused however it is stamped: identity, not time", ).toBe(false); - expect(result.afterRejoin).toMatchObject({ status: "inactive", name: null }); + expect(result.afterRejoin).toMatchObject({ + status: "inactive", + name: null, + }); expect(result.unknown, "deleting an unseen user leaves a tombstone").toBe(true); expect(result.unseenProfile, "which the older profile cannot fill").toBe(false); expect( @@ -883,19 +900,20 @@ describe("mirror readiness", () => { const result = await run( Effect.gen(function* () { const mirror = yield* WorkOsMirror; - const readiness = yield* MirrorReadiness; + const { db } = yield* DbService; + const readiness = () => Effect.promise(() => readMirrorReadiness(db, new Date())); yield* clearEventsRow; - const noRow = yield* readiness.state(); + const noRow = yield* readiness(); yield* mirror.setReplayBoundary(T1); yield* mirror.markBackfillCompleted(T1); - const backfilledOnly = yield* readiness.state(); + const backfilledOnly = yield* readiness(); // A drain as of now: what a reconciler run that just read the stream // to its end records. const drainedAt = new Date(); yield* mirror.markDrained(drainedAt); - const ready = yield* readiness.state(); + const ready = yield* readiness(); return { noRow, backfilledOnly, ready }; - }).pipe(Effect.provide(makeMirrorReadinessLayer().pipe(Layer.provide(DbLive)))), + }), ); expect(result.noRow).toEqual(MirrorReadinessState.BackfillPending()); expect(result.backfilledOnly).toEqual( @@ -1006,7 +1024,12 @@ describe("WorkOsMirror backfill sync state", () => { yield* mirror.applyOrganizationScan({ organizationId: org, listedAt: T2, - members: [{ user: user(kept), membership: membership(org, kept, { updatedAt: T1 }) }], + members: [ + { + user: user(kept), + membership: membership(org, kept, { updatedAt: T1 }), + }, + ], }); // The stalled login resumes and writes what it holds: refused, the // revocation predates the scan and nothing would ever undo the row. diff --git a/apps/cloud/src/extensions/billing/route.node.test.ts b/apps/cloud/src/extensions/billing/route.node.test.ts index 1f10d3abc..956bb83c3 100644 --- a/apps/cloud/src/extensions/billing/route.node.test.ts +++ b/apps/cloud/src/extensions/billing/route.node.test.ts @@ -4,7 +4,6 @@ import { Effect, Layer } from "effect"; import { MemberDirectory } from "@executor-js/api/server"; import { UserStoreService } from "../../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../../auth/mirror-readiness"; import { WorkOSClient, type WorkOSClientService } from "../../auth/workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "../../auth/workos-mirror"; import { resolveBillingOrganization } from "./route"; @@ -40,12 +39,6 @@ const stubWorkOS = Layer.succeed( ); // MEMBER is active in both orgs, as the mirror reports it. -// The mirror is READY in these tests (backfill complete, reconciler caught -// up), so membership is read from the stubbed directory, never from WorkOS. -const stubReadiness = Layer.succeed(MirrorReadiness)({ - state: () => Effect.succeed(MirrorReadinessState.Ready()), -}); - const stubDirectory = Layer.succeed(MemberDirectory)({ membership: (accountId, organizationId) => Effect.succeed( @@ -122,9 +115,7 @@ const run = (headers: Record) => resolveBillingOrganization( new Request("https://executor.test/api/billing/customer", { headers }), { userId: MEMBER }, - ).pipe( - Effect.provide(Layer.mergeAll(stubWorkOS, stubUsers, stubDirectory, stubMirror, stubReadiness)), - ); + ).pipe(Effect.provide(Layer.mergeAll(stubWorkOS, stubUsers, stubDirectory, stubMirror))); describe("billing route org selector", () => { it.effect("fails closed when no selector header is sent", () => diff --git a/apps/cloud/src/extensions/routes.ts b/apps/cloud/src/extensions/routes.ts index 8d2b541d4..f1c4389fe 100644 --- a/apps/cloud/src/extensions/routes.ts +++ b/apps/cloud/src/extensions/routes.ts @@ -31,7 +31,6 @@ import { AccountApi, AdminUsersApi } from "@executor-js/api"; import { requestScopedMiddleware, type MemberDirectory } from "@executor-js/api/server"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { CloudAuthPublicHandlers, @@ -80,9 +79,7 @@ const spec = OpenApi.fromApi(CloudOpenApi); * core. */ export const makeCloudExtensionRoutes = ( - rsLive: Layer.Layer< - DbService | UserStoreService | WorkOsMirror | MemberDirectory | MirrorReadiness - >, + rsLive: Layer.Layer, ) => { // Session routes (login / callback / me / switch-org / …). Handlers yield // `UserStoreService` directly; the per-request DB combine keeps the postgres @@ -119,7 +116,9 @@ export const makeCloudExtensionRoutes = ( // rather than on the protected API because the protected plane's middleware // binds a product-view executor to one acting member — this one authorizes an // org key (or an admin session) and builds a subject-less platform view. - const AdminUsersRoutes = makeCloudAdminUsersRoutes(rsLive, { router: apiPrefixedRouter }); + const AdminUsersRoutes = makeCloudAdminUsersRoutes(rsLive, { + router: apiPrefixedRouter, + }); // The WorkOS webhook needs no per-request DB layer: it verifies the // signature with the boot `WorkOSClient` and detaches a reconciler pass diff --git a/apps/cloud/src/mcp/auth-provider.ts b/apps/cloud/src/mcp/auth-provider.ts index ab538e4dd..b05685af5 100644 --- a/apps/cloud/src/mcp/auth-provider.ts +++ b/apps/cloud/src/mcp/auth-provider.ts @@ -2,7 +2,7 @@ // Cloud McpAuthProvider adapter — the cloud analog of selfHostMcpAuthProviderLayer. // // Folds the entire cloud edge auth/authz surface (WorkOS JWT verify + API-key -// bearer + per-request org-liveness check + the two OAuth discovery docs) into +// bearer + per-request membership check + the two OAuth discovery docs) into // ONE `McpAuthProvider` Layer behind the shared host-mcp envelope. // // `authenticate(request)` runs on EVERY /mcp request and resolves a typed @@ -10,13 +10,16 @@ // - missing bearer -> Unauthorized (challenge: Bearer resource_metadata=…) // - invalid token/api key -> Unauthorized (challenge: Bearer error="invalid_token" …) // - transient JWKS OR membership-lookup infra -> Unavailable (caught here; -// envelope renders a retryable 503 -32001). A WorkOS blip during the live -// org check is a TRANSIENT failure, not evidence the org is gone, so it -// must NOT reach the Forbidden/destroy path below. +// envelope renders a retryable 503 -32001). The membership check reads +// the local mirror (`auth/organization.ts`), so the infra that can fail +// here is the database — or WorkOS, on the one path that still asks it +// (an organization the mirror has never seen). Either is TRANSIENT, not +// evidence the org is gone, and must NOT reach the Forbidden/destroy +// path below. // - no org / revoked org -> Forbidden ("No organization in session …", -32001). // This requires a POSITIVE determination (the lookup SUCCEEDED and the org // is absent), never a failed lookup. Because authenticate reads the -// mcp-session-id header to do the live org check, the envelope's +// mcp-session-id header to do the membership check, the envelope's // dispose-on-Forbidden-with-sessionId path reproduces the old inline // clearExistingSession. // - verified + org allowed -> Authenticated(principal) @@ -70,15 +73,17 @@ const TOOLKIT_PROTECTED_RESOURCE_METADATA_PATH = `${PROTECTED_RESOURCE_METADATA_ const NO_ORGANIZATION_MESSAGE = "No organization in session — log in via the web app first"; -// A transient WorkOS failure (429 / 5xx / timeout / network) during the live -// membership lookup must NOT masquerade as "org revoked" — but the failure -// channel alone is not enough to tell them apart: WorkOS also answers with -// DEFINITIVE 4xx denials (401 revoked/invalid API key, 403, 404 deleted org) -// that the SDK throws as typed exceptions. So the classification is: +// A transient failure during the membership lookup must NOT masquerade as +// "org revoked". The lookup reads the local mirror, so in steady state its +// only failure is the database; the one path that still asks WorkOS (an +// organization the mirror has never seen, resolved for a caller WorkOS +// confirms as its member) can also fail with a DEFINITIVE 4xx denial (401 +// revoked/invalid API key, 403, 404 deleted org) that the SDK throws as a +// typed exception. So the classification is: // - lookup SUCCEEDS with `null` -> genuine absence -> Forbidden // - lookup FAILS with WorkOS 401/403/404 -> definitive denial -> Forbidden // (fail CLOSED: WorkOS answered and said no; retrying cannot help) -// - lookup FAILS any other way (429/5xx/timeout/network/no status) +// - lookup FAILS any other way (database, 429/5xx/timeout/network/no status) // -> transient -> retryable 503, session preserved // The status rides on `WorkOSError.status` (threaded from the SDK exception at // the service boundary in auth/workos.ts); `isDefinitiveWorkOSDenial` is the diff --git a/apps/cloud/src/mcp/auth.ts b/apps/cloud/src/mcp/auth.ts index 81ac8bc73..9d34df7c5 100644 --- a/apps/cloud/src/mcp/auth.ts +++ b/apps/cloud/src/mcp/auth.ts @@ -18,7 +18,6 @@ import { BEARER_PREFIX } from "../auth/bearer"; import { authorizeOrganization } from "../auth/organization"; import { UserStoreService, makeUserStoreLayer } from "../auth/context"; import { makeMemberDirectoryLayer } from "../auth/member-directory"; -import { makeMirrorReadinessLayer } from "../auth/mirror-readiness"; import { makeWorkOsMirrorLayer } from "../auth/workos-mirror"; import { CoreSharedServices } from "../auth/workos"; import { makeDbLayer } from "../db/db"; @@ -211,13 +210,11 @@ const makeMcpOrganizationAuthServices = () => { const dbLive = makeDbLayer(); const userStoreLive = makeUserStoreLayer().pipe(Layer.provide(dbLive)); const memberDirectoryLive = makeMemberDirectoryLayer().pipe(Layer.provide(dbLive)); - const mirrorReadinessLive = makeMirrorReadinessLayer().pipe(Layer.provide(dbLive)); const workOsMirrorLive = makeWorkOsMirrorLayer().pipe(Layer.provide(dbLive)); return Layer.mergeAll( dbLive, userStoreLive, memberDirectoryLive, - mirrorReadinessLive, workOsMirrorLive, CoreSharedServices, ); @@ -329,7 +326,9 @@ export const McpAuthLive = Layer.effect( if (!verified) return mcpUnauthorized("invalid_token", "The access token is invalid"); if (Predicate.isTagged(verified, "Unauthorized")) return verified; if (!verified.accountId) { - yield* Effect.annotateCurrentSpan({ "mcp.auth.outcome": "missing_subject" }); + yield* Effect.annotateCurrentSpan({ + "mcp.auth.outcome": "missing_subject", + }); return mcpUnauthorized("invalid_token", "The access token is invalid"); } yield* Effect.annotateCurrentSpan({ @@ -344,7 +343,9 @@ export const McpAuthLive = Layer.effect( verifyBearer: Effect.fn("mcp.auth.verify_bearer")(function* (request) { const authHeader = request.headers.get("authorization"); if (!authHeader?.startsWith(BEARER_PREFIX)) { - yield* Effect.annotateCurrentSpan({ "mcp.auth.outcome": "missing_bearer" }); + yield* Effect.annotateCurrentSpan({ + "mcp.auth.outcome": "missing_bearer", + }); return mcpUnauthorized("missing_bearer"); } const token = authHeader.slice(BEARER_PREFIX.length).trim(); diff --git a/apps/cloud/src/org/auth-middleware.ts b/apps/cloud/src/org/auth-middleware.ts index d9f562494..9c61236f3 100644 --- a/apps/cloud/src/org/auth-middleware.ts +++ b/apps/cloud/src/org/auth-middleware.ts @@ -9,7 +9,6 @@ import { import { UserStoreService } from "../auth/context"; import { sessionFromSealed } from "../auth/middleware"; -import { MirrorReadiness } from "../auth/mirror-readiness"; import { WorkOsMirror } from "../auth/workos-mirror"; import { ORG_SELECTOR_HEADER, authorizeOrganizationSelector } from "../auth/organization"; import { WorkOSClient } from "../auth/workos"; @@ -35,19 +34,19 @@ const noOrganization = () => /** * The caller's role in the session org, as `authorizeOrganizationSelector` - * read it for THIS request: from the mirror while the mirror is ready, from - * WorkOS otherwise (`auth/organization.ts`). Provided beside `AuthContext` — - * the shared seam, which carries no role — so the domain handlers' admin gate - * is this one value, never a second read of the mirror that would skip the - * readiness rule and admit a demoted admin on a stale row while the - * reconciler is behind. + * read it for THIS request from the membership mirror (`auth/organization.ts`). + * Provided beside `AuthContext` — the shared seam, which carries no role — so + * the domain handlers' admin gate is this one value, never a second read of + * the mirror. */ export class OrgMemberRole extends Context.Service< OrgMemberRole, { readonly memberRole: "admin" | "member" } >()("@executor-js/cloud/OrgMemberRole") {} -const OrgAuthMiddleware = HttpRouter.middleware<{ provides: AuthContext | OrgMemberRole }>()( +const OrgAuthMiddleware = HttpRouter.middleware<{ + provides: AuthContext | OrgMemberRole; +}>()( Effect.gen(function* () { const captured = yield* Effect.context(); const workos = yield* WorkOSClient; @@ -93,7 +92,5 @@ const OrgAuthMiddleware = HttpRouter.middleware<{ provides: AuthContext | OrgMem ); export const orgAuthMiddleware = ( - rsLive: Layer.Layer< - DbService | UserStoreService | MemberDirectory | MirrorReadiness | WorkOsMirror - >, + rsLive: Layer.Layer, ) => OrgAuthMiddleware.combine(requestScopedMiddleware(rsLive)).layer; diff --git a/apps/cloud/src/org/handlers.test.ts b/apps/cloud/src/org/handlers.test.ts index e1f4459cb..1cae2aa9a 100644 --- a/apps/cloud/src/org/handlers.test.ts +++ b/apps/cloud/src/org/handlers.test.ts @@ -5,7 +5,6 @@ import { HttpApiBuilder } from "effect/unstable/httpapi"; import { AuthContext, MemberDirectory, type DirectoryMember } from "@executor-js/api/server"; import { UserStoreService } from "../auth/context"; -import { MirrorReadiness, MirrorReadinessState } from "../auth/mirror-readiness"; import { ORG_SELECTOR_HEADER } from "../auth/organization"; import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { WorkOsMirror, type WorkOsMirrorShape } from "../auth/workos-mirror"; @@ -23,8 +22,8 @@ import { OrgHandlers, assertDomainInSessionOrg, requireAdmin } from "./handlers" // share — the REAL `requireAdmin` and `assertDomainInSessionOrg` exported from // `org/handlers.ts`, so a change to the gate cannot pass on a stale copy — and // the admin gate's SOURCE: the role `orgAuthMiddleware` resolved for the -// request, so a stale mirror row cannot admit a demoted admin while the mirror -// is not trusted (the readiness rule in `auth/organization.ts`). +// request, read from the local membership mirror unconditionally +// (`auth/organization.ts`). // --------------------------------------------------------------------------- // eslint-disable-next-line @typescript-eslint/no-explicit-any -- test stub needs wide function types @@ -99,7 +98,11 @@ describe("Org domain handlers", () => { Effect.provide( provide("admin", { getOrganizationDomain: () => - Effect.succeed({ id: "dom_1", organizationId: "org_1", domain: "acme.test" }), + Effect.succeed({ + id: "dom_1", + organizationId: "org_1", + domain: "acme.test", + }), }), ), ), @@ -113,7 +116,11 @@ describe("Org domain handlers", () => { Effect.provide( provide("admin", { getOrganizationDomain: () => - Effect.succeed({ id: "dom_other", organizationId: "org_2", domain: "evil.test" }), + Effect.succeed({ + id: "dom_other", + organizationId: "org_2", + domain: "evil.test", + }), }), ), ), @@ -136,11 +143,12 @@ describe("Org domain handlers", () => { // --------------------------------------------------------------------------- // The admin gate over HTTP, through `orgAuthMiddleware`: the role the gate -// sees is the one the middleware resolved through `authorizeOrganizationSelector` -// — the mirror while it is ready, WorkOS otherwise. The mirror row below is -// STALE: it still says `admin` for a caller WorkOS has demoted to `member`. -// While the mirror is not trusted (the reconciler is behind), WorkOS's answer -// must decide, and the delete must be refused. +// sees is the one the middleware resolved through `authorizeOrganizationSelector`, +// which reads the local membership mirror unconditionally — there is no +// readiness gate and no WorkOS fallback (`auth/organization.ts`). The mirror +// row below is the sole source of the role, so `stubWorkOS` here serves only +// session authentication and dies on anything else, proving membership is +// never re-checked against WorkOS. // --------------------------------------------------------------------------- const ORG = "org_1"; @@ -148,33 +156,30 @@ const CALLER = "user_caller"; const DOMAIN = "dom_1"; const createdAt = new Date("2026-01-01T00:00:00.000Z"); -// The mirror's row for the caller: an active admin — stale once WorkOS has -// demoted them and the reconciler has not landed the change yet. -const staleAdminRow: DirectoryMember = { +// The mirror's row for the caller, at whatever role a test sets. +const callerRow = (role: "admin" | "member"): DirectoryMember => ({ accountId: CALLER, membershipId: `om_${CALLER}_${ORG}`, organizationId: ORG, email: null, name: null, avatarUrl: null, - role: "admin", + role, status: "active", lastActiveAt: null, -}; - -const unread = (why: string) => () => Effect.die(why); -const stubDirectory = Layer.succeed(MemberDirectory)({ - membership: (accountId, organizationId) => - Effect.succeed(accountId === CALLER && organizationId === ORG ? staleAdminRow : null), - membershipById: unread("the org plane does not look up by membership id"), - membershipsOf: unread("the org plane reads one membership, not the list"), - members: unread("the org plane does not list members"), - membersById: unread("the org plane does not batch members"), - findByEmail: unread("the org plane does not resolve emails"), }); -const readiness = (state: MirrorReadinessState) => - Layer.succeed(MirrorReadiness)({ state: () => Effect.succeed(state) }); +const unread = (why: string) => () => Effect.die(why); +const stubDirectory = (role: "admin" | "member") => + Layer.succeed(MemberDirectory)({ + membership: (accountId, organizationId) => + Effect.succeed(accountId === CALLER && organizationId === ORG ? callerRow(role) : null), + membershipById: unread("the org plane does not look up by membership id"), + membershipsOf: unread("the org plane reads one membership, not the list"), + members: unread("the org plane does not list members"), + membersById: unread("the org plane does not batch members"), + findByEmail: unread("the org plane does not resolve emails"), + }); const organizationRow = (id: string) => ({ id, @@ -226,22 +231,16 @@ const stubAutumn = Layer.succeed(AutumnService)({ setMemberSeats: unread("the delete does not count seats"), }); -// WorkOS as the org plane sees it: the caller's session, their CURRENT -// membership list (demoted to member), and the domain to delete. -const workosWithCallerAs = (role: "admin" | "member", deleted: string[]) => +// WorkOS as the org plane sees it: session authentication and the domain to +// delete. `stubWorkOS` dies on anything else, so a `listUserMemberships` call +// would fail the test — membership is never re-checked against WorkOS. +const workosForCaller = (deleted: string[]) => stubWorkOS({ authenticateSealedSession: () => - Effect.succeed({ userId: CALLER, email: "caller@placeholder.test", organizationId: ORG }), - listUserMemberships: () => Effect.succeed({ - data: [ - { - id: staleAdminRow.membershipId, - organizationId: ORG, - status: "active", - role: { slug: role }, - }, - ], + userId: CALLER, + email: "caller@placeholder.test", + organizationId: ORG, }), getOrganizationDomain: () => Effect.succeed({ id: DOMAIN, organizationId: ORG, domain: "acme.test" }), @@ -251,8 +250,8 @@ const workosWithCallerAs = (role: "admin" | "member", deleted: string[]) => }), }); -const orgApp = (state: MirrorReadinessState, workos: Layer.Layer) => { - const rsLive = Layer.mergeAll(stubDb, stubUsers, stubDirectory, stubMirror, readiness(state)); +const orgApp = (role: "admin" | "member", workos: Layer.Layer) => { + const rsLive = Layer.mergeAll(stubDb, stubUsers, stubDirectory(role), stubMirror); const App = HttpApiBuilder.layer(OrgHttpApi).pipe( Layer.provide(OrgHandlers), Layer.provide(orgAuthMiddleware(rsLive)), @@ -268,9 +267,9 @@ afterAll(async () => { await Promise.all(apps.map((app) => app.dispose())); }); -const deleteDomain = async (state: MirrorReadinessState, role: "admin" | "member") => { +const deleteDomain = async (role: "admin" | "member") => { const deleted: string[] = []; - const app = orgApp(state, workosWithCallerAs(role, deleted)); + const app = orgApp(role, workosForCaller(deleted)); apps.push(app); const response = await app.handler( new Request(`https://executor.test/org/domains/${DOMAIN}`, { @@ -285,30 +284,15 @@ const deleteDomain = async (state: MirrorReadinessState, role: "admin" | "member }; describe("Org domain handlers over HTTP: the admin gate is the authorized role", () => { - it("lets a mirrored admin delete a domain while the mirror is ready", async () => { - const { status, deleted } = await deleteDomain(MirrorReadinessState.Ready(), "member"); - // WorkOS is not consulted for membership while the mirror is ready: the - // mirror row (admin) decides, and the demotion lands through the - // reconciler within its lag budget. + it("lets a mirrored admin delete a domain", async () => { + const { status, deleted } = await deleteDomain("admin"); expect(status).toBe(200); expect(deleted).toEqual([DOMAIN]); }); - it("refuses a demoted admin while the mirror is not ready, however stale the mirror row is", async () => { - const { status, deleted } = await deleteDomain( - MirrorReadinessState.ReconcilerStale({ drainedAt: null }), - "member", - ); - expect(status, "WorkOS says member; the stale admin row does not grant the delete").toBe(403); + it("refuses a mirrored plain member, without ever consulting WorkOS", async () => { + const { status, deleted } = await deleteDomain("member"); + expect(status).toBe(403); expect(deleted).toEqual([]); }); - - it("lets an admin WorkOS confirms delete a domain while the mirror is not ready", async () => { - const { status, deleted } = await deleteDomain( - MirrorReadinessState.ReconcilerStale({ drainedAt: null }), - "admin", - ); - expect(status).toBe(200); - expect(deleted).toEqual([DOMAIN]); - }); }); diff --git a/e2e/cloud/mcp-workos-blip-session-survival.test.ts b/e2e/cloud/mcp-workos-blip-session-survival.test.ts index 905dff41c..d6a5022aa 100644 --- a/e2e/cloud/mcp-workos-blip-session-survival.test.ts +++ b/e2e/cloud/mcp-workos-blip-session-survival.test.ts @@ -1,25 +1,25 @@ -// Cloud: how the per-request live-membership check classifies WorkOS failures, -// pinned in BOTH directions at the real upstream (faults armed on the WorkOS -// emulator's membership endpoint — the same emulator the product's real WorkOS -// SDK talks to; no product code or stubs touched): +// Cloud: an MCP session's relationship to WorkOS after the membership mirror. // -// 1. A TRANSIENT WorkOS outage (5xx/timeout) must NOT destroy a live MCP -// session. This is the churn-risk defect: a WorkOS blip used to collapse to -// Forbidden, and a Forbidden carrying a session id schedules the session -// Durable Object for destruction (in-flight executions, paused approvals, -// undelivered results — all gone). For a shared-API-key org a single blip -// could mass-condemn every session at once. Contract: the blip request fails -// RETRYABLY (503 + Retry-After), and once WorkOS recovers the SAME session -// id keeps serving requests. +// Membership is authorized from the local mirror on every /mcp request +// (`auth/organization.ts`); WorkOS is a write target and an event source, not +// a per-request read. Two contracts follow, pinned here at the real upstream +// (faults armed on the WorkOS emulator's membership endpoint — the same +// emulator the product's real WorkOS SDK talks to; no product code or stubs +// touched): // -// 2. A DEFINITIVE WorkOS denial (401 — the revoked/invalid API key answer) must -// fail CLOSED: Forbidden, session condemned. Retrying cannot help; treating -// it as transient would preserve sessions indefinitely for a revoked -// customer (the fail-open inversion the adversarial review caught). +// 1. A WorkOS OUTAGE is INVISIBLE to a live session. Before the mirror, a +// 5xx from the membership lookup had to be classified as transient (a +// retryable 503 that left the session alive) so a blip could not +// mass-condemn every session of a shared-API-key org. Now the request never +// asks WorkOS at all: a request issued during the outage is a plain 200, +// and the SAME session id keeps serving afterwards. The fault is armed on +// the exact endpoint the old check hit, so an unnoticed regression back to +// a per-request WorkOS read would fail this as a 503 (or worse, a 403). // -// Red/green for (1): pre-fix, the outage request returns a session-destroying -// Forbidden and the post-outage request gets 404 "reconnect". With the fix the -// outage request is a 503 and the post-outage request is a clean 200. +// 2. A REVOKED membership still fails CLOSED. The mirror is not a cache with a +// TTL: a removal made through the product writes the mirror in the same +// request, so the removed member's next /mcp request is a Forbidden, the +// session is condemned, and the id is dead. Retrying cannot help. import { expect } from "@effect/vitest"; import { Effect } from "effect"; @@ -29,6 +29,7 @@ import { scenario } from "../src/scenario"; import { Mcp, Target } from "../src/services"; import type { Identity } from "../src/target"; import { WORKOS_EMULATOR_PORT } from "../targets/cloud"; +import { cookieOf, joinOrg, orgSelectorOf } from "./support/session"; const JSON_AND_SSE = "application/json, text/event-stream"; const PROTOCOL_VERSION = "2025-03-26"; @@ -105,11 +106,11 @@ const openSession = async (mcpUrl: string, bearer: string): Promise => { return sessionId; }; -// The live membership check is `GET /user_management/organization_memberships` -// (WorkOS `listOrganizationMemberships`). A bounded count covers the outage -// request without leaking into later (post-clear) requests; we also clear -// explicitly. `times` is generous so any internal retry inside the one faulted -// request still sees the outage, but the finalizer removes whatever remains. +// The endpoint the pre-mirror per-request check hit +// (`GET /user_management/organization_memberships`). Armed to prove it is no +// longer on the request path: a request that reached it would fail. `times` +// is generous so any retry inside a faulted request still sees the outage; the +// finalizer removes whatever remains. const MEMBERSHIP_FAULT = { match: { method: "GET", @@ -119,22 +120,8 @@ const MEMBERSHIP_FAULT = { times: 8, } as const; -// The definitive-denial counterpart: WorkOS ANSWERS the membership lookup with -// 401 — the shape of a revoked/invalid API key. Not a blip; must fail closed. -const MEMBERSHIP_DENIAL_FAULT = { - match: { - method: "GET", - pathPattern: "/user_management/organization_memberships*", - }, - response: { - status: 401, - body: { message: "Could not authorize the request. Maybe your API key is invalid?" }, - }, - times: 8, -} as const; - scenario( - "MCP sessions · a transient WorkOS outage 503s retryably and leaves the session alive", + "MCP sessions · a WorkOS outage is invisible to a live session, which is authorized from the mirror", {}, Effect.gen(function* () { const target = yield* Target; @@ -151,106 +138,150 @@ scenario( const healthy = yield* Effect.promise(() => mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(2) }), ); - expect(healthy.status, "the session serves requests before the blip").toBe(200); + expect(healthy.status, "the session serves requests before the outage").toBe(200); yield* Effect.promise(() => healthy.text()); yield* Effect.gen(function* () { - // The blip: WorkOS membership lookups start failing with 503. + // The outage: WorkOS membership lookups would fail with 503 — if + // anything asked. yield* Effect.promise(() => workos.faults.arm(MEMBERSHIP_FAULT)); - // A request issued DURING the outage. The membership lookup fails - // transiently — this must be a retryable 503, NOT a Forbidden (which - // would condemn the session). + // A request issued DURING the outage. Membership is read from the + // mirror, so WorkOS is never consulted and the request is a plain + // success — not a retryable 503 (the pre-mirror contract) and never a + // Forbidden (which would condemn the session). const duringOutage = yield* Effect.promise(() => mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(3) }), ); - const outageBody = (yield* Effect.promise(() => duringOutage.json())) as JsonRpcError; expect( duringOutage.status, - "a WorkOS blip is a retryable 503, not a session-destroying error", - ).toBe(503); - expect( - duringOutage.status, - "the blip is NOT surfaced as a 404 reconnect (which would mean the session was destroyed)", - ).not.toBe(404); - expect( - outageBody.error.code, - "the 503 is a JSON-RPC error envelope the transport retries", - ).toBe(-32001); - expect( - duringOutage.headers.get("retry-after"), - "the 503 advertises a Retry-After so clients back off", - ).toEqual(expect.any(String)); + "a WorkOS outage does not touch a request: membership comes from the mirror", + ).toBe(200); + yield* Effect.promise(() => duringOutage.text()); }).pipe( - // Always lift the outage, even if an assertion above fails, so the - // recovery request runs against a healthy WorkOS. + // Always lift the outage, even if an assertion above fails. Effect.ensuring(Effect.promise(() => workos.faults.clear())), ); - // WorkOS has recovered. The SAME session id must still serve requests: the - // blip left it untouched. On the pre-fix code this is a 404 (the outage - // request destroyed the DO); with the fix it is a clean 200. + // The SAME session id keeps serving after the outage: nothing condemned it. const afterOutage = yield* Effect.promise(() => mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(4) }), ); - expect( - afterOutage.status, - "the session survived the blip and resumes work once WorkOS recovers", - ).toBe(200); + expect(afterOutage.status, "the session is untouched by the outage").toBe(200); yield* Effect.promise(() => afterOutage.text()); }), ); scenario( - "MCP sessions · a definitive WorkOS denial fails closed and condemns the session", + "MCP sessions · a revoked membership fails closed on the next request and condemns the session", {}, Effect.gen(function* () { const target = yield* Target; const mcp = yield* Mcp; - const identity = yield* target.newIdentity(); - const bearer = yield* mcp.mintBearer(emailOf(identity)); - const workos = yield* Effect.promise(() => - connectEmulator({ baseUrl: `http://127.0.0.1:${WORKOS_EMULATOR_PORT}` }), - ); + // An admin's org with one plain member, joined through the real invite → + // accept flow. The member is the one whose access is revoked. + const admin = yield* target.newIdentity(); + const invitee = yield* target.newIdentity({ org: false }); + const member = yield* joinOrg(target, admin, invitee); + const bearer = yield* mcp.mintBearer(emailOf(member)); + const orgSelector = orgSelectorOf(member); - // A healthy session doing real work before the denial. - const sessionId = yield* Effect.promise(() => openSession(target.mcpUrl, bearer)); - const healthy = yield* Effect.promise(() => - mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(2) }), - ); - expect(healthy.status, "the session serves requests before the denial").toBe(200); + // The member's healthy session doing real work before the revocation. + const mcpUrl = `${target.mcpUrl}`; + const withOrg = (body: unknown, sessionId?: string) => + fetch(mcpUrl, { + method: "POST", + headers: { + accept: JSON_AND_SSE, + "content-type": "application/json", + authorization: `Bearer ${bearer}`, + "x-executor-mcp-organization": orgSelector, + ...(sessionId ? { "mcp-session-id": sessionId } : {}), + }, + body: JSON.stringify(body), + }); + const initialize = yield* Effect.promise(() => withOrg(INITIALIZE_REQUEST)); + const sessionId = initialize.headers.get("mcp-session-id"); + yield* Effect.promise(() => initialize.text()); + expect(initialize.status, "the member opens a session in the org").toBe(200); + if (!sessionId) throw new Error("initialize returned no session id"); + const initialized = yield* Effect.promise(() => withOrg(INITIALIZED_NOTIFICATION, sessionId)); + yield* Effect.promise(() => initialized.text()); + const healthy = yield* Effect.promise(() => withOrg(toolsList(2), sessionId)); + expect(healthy.status, "the session serves requests before the revocation").toBe(200); yield* Effect.promise(() => healthy.text()); - yield* Effect.gen(function* () { - // WorkOS starts ANSWERING the membership lookup with 401 — the - // revoked/invalid API key shape. Deterministic denial, not a blip. - yield* Effect.promise(() => workos.faults.arm(MEMBERSHIP_DENIAL_FAULT)); + // The admin removes the member through the product. The removal writes + // the mirror in the same request (a deletion tombstone keyed to the + // WorkOS membership id), so no reconciler tick is needed for it to land. + const members = yield* Effect.promise(async () => { + const response = await fetch(new URL("/api/account/members", target.baseUrl), { + headers: { ...(admin.headers ?? {}) }, + }); + if (!response.ok) throw new Error(`/api/account/members failed (${response.status})`); + return (await response.json()) as { + readonly members: ReadonlyArray<{ readonly id: string; readonly isCurrentUser: boolean }>; + }; + }); + const removed = members.members.find((row) => !row.isCurrentUser); + if (!removed) throw new Error("the joined member is not listed in the org"); + const removal = yield* Effect.promise(() => + fetch(new URL(`/api/account/members/${removed.id}`, target.baseUrl), { + method: "DELETE", + headers: { ...(admin.headers ?? {}), origin: new URL(target.baseUrl).origin }, + }), + ); + expect(removal.status, "the admin removes the member").toBe(200); + yield* Effect.promise(() => removal.text()); - const denied = yield* Effect.promise(() => - mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(3) }), - ); - const deniedBody = (yield* Effect.promise(() => denied.json())) as JsonRpcError; - expect( - denied.status, - "a definitive WorkOS denial fails closed as Forbidden, never a retryable 503", - ).toBe(403); - expect(deniedBody.error.code, "the denial is a JSON-RPC error envelope").toBe(-32001); - }).pipe(Effect.ensuring(Effect.promise(() => workos.faults.clear()))); + // The removed member's NEXT request on the live session: a positive + // determination from the mirror that they hold no active membership — a + // real Forbidden, which condemns the session. + const denied = yield* Effect.promise(() => withOrg(toolsList(3), sessionId)); + const deniedBody = (yield* Effect.promise(() => denied.json())) as JsonRpcError; + expect(denied.status, "a revoked member fails closed as Forbidden on the next request").toBe( + 403, + ); + expect(deniedBody.error.code, "the denial is a JSON-RPC error envelope").toBe(-32001); - // The Forbidden carried the session id, so the session was condemned: the - // id must NOT serve requests once WorkOS recovers. If this returned 200 the - // fail-closed contract is broken (a revoked customer kept a live session). - const afterDenial = yield* Effect.promise(() => - mcpPost(target.mcpUrl, { bearer, sessionId, body: toolsList(4) }), + // While revoked, every further request is refused at the gate — still a + // Forbidden, never a 200 (a removed member kept a live session) and never + // a retryable 503 (nothing about this is transient). + const stillDenied = yield* Effect.promise(() => withOrg(toolsList(4), sessionId)); + expect(stillDenied.status, "a revoked member stays refused, deterministically").toBe(403); + yield* Effect.promise(() => stillDenied.text()); + + // The Forbidden carried the session id, so the session was condemned. A + // caller the gate admits proves it: the admin, still a member, presents + // the condemned id with their own bearer. Had the id survived, the answer + // would be the ownership Forbidden (-32003: the session belongs to someone + // else); condemned, it is dead and the client is told to reconnect. + const adminBearer = yield* mcp.mintBearer(emailOf(admin)); + const condemned = yield* Effect.promise(() => + fetch(mcpUrl, { + method: "POST", + headers: { + accept: JSON_AND_SSE, + "content-type": "application/json", + authorization: `Bearer ${adminBearer}`, + "x-executor-mcp-organization": orgSelectorOf(admin), + "mcp-session-id": sessionId, + }, + body: JSON.stringify(toolsList(5)), + }), ); - expect( - afterDenial.status, - "the condemned session id is dead after a definitive denial (reconnect required)", - ).toBe(404); - const afterBody = (yield* Effect.promise(() => afterDenial.json())) as JsonRpcError; - expect(afterBody.error.message, "the client is told to reconnect").toMatch( + expect(condemned.status, "the condemned session id is dead (reconnect required)").toBe(404); + const condemnedBody = (yield* Effect.promise(() => condemned.json())) as JsonRpcError; + expect(condemnedBody.error.message, "the client is told to reconnect").toMatch( /timed out|reconnect|not found/i, ); + + // The admin's own access is unaffected by removing someone else. + const adminStillIn = yield* Effect.promise(() => + fetch(new URL("/api/account/me", target.baseUrl), { headers: { cookie: cookieOf(admin) } }), + ); + expect(adminStillIn.status, "the admin keeps their access").toBe(200); + yield* Effect.promise(() => adminStillIn.text()); }), );