Classify non-conform OAuth refresh rejections and stop retrying dead grants (#1507)

Author: RhysSullivan

.changeset/oauth-dead-grant-classification.md

@@ -0,0 +1,5 @@
+---
+"executor": patch
+---
+
+**Fix: OAuth refresh rejections with non-spec error bodies (e.g. Datadog) now surface as expired connections with a reconnect path, and definitively dead refresh tokens are no longer retried against the authorization server**

packages/core/sdk/src/executor.ts

@@ -783,6 +783,19 @@ const missingOAuthScopesFromProviderState = (value: unknown): readonly string[]
     : [];
 };
 
+/** Epoch ms of the definitive refresh rejection recorded on `provider_state`,
+ *  or null. Set when the AS rejects the grant itself (RFC 6749 invalid_grant —
+ *  retrying cannot change the verdict); cleared by the reconnect mint, which
+ *  rewrites `provider_state` wholesale. While set, refresh attempts are
+ *  skipped: the pre-fix behavior re-sent a known-dead grant to the AS every
+ *  proactive cycle, forever, and surfaced nothing to the user. */
+const oauthReauthRequiredAtFromProviderState = (value: unknown): number | null => {
+  const decoded = decodeJsonColumn(value);
+  if (decoded == null || typeof decoded !== "object" || Array.isArray(decoded)) return null;
+  const at = (decoded as Record<string, unknown>).oauthReauthRequiredAt;
+  return typeof at === "number" ? at : null;
+};
+
 const rowToConnection = (row: ConnectionRow): Connection => {
   const owner = row.owner as Owner;
   const integration = IntegrationSlug.make(row.integration);
@@ -1744,6 +1757,44 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
      *  upstream 401 on a token we believed was still valid (`reactive`). */
     type RefreshTrigger = "proactive" | "reactive";
 
+    /** Record the AS's invalid_grant verdict on the row so later refreshes
+     *  skip the doomed token request, and stamp `last_health` expired so the
+     *  accounts list shows the dead connection at a glance instead of only
+     *  after a manual probe. Merges into `provider_state` (preserving
+     *  `missingOAuthScopes`); the reconnect mint rewrites the column wholesale,
+     *  which is what re-arms refresh. Best-effort: a bookkeeping write failure
+     *  must not mask the refresh failure being reported. */
+    const markRefreshGrantDead = (
+      row: ConnectionRow,
+      detail: string,
+    ): Effect.Effect<void, never> => {
+      const existingState = decodeJsonColumn(row.provider_state);
+      const mergedState =
+        existingState != null && typeof existingState === "object" && !Array.isArray(existingState)
+          ? (existingState as Record<string, unknown>)
+          : {};
+      const health: HealthCheckResult = {
+        status: "expired",
+        checkedAt: Date.now(),
+        detail,
+      };
+      return core
+        .updateMany("connection", {
+          where: (b: AnyCb) =>
+            b.and(
+              byOwner(row.owner as Owner)(b),
+              b("integration", "=", String(row.integration)),
+              b("name", "=", String(row.name)),
+            ),
+          set: {
+            provider_state: { ...mergedState, oauthReauthRequiredAt: Date.now() },
+            last_health: health,
+            updated_at: new Date(),
+          },
+        })
+        .pipe(Effect.ignore);
+    };
+
     // Perform the actual refresh-token grant and persist the rotated material.
     const performTokenRefresh = (
       row: ConnectionRow,
@@ -1761,6 +1812,20 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
             reauthRequired: true,
           });
 
+        // A recorded invalid_grant is the AS's standing verdict on this grant:
+        // re-sending it cannot succeed, so don't. Fail as reauth-required
+        // without a token request — the reconnect mint rewrites
+        // `provider_state` and thereby re-arms refresh. Without this gate a
+        // dead connection re-sent its dead grant on every proactive cycle,
+        // indefinitely (owner.com's Datadog connections: 100+ identical
+        // rejections over two days, surfacing nothing).
+        if (oauthReauthRequiredAtFromProviderState(row.provider_state) !== null) {
+          yield* Effect.annotateCurrentSpan({ "executor.oauth.refresh.skipped_known_dead": true });
+          return yield* reauth(
+            "The authorization server rejected this connection's refresh token (invalid_grant). Reconnect to continue.",
+          );
+        }
+
         // Load the backing app by the owner STORED on the connection (a Personal
         // connection may be backed by a shared Workspace app) — no derivation.
         const clientOwner = (row.oauth_client_owner ?? row.owner) as Owner;
@@ -1866,6 +1931,16 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
                       cause,
                     });
                   }),
