From fd4fb907619a4da9ec08102b77b928135a6b32d6 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:35:04 -0700 Subject: [PATCH 1/2] Surface AS-rejected token refreshes as actionable auth failures A refresh-token grant the authorization server rejects with an RFC 6749 error code other than invalid_grant was classified as a StorageError, which the sandbox boundary scrubs to "Internal tool error [id]" - the caller had no signal that the connection needed re-authentication. Any refresh rejection carrying an RFC 6749 error code now maps to CredentialResolutionError, so it reaches the sandbox as an oauth_refresh_failed auth failure with the server's error code and description (invalid_grant still maps to oauth_reauth_required). Code-less failures (transport blips, non-OAuth-shaped responses) stay StorageError so the next invoke retries. --- .changeset/oauth-refresh-rejection-surface.md | 6 + e2e/scenarios/oauth-refresh-rejected.test.ts | 272 ++++++++++++++++++ .../core/execution/src/tool-invoker.test.ts | 89 ++++++ packages/core/sdk/src/executor.ts | 40 ++- .../core/sdk/src/testing/oauth-test-server.ts | 7 +- 5 files changed, 401 insertions(+), 13 deletions(-) create mode 100644 .changeset/oauth-refresh-rejection-surface.md create mode 100644 e2e/scenarios/oauth-refresh-rejected.test.ts diff --git a/.changeset/oauth-refresh-rejection-surface.md b/.changeset/oauth-refresh-rejection-surface.md new file mode 100644 index 000000000..c67223fc1 --- /dev/null +++ b/.changeset/oauth-refresh-rejection-surface.md @@ -0,0 +1,6 @@ +--- +"@executor-js/sdk": patch +"@executor-js/execution": patch +--- + +A token refresh the authorization server definitively rejects (any RFC 6749 error code, not just `invalid_grant`) now surfaces to the sandbox as an `oauth_refresh_failed` auth failure carrying the server's error code and description, instead of being scrubbed to "Internal tool error". `invalid_grant` still classifies as `oauth_reauth_required`. Code-less failures (transport blips) keep retrying as before. diff --git a/e2e/scenarios/oauth-refresh-rejected.test.ts b/e2e/scenarios/oauth-refresh-rejected.test.ts new file mode 100644 index 000000000..0fe868e3d --- /dev/null +++ b/e2e/scenarios/oauth-refresh-rejected.test.ts @@ -0,0 +1,272 @@ +// Cross-target: when the authorization server definitively rejects a +// refresh-token grant, the sandbox must see an auth failure it can act on — +// never the scrubbed "Internal tool error [hex]" defect. +// +// Prod regression (2026-07-16): an AS answered refresh grants with a 400 +// whose RFC 6749 error code was NOT `invalid_grant`. Executor classified +// that as a storage failure, and the opaque-defect boundary scrubbed it to +// "Internal tool error [id]" — so agents (and the humans driving them) had +// no signal that the connection simply needed to be re-authenticated. +// +// The journey: an OpenAPI integration completes a real authorization-code +// flow against a live test AS that mints instantly-expiring access tokens +// and rejects every refresh grant with `invalid_request`. The first tool +// call must therefore refresh, the AS says a definitive no, and the failure +// the sandbox sees carries code `oauth_refresh_failed`, the AS's own error +// code and description, and connection-recovery guidance. +import { randomBytes } from "node:crypto"; +import { createServer } from "node:http"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, +} from "@executor-js/sdk/shared"; +import { serveOAuthTestServer } from "@executor-js/sdk/testing"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +/** Upstream on 127.0.0.1: `GET /issues` is 200 for any bearer. The refresh is + * rejected before any upstream call, so this only proves the failure came + * from the token endpoint, not from here. */ +const serveUpstream = () => + Effect.acquireRelease( + Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => { + const server = createServer((request, response) => { + if (request.method === "GET" && (request.url ?? "").startsWith("/issues")) { + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ issues: [] })); + return; + } + response.writeHead(404, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: "not_found" })); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resume( + Effect.succeed({ + url: `http://127.0.0.1:${port}`, + close: () => { + server.close(); + server.closeAllConnections(); + }, + }), + ); + }); + }), + (server) => Effect.sync(server.close), + ); + +const spec = ( + baseUrl: string, + oauth: { readonly authorizationEndpoint: string; readonly tokenEndpoint: string }, +): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Issues API", version: "1.0.0" }, + servers: [{ url: baseUrl }], + paths: { + "/issues": { + get: { + operationId: "listIssues", + summary: "List issues", + security: [{ oauth: ["issues.read"] }], + responses: { "200": { description: "issues" } }, + }, + }, + }, + components: { + securitySchemes: { + oauth: { + type: "oauth2", + flows: { + authorizationCode: { + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + scopes: { "issues.read": "Read issues" }, + }, + }, + }, + }, + }, + }); + +const invokeByAddressCode = (address: string, args: unknown) => ` +const segments = ${JSON.stringify(address)}.split(".").slice(1); +let node = tools; +for (const segment of segments) node = node[segment]; +const result = await node(${JSON.stringify(args)}); +return JSON.stringify(result); +`; + +type ToolEnvelope = { + readonly ok: boolean; + readonly error?: { + readonly code?: string; + readonly message?: string; + readonly details?: { + readonly category?: string; + readonly recovery?: Record; + }; + }; +}; + +scenario( + "Auth failures · a refresh rejected by the authorization server surfaces as an actionable auth failure, not an internal error", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const upstream = yield* serveUpstream(); + // Instantly-expiring access tokens force the first tool call to + // refresh; the AS rejects every refresh grant with a non-invalid_grant + // 400, mirroring the prod AS that triggered the regression. + const oauth = yield* serveOAuthTestServer({ + scopes: ["issues.read"], + tokenExpiresInSeconds: 0, + supportRefresh: false, + invalidRefreshTokenErrorCode: "invalid_request", + invalidRefreshTokenDescription: "Refresh token expired", + }); + const slug = unique("refreshrej"); + const clientSlug = OAuthClientSlug.make(unique("refreshrejc")); + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: spec(upstream.url, oauth) }, + slug, + baseUrl: upstream.url, + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2", + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + scopes: ["issues.read"], + }, + ], + }, + }); + yield* client.oauth.createClient({ + payload: { + owner: "org", + slug: clientSlug, + grant: "authorization_code", + authorizationUrl: oauth.authorizationEndpoint, + tokenUrl: oauth.tokenEndpoint, + clientId: "test-client", + clientSecret: "test-secret", + originIntegration: IntegrationSlug.make(slug), + }, + }); + + const started = yield* client.oauth.start({ + payload: { + client: clientSlug, + clientOwner: "org", + owner: "org", + name: ConnectionName.make("main"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("oauth"), + }, + }); + expect(started.status, "oauth.start redirects to the authorization server").toBe( + "redirect", + ); + if (started.status !== "redirect") return yield* Effect.die("no redirect"); + + // Drive the test IdP's consent by hand (authorize → login → code). + const code = yield* Effect.promise(async () => { + const authorize = await fetch(started.authorizationUrl, { redirect: "manual" }); + const loginUrl = authorize.headers.get("location"); + if (!loginUrl) throw new Error(`authorize did not redirect: ${authorize.status}`); + const login = await fetch(loginUrl, { + method: "POST", + headers: { + authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`, + }, + redirect: "manual", + }); + const callbackUrl = login.headers.get("location"); + if (!callbackUrl) throw new Error(`login did not redirect: ${login.status}`); + const minted = new URL(callbackUrl).searchParams.get("code"); + if (!minted) throw new Error("callback carried no authorization code"); + return minted; + }); + yield* client.oauth.complete({ payload: { state: started.state, code } }); + + const tools = yield* client.tools.list({ query: {} }); + const address = tools + .filter((tool) => String(tool.integration) === slug) + .map((tool) => String(tool.address)) + .find((addr) => addr.endsWith("listIssues")); + expect(address, "the listIssues tool is in the catalog").toBeDefined(); + + const executed = yield* client.executions.execute({ + payload: { code: invokeByAddressCode(address!, {}), autoApprove: true }, + }); + expect(executed.status, "the sandbox execution completed").toBe("completed"); + const failure = JSON.parse(executed.text) as ToolEnvelope; + + // THE guarantee: the AS's definitive rejection reaches the agent + // as an auth failure with the AS's own verdict in the message. + expect(failure.ok, "the tool call failed").toBe(false); + expect( + failure.error?.code, + "the failure is classified as a rejected refresh, not an internal defect", + ).toBe("oauth_refresh_failed"); + expect( + failure.error?.message ?? "", + "the message carries the AS's RFC 6749 error code", + ).toContain("invalid_request"); + expect( + failure.error?.message ?? "", + "the message carries the AS's error description", + ).toContain("Refresh token expired"); + expect( + failure.error?.message ?? "", + "the opaque defect message never reaches the sandbox", + ).not.toContain("Internal tool error"); + expect(failure.error?.details?.category, "the failure is auth-flavored").toBe( + "authentication", + ); + expect( + failure.error?.details?.recovery?.oauthInstructions ?? "", + "the recovery tells the agent how to re-connect via OAuth", + ).toContain("startOAuthTool"); + }), + Effect.gen(function* () { + yield* client.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("main"), + }, + }) + .pipe(Effect.ignore); + yield* client.oauth + .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), + ), +); diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index 407667d43..0a8018b0d 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -1285,6 +1285,95 @@ describe("tool discovery", () => { ), ); + // Prod regression (Pylon, 2026-07-16): the AS rejected refresh grants with a + // 400 whose error code was NOT invalid_grant. That is a definitive "no" from + // the AS, but it surfaced to the sandbox as an opaque "Internal tool error + // [hex]" — the caller had no way to know the connection needed re-auth. + it.effect("returns AS-rejected token refreshes (non-invalid_grant) as ToolResult.fail", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ + scopes: ["read"], + supportRefresh: false, + invalidRefreshTokenErrorCode: "invalid_request", + invalidRefreshTokenDescription: "Refresh token expired", + }); + const config = makeTestConfig({ + plugins: [memoryCredentialsPlugin(), oauthErrorPlugin] as const, + }); + const executor = yield* createExecutor(config); + yield* executor["oauth-error-test"].seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: OAUTH_CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "test-client", + clientSecret: "test-secret", + }); + + const started = yield* executor.oauth.start({ + owner: "org", + client: OAUTH_CLIENT, + clientOwner: "org", + name: CONN, + integration: IntegrationSlug.make("oauth_records"), + template: OAUTH_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 }); + + // Expire the access token so the next resolve fires a real + // refresh-token grant against the live test AS, which rejects it. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => + b.and(b("integration", "=", "oauth_records"), b("name", "=", String(CONN))), + set: { expires_at: Date.now() - 60_000 }, + }), + ); + + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + const result = yield* invoker.invoke({ + path: "oauth_records.org.main.queryRows", + args: {}, + }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_refresh_failed", + details: { + category: "authentication", + credential: { + kind: "oauth", + label: "oauth_records.org.main", + }, + }, + retryable: false, + }, + }); + const failure = result as { + readonly ok: false; + readonly error: { readonly message: string }; + }; + // The AS's verdict (code + description) reaches the caller verbatim — + // never the scrubbed "Internal tool error [hex]". + expect(failure.error.message).toContain("invalid_request"); + expect(failure.error.message).toContain("Refresh token expired"); + expect(failure.error.message).not.toContain("Internal tool error"); + }), + ), + ); + it.effect("returns missing tool dispatches as ToolResult.fail", () => Effect.gen(function* () { const executor = yield* makeExecutorWith([] as const); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 36921b1cb..42dee8cd1 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1592,18 +1592,34 @@ export const createExecutor = - cause.error === "invalid_grant" - ? reauth( - // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` - `OAuth token refresh was rejected (invalid_grant): ${cause.message}`, - ) - : new StorageError({ - // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` - message: `OAuth token refresh failed: ${cause.message}`, - cause, - }), - ), + Effect.mapError((cause) => { + // An RFC 6749 §5.2 error code is the AS's definitive + // verdict on this grant — retrying cannot change it. + // invalid_grant means the refresh token itself is dead + // (re-auth required); every other code must still reach + // the caller as an auth failure, because a StorageError + // is scrubbed to "Internal tool error [id]" at the + // sandbox boundary (the Pylon prod regression: the AS + // rejected refreshes with a non-invalid_grant 400 and + // callers saw only the opaque defect). Code-less + // failures (transport blips, non-OAuth-shaped responses) + // stay StorageError so the next invoke retries. + if (cause.error !== undefined) { + return new CredentialResolutionError({ + owner, + integration: IntegrationSlug.make(row.integration), + name: ConnectionName.make(row.name), + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` + message: `OAuth token refresh was rejected (${cause.error}): ${cause.message}`, + reauthRequired: cause.error === "invalid_grant", + }); + } + return new StorageError({ + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message` + message: `OAuth token refresh failed: ${cause.message}`, + cause, + }); + }), ); }); diff --git a/packages/core/sdk/src/testing/oauth-test-server.ts b/packages/core/sdk/src/testing/oauth-test-server.ts index cfaaac6bc..17413f5f4 100644 --- a/packages/core/sdk/src/testing/oauth-test-server.ts +++ b/packages/core/sdk/src/testing/oauth-test-server.ts @@ -55,6 +55,10 @@ export interface OAuthTestServerOptions { readonly supportRefresh?: boolean; readonly tokenExpiresInSeconds?: number; readonly invalidRefreshTokenDescription?: string; + /** RFC 6749 error code returned when a refresh-token grant is rejected. + * Defaults to `invalid_grant`; set to e.g. `invalid_request` to mirror + * authorization servers that reject dead refresh tokens with other codes. */ + readonly invalidRefreshTokenErrorCode?: string; readonly idTokenClaims?: Readonly>; readonly refreshIdTokenClaims?: Readonly>; /** Gate Dynamic Client Registration on the requested redirect URIs. When set, @@ -426,6 +430,7 @@ export const serveOAuthTestServer = ( const tokenExpiresInSeconds = options.tokenExpiresInSeconds ?? 3600; const invalidRefreshTokenDescription = options.invalidRefreshTokenDescription ?? "Unknown refresh token"; + const invalidRefreshTokenErrorCode = options.invalidRefreshTokenErrorCode ?? "invalid_grant"; const scopes = options.scopes ?? defaultScopes; const omittedTokenResponseScopes = new Set(options.omitTokenResponseScopes ?? []); const tokenResponseScope = (scope: string | null): string | undefined => { @@ -671,7 +676,7 @@ export const serveOAuthTestServer = ( const refreshToken = params.get("refresh_token"); const record = refreshToken ? refreshTokens.get(refreshToken) : undefined; if (!supportRefresh || !refreshToken || !record || record.clientId !== clientId) { - return oauthError(400, "invalid_grant", invalidRefreshTokenDescription); + return oauthError(400, invalidRefreshTokenErrorCode, invalidRefreshTokenDescription); } const nextAccessToken = `at_${randomUUID()}`; const nextRefreshToken = `rt_${randomUUID()}`; From 35d0586b441418c0599aac3ea9edfd179d9f0f6d Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:14:23 -0700 Subject: [PATCH 2/2] Assert the rejected-refresh failure over the MCP surface The scenario now invokes the tool through a real MCP session (the channel the regression actually hid the failure on) instead of the executions API, so it covers dispatch, classification, and MCP result rendering end to end. --- e2e/scenarios/oauth-refresh-rejected.test.ts | 24 ++++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/e2e/scenarios/oauth-refresh-rejected.test.ts b/e2e/scenarios/oauth-refresh-rejected.test.ts index 0fe868e3d..e71ec3922 100644 --- a/e2e/scenarios/oauth-refresh-rejected.test.ts +++ b/e2e/scenarios/oauth-refresh-rejected.test.ts @@ -30,7 +30,7 @@ import { import { serveOAuthTestServer } from "@executor-js/sdk/testing"; import { scenario } from "../src/scenario"; -import { Api, Target } from "../src/services"; +import { Api, Mcp, Target } from "../src/services"; const api = composePluginApi([openApiHttpPlugin()] as const); @@ -129,6 +129,7 @@ scenario( Effect.gen(function* () { const target = yield* Target; const { client: makeClient } = yield* Api; + const mcp = yield* Mcp; const identity = yield* target.newIdentity(); const client = yield* makeClient(api, identity); const upstream = yield* serveUpstream(); @@ -218,11 +219,24 @@ scenario( .find((addr) => addr.endsWith("listIssues")); expect(address, "the listIssues tool is in the catalog").toBeDefined(); - const executed = yield* client.executions.execute({ - payload: { code: invokeByAddressCode(address!, {}), autoApprove: true }, + // Call through the real MCP surface — the exact channel the prod + // regression hid the failure on — so the assertion covers the whole + // path an MCP client sees: dispatch, classification, rendering. + const session = mcp.session(identity); + let called = yield* session.call("execute", { + code: invokeByAddressCode(address!, {}), }); - expect(executed.status, "the sandbox execution completed").toBe("completed"); - const failure = JSON.parse(executed.text) as ToolEnvelope; + // Approval-gated tools pause the execution once per gated call. + let guard = 0; + while (called.text.includes("executionId:") && guard < 10) { + called = yield* session.approvePaused(called.text); + guard += 1; + } + expect( + called.ok, + `the MCP execute call itself completed (got: ${called.text.slice(0, 400)})`, + ).toBe(true); + const failure = JSON.parse(called.text) as ToolEnvelope; // THE guarantee: the AS's definitive rejection reaches the agent // as an auth failure with the AS's own verdict in the message.