+                  // Persist the definitive verdict so the NEXT refresh skips
+                  // the doomed grant (see the known-dead gate above) and the
+                  // connection shows `expired` without waiting for a probe.
+                  Effect.tapError((error) =>
+                    Predicate.isTagged(error, "CredentialResolutionError") &&
+                    error.reauthRequired === true
+                      ? // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field
+                        markRefreshGrantDead(row, error.message)
+                      : Effect.void,
+                  ),
                 );
               });
 
@@ -1932,6 +2007,12 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         ),
         Effect.withSpan("executor.oauth.refresh", {
           attributes: {
+            // Tenant + subject make refresh outcomes answerable PER CUSTOMER
+            // ("is org X's Datadog refresh healthy?") — without them the only
+            // grouping dimensions were integration-wide. Opaque ids, never
+            // emails or org names.
+            "executor.tenant": tenant,
+            ...(subject != null ? { "executor.subject": subject } : {}),
             "executor.integration": String(row.integration),
             "executor.connection": String(row.name),
             // Which path drove this refresh: the expiry check ahead of a call,
@@ -3277,6 +3358,8 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
       }).pipe(
         Effect.withSpan("executor.connection.health.check", {
           attributes: {
+            "executor.tenant": tenant,
+            ...(subject != null ? { "executor.subject": subject } : {}),
             "executor.integration": String(ref.integration),
             "executor.connection": String(ref.name),
           },
@@ -3332,7 +3415,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         return result;
       }).pipe(
         Effect.withSpan("executor.connection.validate", {
-          attributes: { "executor.integration": String(input.integration) },
+          attributes: {
+            "executor.tenant": tenant,
+            ...(subject != null ? { "executor.subject": subject } : {}),
+            "executor.integration": String(input.integration),
+          },
         }),
       );
 

packages/core/sdk/src/oauth-flow.test.ts

@@ -962,6 +962,116 @@ describe("oauth token refresh in resolveConnectionValue", () => {
     ),
   );
 
+  it.effect("a definitively rejected grant is not re-sent to the AS on later refreshes", () =>
+    Effect.scoped(
+      Effect.gen(function* () {
+        const server = yield* serveOAuthTestServer({
+          scopes: ["read"],
+          supportRefresh: false,
+          tokenExpiresInSeconds: 0,
+          invalidRefreshTokenDescription: "Grant revoked",
+        });
+        const harness = yield* makeTestWorkspaceHarness({ plugins });
+        const { executor, config } = harness;
+        yield* executor.acme.seed();
+
+        yield* executor.oauth.createClient({
+          owner: "org",
+          slug: CLIENT,
+          authorizationUrl: server.authorizationEndpoint,
+          tokenUrl: server.tokenEndpoint,
+          grant: "authorization_code",
+          clientId: "test-client",
+          clientSecret: "test-secret",
+          resource: server.mcpResourceUrl,
+        });
+
+        const started = yield* executor.oauth.start({
+          owner: "org",
+          client: CLIENT,
+          clientOwner: "org",
+          name: ConnectionName.make("main"),
+          integration: INTEG,
+          template: TEMPLATE,
+        });
+        expect(started.status).toBe("redirect");
+        if (started.status !== "redirect") return;
+        const callback = yield* server.completeAuthorizationCodeFlow({
+          authorizationUrl: started.authorizationUrl,
+        });
+        yield* executor.oauth.complete({ state: started.state, code: callback.code });
+
+        yield* Effect.promise(() =>
+          config.db.updateMany("connection", {
+            where: (b) => b("name", "=", "main"),
+            set: { expires_at: Date.now() - 60_000 },
+          }),
+        );
+        yield* server.clearRequests;
+
+        // First resolve: the refresh grant reaches the AS and is rejected
+        // with invalid_grant — the AS's definitive verdict.
+        const first = yield* Effect.flip(
+          executor.execute(ToolAddress.make("tools.acme.org.main.whoami"), {}),
+        );
+        expect(JSON.stringify(first)).toContain("invalid_grant");
+
+        // The verdict is persisted: the dead-grant marker plus an expired
+        // health record, without waiting for a probe.
+        const row = yield* Effect.promise(() =>
+          config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }),
+        );
+        expect(
+          (row?.provider_state as { oauthReauthRequiredAt?: number } | null)?.oauthReauthRequiredAt,
+        ).toEqual(expect.any(Number));
+        expect(row?.last_health).toMatchObject({ status: "expired" });
+
+        const grantRequests = () =>
+          server.requests.pipe(
+            Effect.map(
+              (all) =>
+                all.filter((r) => r.path === "/token" && r.body.includes("refresh_token")).length,
+            ),
+          );
+        const sentBefore = yield* grantRequests();
+        expect(sentBefore).toBe(1);
+
+        // Later resolves still fail reauth-required, but WITHOUT re-sending
+        // the dead grant: the token endpoint sees no further traffic.
+        const second = yield* Effect.flip(
+          executor.execute(ToolAddress.make("tools.acme.org.main.whoami"), {}),
+        );
+        expect(JSON.stringify(second)).toContain("Reconnect");
+        expect(yield* grantRequests()).toBe(sentBefore);
+
+        // Reconnecting mints a fresh grant and re-arms refresh: the marker is
+        // gone and resolution works again.
+        const restarted = yield* executor.oauth.start({
+          owner: "org",
+          client: CLIENT,
+          clientOwner: "org",
+          name: ConnectionName.make("main"),
+          integration: INTEG,
+          template: TEMPLATE,
+        });
+        expect(restarted.status).toBe("redirect");
+        if (restarted.status !== "redirect") return;
+        const reCallback = yield* server.completeAuthorizationCodeFlow({
+          authorizationUrl: restarted.authorizationUrl,
+        });
+        yield* executor.oauth.complete({ state: restarted.state, code: reCallback.code });
+
+        const cleared = yield* Effect.promise(() =>
+          config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }),
+        );
+        expect(
+          (cleared?.provider_state as { oauthReauthRequiredAt?: number } | null)
+            ?.oauthReauthRequiredAt,
+        ).toBeUndefined();
+      }),
+    ),
+  );
+
   it.effect(
     "checkHealth reports healthy from OAuth credential resolution when no probe is configured",
     () =>

packages/core/sdk/src/oauth-helpers.test.ts

@@ -871,6 +871,68 @@ describe("refreshAccessToken", () => {
       }),
     ),
   );
+
+  // Datadog answers refresh grants with a non-conform envelope; the §5.2 code
+  // must still be recovered so invalid_grant classifies as reauth-required
+  // instead of a retried-forever transient (owner.com prod, 2026-07-30).
+  it.effect("recovers invalid_grant from Datadog's non-conform errors array", () =>
+    withTokenEndpoint(
+      () =>
+        json(400, {
+          errors: ["invalid_grant - Invalid or expired refresh token or code verifier."],
+        }),
+      ({ tokenUrl }) =>
+        Effect.gen(function* () {
+          const error = yield* Effect.flip(
+            refreshAccessToken({ tokenUrl, clientId: "cid", refreshToken: "old" }),
+          );
+          expect(error).toBeInstanceOf(OAuth2Error);
+          expect((error as OAuth2Error).error).toBe("invalid_grant");
+        }),
+    ),
+  );
+
+  it.effect("recovers a bare non-conform `error` string outside the spec envelope shape", () =>
+    withTokenEndpoint(
+      () => json(400, { error: "invalid_grant", detail: 42 }),
+      ({ tokenUrl }) =>
+        Effect.gen(function* () {
+          const error = yield* Effect.flip(
+            refreshAccessToken({ tokenUrl, clientId: "cid", refreshToken: "old" }),
+          );
+          expect(error).toBeInstanceOf(OAuth2Error);
+          expect((error as OAuth2Error).error).toBe("invalid_grant");
+        }),
+    ),
+  );
+
+  it.effect("does not invent a code from free-text error bodies", () =>
+    withTokenEndpoint(
+      () => json(400, { errors: ["something went wrong, try again later"] }),
+      ({ tokenUrl }) =>
+        Effect.gen(function* () {
+          const error = yield* Effect.flip(
+            refreshAccessToken({ tokenUrl, clientId: "cid", refreshToken: "old" }),
+          );
+          expect(error).toBeInstanceOf(OAuth2Error);
+          expect((error as OAuth2Error).error).toBeUndefined();
+        }),
+    ),
+  );
+
+  it.effect("does not probe non-conform bodies on 5xx responses", () =>
+    withTokenEndpoint(
+      () => json(502, { errors: ["invalid_grant - upstream proxy noise"] }),
+      ({ tokenUrl }) =>
+        Effect.gen(function* () {
+          const error = yield* Effect.flip(
+            refreshAccessToken({ tokenUrl, clientId: "cid", refreshToken: "old" }),
+          );
+          expect(error).toBeInstanceOf(OAuth2Error);
+          expect((error as OAuth2Error).error).toBeUndefined();
+        }),
+    ),
+  );
 });
 
 describe("shouldRefreshToken", () => {