From fe5a4fdebd1352b11a2a30848f9255dbda4cba1c Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 20 May 2026 01:01:21 -0700 Subject: [PATCH 1/2] surface auth tool failures --- packages/core/sdk/src/auth-tool-failure.ts | 62 +++++ packages/core/sdk/src/index.ts | 5 + packages/plugins/graphql/src/sdk/errors.ts | 16 ++ .../plugins/graphql/src/sdk/plugin.test.ts | 36 +++ packages/plugins/graphql/src/sdk/plugin.ts | 199 +++++++++++++--- packages/plugins/mcp/src/sdk/errors.ts | 18 +- packages/plugins/mcp/src/sdk/invoke.ts | 20 +- .../src/sdk/per-user-auth-isolation.test.ts | 65 ++--- packages/plugins/mcp/src/sdk/plugin.ts | 224 +++++++++++++++--- packages/plugins/openapi/src/sdk/errors.ts | 16 ++ .../src/sdk/multi-scope-bearer.test.ts | 23 +- .../openapi/src/sdk/oauth-refresh.test.ts | 23 +- .../plugins/openapi/src/sdk/plugin.test.ts | 22 +- packages/plugins/openapi/src/sdk/plugin.ts | 192 +++++++++++++-- .../openapi/src/sdk/upstream-failures.test.ts | 28 +++ 15 files changed, 809 insertions(+), 140 deletions(-) create mode 100644 packages/core/sdk/src/auth-tool-failure.ts diff --git a/packages/core/sdk/src/auth-tool-failure.ts b/packages/core/sdk/src/auth-tool-failure.ts new file mode 100644 index 000000000..b43fad8a6 --- /dev/null +++ b/packages/core/sdk/src/auth-tool-failure.ts @@ -0,0 +1,62 @@ +import { ToolResult, type ToolError } from "./tool-result"; + +export type AuthToolFailureCode = + | "credential_binding_missing" + | "credential_secret_missing" + | "credential_rejected" + | "oauth_connection_missing" + | "oauth_connection_failed" + | "oauth_reauth_required"; + +export type AuthToolFailureInput = { + readonly code: AuthToolFailureCode; + readonly message: string; + readonly source?: { + readonly id: string; + readonly scope?: string; + }; + readonly credential?: { + readonly kind: "secret" | "connection" | "oauth" | "upstream"; + readonly label?: string; + readonly slotKey?: string; + readonly secretId?: string; + readonly connectionId?: string; + }; + readonly status?: number; + readonly upstream?: { + readonly status?: number; + readonly details?: unknown; + }; + readonly recovery?: { + readonly configureSourceTool?: string; + }; +}; + +const authRecovery = (input?: AuthToolFailureInput["recovery"]) => ({ + secretsUrl: "https://executor.sh/secrets", + createSecretTool: "executor.coreTools.secrets.create", + startOAuthTool: "executor.coreTools.oauth.start", + listConnectionsTool: "executor.coreTools.connections.list", + ...(input?.configureSourceTool ? { configureSourceTool: input.configureSourceTool } : {}), + secretInstructions: + "For API keys, tokens, and other manually entered credentials, call createSecretTool and give the returned browser URL to the user before configuring the source binding.", + oauthInstructions: + "For OAuth credentials, call startOAuthTool and give the returned authorizationUrl to the user, then bind the completed connection with the source configuration tool.", +}); + +export const authToolFailure = (input: AuthToolFailureInput): ToolResult => { + const error: ToolError = { + code: input.code, + message: input.message, + retryable: false, + ...(input.status !== undefined ? { status: input.status } : {}), + details: { + category: "authentication", + ...(input.source ? { source: input.source } : {}), + ...(input.credential ? { credential: input.credential } : {}), + ...(input.upstream ? { upstream: input.upstream } : {}), + recovery: authRecovery(input.recovery), + }, + }; + return ToolResult.fail(error); +}; diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 2d995f8f6..6b23064bb 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -363,3 +363,8 @@ export { InternalError } from "./api-errors"; // TypeScript permits the two to share a name because one is purely a // value and the other purely a type. export { ToolResult, isToolResult, type ToolError } from "./tool-result"; +export { + authToolFailure, + type AuthToolFailureCode, + type AuthToolFailureInput, +} from "./auth-tool-failure"; diff --git a/packages/plugins/graphql/src/sdk/errors.ts b/packages/plugins/graphql/src/sdk/errors.ts index cee3a7354..ae724d158 100644 --- a/packages/plugins/graphql/src/sdk/errors.ts +++ b/packages/plugins/graphql/src/sdk/errors.ts @@ -1,5 +1,6 @@ import { Data, Schema } from "effect"; import type { Option } from "effect"; +import type { AuthToolFailureCode } from "@executor-js/sdk/core"; export class GraphqlIntrospectionError extends Schema.TaggedErrorClass()( "GraphqlIntrospectionError", @@ -20,3 +21,18 @@ export class GraphqlInvocationError extends Data.TaggedError("GraphqlInvocationE readonly statusCode: Option.Option; readonly cause?: unknown; }> {} + +export class GraphqlAuthRequiredError extends Data.TaggedError("GraphqlAuthRequiredError")<{ + readonly code: AuthToolFailureCode; + readonly message: string; + readonly sourceId: string; + readonly sourceScope: string; + readonly credentialKind: "secret" | "connection" | "oauth" | "upstream"; + readonly credentialLabel?: string; + readonly slotKey?: string; + readonly secretId?: string; + readonly connectionId?: string; + readonly status?: number; + readonly details?: unknown; + readonly cause?: unknown; +}> {} diff --git a/packages/plugins/graphql/src/sdk/plugin.test.ts b/packages/plugins/graphql/src/sdk/plugin.test.ts index 9eb8766d1..67c003028 100644 --- a/packages/plugins/graphql/src/sdk/plugin.test.ts +++ b/packages/plugins/graphql/src/sdk/plugin.test.ts @@ -607,6 +607,42 @@ describe("graphqlPlugin real protocol server", () => { expect(requests[0]?.headers.authorization).toBe("Bearer secret-token"); }), ); + + it.effect("returns an auth failure when an OAuth-backed source has no connection binding", () => + Effect.gen(function* () { + const server = yield* serveGreetingServer; + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [memorySecretsPlugin(), graphqlPlugin()] as const, + }), + ); + + yield* executor.graphql.addSource({ + endpoint: server.endpoint, + scope: TEST_SCOPE, + namespace: "oauth_missing_connection", + oauth2: graphqlOAuth2Config, + }); + + const result = yield* executor.tools.invoke("oauth_missing_connection.query.hello", { + name: "Ada", + }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "oauth_connection_missing", + message: expect.stringContaining("Missing OAuth connection binding"), + details: { + category: "authentication", + recovery: { + startOAuthTool: "executor.coreTools.oauth.start", + }, + }, + }, + }); + }), + ); }); describe("graphqlPlugin", () => { diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 6496e102d..4d571ddb4 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -12,6 +12,7 @@ import { SourceDetectionResult, StorageError, ToolResult, + authToolFailure, type PluginCtx, type StorageFailure, type ToolAnnotations, @@ -39,7 +40,11 @@ import { type IntrospectionTypeRef, } from "./introspect"; import { extract } from "./extract"; -import { GraphqlIntrospectionError, GraphqlInvocationError } from "./errors"; +import { + GraphqlAuthRequiredError, + GraphqlIntrospectionError, + GraphqlInvocationError, +} from "./errors"; import { invokeWithLayer } from "./invoke"; import { graphqlPresets } from "./presets"; import { @@ -192,6 +197,30 @@ const graphqlToolFailure = (code: string, message: string, details?: unknown) => ...(details === undefined ? {} : { details }), }); +const graphqlAuthToolFailure = (failure: GraphqlAuthRequiredError) => + authToolFailure({ + code: failure.code, + message: failure.message, + source: { id: failure.sourceId, scope: failure.sourceScope }, + credential: { + kind: failure.credentialKind, + ...(failure.credentialLabel ? { label: failure.credentialLabel } : {}), + ...(failure.slotKey ? { slotKey: failure.slotKey } : {}), + ...(failure.secretId ? { secretId: failure.secretId } : {}), + ...(failure.connectionId ? { connectionId: failure.connectionId } : {}), + }, + ...(failure.status !== undefined ? { status: failure.status } : {}), + ...(failure.details !== undefined + ? { + upstream: { + ...(failure.status !== undefined ? { status: failure.status } : {}), + details: failure.details, + }, + } + : {}), + recovery: { configureSourceTool: "executor.graphql.configureSource" }, + }); + const resolveStaticScopeInput = ( ctx: { readonly scopes: readonly { readonly id: ScopeId; readonly name: string }[] }, value: string, @@ -609,16 +638,15 @@ const resolveInitialOAuthHeaders = ( return { Authorization: `Bearer ${accessToken}` }; }); -const resolveGraphqlBindingValueMap = ( +const resolveGraphqlBindingValueMap = ( ctx: PluginCtx, values: Record | undefined, params: { readonly sourceId: string; readonly sourceScope: string; readonly missingLabel: string; - readonly makeError: (message: string) => E; }, -): Effect.Effect | undefined, E | StorageFailure> => +): Effect.Effect | undefined, GraphqlAuthRequiredError | StorageFailure> => Effect.gen(function* () { if (!values) return undefined; const resolved: Record = {}; @@ -634,21 +662,34 @@ const resolveGraphqlBindingValueMap = ( value.slot, ); if (binding?.value.kind === "secret") { - const secret = yield* ctx.secrets - .getAtScope(binding.value.secretId, binding.scopeId) - .pipe( - Effect.catchTag("SecretOwnedByConnectionError", () => - Effect.fail( - params.makeError(`Secret not found for ${params.missingLabel} "${name}"`), - ), + const secretBinding = binding.value; + const secret = yield* ctx.secrets.getAtScope(secretBinding.secretId, binding.scopeId).pipe( + Effect.catchTag("SecretOwnedByConnectionError", () => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), + message: `Secret not found for ${params.missingLabel} "${name}"`, + }), ), - ); + ), + ); if (secret === null) { - return yield* Effect.fail( - params.makeError( - `Missing secret "${binding.value.secretId}" for ${params.missingLabel} "${name}"`, - ), - ); + return yield* new GraphqlAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), + message: `Missing secret "${secretBinding.secretId}" for ${params.missingLabel} "${name}"`, + }); } resolved[name] = value.prefix ? `${value.prefix}${secret}` : secret; continue; @@ -657,9 +698,15 @@ const resolveGraphqlBindingValueMap = ( resolved[name] = value.prefix ? `${value.prefix}${binding.value.text}` : binding.value.text; continue; } - return yield* Effect.fail( - params.makeError(`Missing binding for ${params.missingLabel} "${name}"`), - ); + return yield* new GraphqlAuthRequiredError({ + code: "credential_binding_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + message: `Missing binding for ${params.missingLabel} "${name}"`, + }); } return Object.keys(resolved).length > 0 ? resolved : undefined; }); @@ -679,15 +726,88 @@ const resolveGraphqlStoredOAuthHeader = ( auth.connectionSlot, ); if (binding?.value.kind !== "connection") { - return yield* new GraphqlInvocationError({ + return yield* new GraphqlAuthRequiredError({ + code: "oauth_connection_missing", + sourceId, + sourceScope, + credentialKind: "connection", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, message: `Missing OAuth connection binding for GraphQL source "${sourceId}"`, - statusCode: Option.none(), }); } - const accessToken = yield* ctx.connections.accessTokenAtScope( - binding.value.connectionId, - binding.scopeId, - ); + const connectionId = binding.value.connectionId; + const accessToken = yield* ctx.connections + .accessTokenAtScope(connectionId, binding.scopeId) + .pipe( + Effect.catchTags({ + ConnectionReauthRequiredError: ({ message, connectionId }) => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "oauth_reauth_required", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" needs re-authentication: ${message}`, + }), + ), + ConnectionNotFoundError: ({ connectionId }) => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "oauth_connection_missing", + sourceId, + sourceScope, + credentialKind: "connection", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" was not found for GraphQL source "${sourceId}"`, + }), + ), + ConnectionProviderNotRegisteredError: ({ provider }) => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth provider "${provider}" is not registered`, + }), + ), + ConnectionRefreshNotSupportedError: ({ provider, connectionId }) => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth provider "${provider}" cannot refresh connection "${connectionId}"`, + }), + ), + ConnectionRefreshError: ({ message, connectionId }) => + Effect.fail( + new GraphqlAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" refresh failed: ${message}`, + }), + ), + }), + ); return { Authorization: `Bearer ${accessToken}` }; }); @@ -1082,16 +1202,12 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { sourceId: source.namespace, sourceScope: source.scope, missingLabel: "header", - makeError: (message) => - new GraphqlInvocationError({ message, statusCode: Option.none() }), })) ?? {}; const resolvedQueryParams = (yield* resolveGraphqlBindingValueMap(ctx, source.queryParams, { sourceId: source.namespace, sourceScope: source.scope, missingLabel: "query parameter", - makeError: (message) => - new GraphqlInvocationError({ message, statusCode: Option.none() }), })) ?? {}; const oauthHeader = yield* resolveGraphqlStoredOAuthHeader( ctx, @@ -1120,6 +1236,23 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { }); } if (result.status < 200 || result.status >= 300) { + if (result.status === 401 || result.status === 403) { + return authToolFailure({ + code: "credential_rejected", + status: result.status, + message: `Upstream rejected credentials for GraphQL source "${source.namespace}" with HTTP ${result.status}. Re-authenticate or update the source credentials before retrying this tool.`, + source: { id: source.namespace, scope: source.scope }, + credential: { kind: "upstream", label: "Upstream authorization" }, + upstream: { + status: result.status, + details: { + data: result.data, + errors: result.errors, + }, + }, + recovery: { configureSourceTool: "executor.graphql.configureSource" }, + }); + } return ToolResult.fail({ code: "graphql_http_error", status: result.status, @@ -1132,7 +1265,11 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { }); } return ToolResult.ok(result.data); - }), + }).pipe( + Effect.catchTag("GraphqlAuthRequiredError", (error) => + Effect.succeed(graphqlAuthToolFailure(error)), + ), + ), resolveAnnotations: ({ ctx, sourceId, toolRows }) => Effect.gen(function* () { diff --git a/packages/plugins/mcp/src/sdk/errors.ts b/packages/plugins/mcp/src/sdk/errors.ts index 4e64584c4..15faef09e 100644 --- a/packages/plugins/mcp/src/sdk/errors.ts +++ b/packages/plugins/mcp/src/sdk/errors.ts @@ -3,7 +3,8 @@ // these and HttpApi encodes them as 4xx responses with a typed body. No // per-handler sanitisation step. -import { Schema } from "effect"; +import { Data, Schema } from "effect"; +import type { AuthToolFailureCode } from "@executor-js/sdk/core"; export class McpConnectionError extends Schema.TaggedErrorClass()( "McpConnectionError", @@ -39,3 +40,18 @@ export class McpOAuthError extends Schema.TaggedErrorClass()( }, { httpApiStatus: 400 }, ) {} + +export class McpAuthRequiredError extends Data.TaggedError("McpAuthRequiredError")<{ + readonly code: AuthToolFailureCode; + readonly message: string; + readonly sourceId: string; + readonly sourceScope: string; + readonly credentialKind: "secret" | "connection" | "oauth" | "upstream"; + readonly credentialLabel?: string; + readonly slotKey?: string; + readonly secretId?: string; + readonly connectionId?: string; + readonly status?: number; + readonly details?: unknown; + readonly cause?: unknown; +}> {} diff --git a/packages/plugins/mcp/src/sdk/invoke.ts b/packages/plugins/mcp/src/sdk/invoke.ts index f0cf8b4a4..468737892 100644 --- a/packages/plugins/mcp/src/sdk/invoke.ts +++ b/packages/plugins/mcp/src/sdk/invoke.ts @@ -21,7 +21,7 @@ import { type ElicitationRequest, } from "@executor-js/sdk/core"; -import { McpConnectionError, McpInvocationError } from "./errors"; +import { McpAuthRequiredError, McpConnectionError, McpInvocationError } from "./errors"; import type { McpConnection } from "./connection"; import type { McpStoredSourceData } from "./types"; @@ -190,15 +190,25 @@ export interface InvokeMcpToolInput { * connection cache key so per-user OAuth/secret resolution doesn't * collapse multiple users onto one shared connection. */ readonly invokerScope: string; - readonly resolveConnector: () => Effect.Effect; - readonly connectionCache: ScopedCache.ScopedCache; - readonly pendingConnectors: Map>; + readonly resolveConnector: () => Effect.Effect< + McpConnection, + McpAuthRequiredError | McpConnectionError + >; + readonly connectionCache: ScopedCache.ScopedCache< + string, + McpConnection, + McpAuthRequiredError | McpConnectionError + >; + readonly pendingConnectors: Map< + string, + Effect.Effect + >; readonly elicit: Elicit; } export const invokeMcpTool = ( input: InvokeMcpToolInput, -): Effect.Effect => { +): Effect.Effect => { const transport: string = input.sourceData.transport === "stdio" ? "stdio" : (input.sourceData.remoteTransport ?? "auto"); return Effect.gen(function* () { diff --git a/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts b/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts index fc2f4c936..07557439e 100644 --- a/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts +++ b/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { Cause, Effect, Exit, Predicate } from "effect"; +import { Effect } from "effect"; import { ConnectionId, @@ -11,7 +11,6 @@ import { SetSecretInput, TokenMaterial, createExecutor, - type ToolInvocationError, } from "@executor-js/sdk"; import { makeTestConfig, memorySecretsPlugin } from "@executor-js/sdk/testing"; @@ -40,12 +39,6 @@ const mcpOAuth2Config = { const scope = (id: ScopeId, name: string): Scope => Scope.make({ id, name, createdAt: new Date() }); -const failureError = (exit: Exit.Exit): E | undefined => - Exit.isFailure(exit) ? exit.cause.reasons.find(Cause.isFailReason)?.error : undefined; - -const isToolInvocationError = (error: unknown): error is ToolInvocationError => - Predicate.isTagged(error, "ToolInvocationError"); - const createAuthRecordingMcpServer = () => makeEchoMcpServer({ name: "iso-test", @@ -156,19 +149,25 @@ describe("per-user MCP auth isolation", () => { const whoamiForB = userBTools.find((tool) => tool.name === "whoami"); expect(whoamiForB).toBeDefined(); - const userBResult = yield* Effect.exit( - execUserB.tools.invoke( - whoamiForB!.id, - { marker: "from-user-b" }, - { onElicitation: "accept-all" }, - ), + const userBResult = yield* execUserB.tools.invoke( + whoamiForB!.id, + { marker: "from-user-b" }, + { onElicitation: "accept-all" }, ); - expect(Exit.isFailure(userBResult)).toBe(true); - const outer = failureError(userBResult); - expect(isToolInvocationError(outer)).toBe(true); - const inner = isToolInvocationError(outer) ? outer.cause : undefined; - expect(Predicate.isTagged(inner, "McpConnectionError")).toBe(true); + expect(userBResult).toMatchObject({ + ok: false, + error: { + code: "oauth_connection_missing", + message: expect.stringContaining("Missing OAuth connection binding"), + details: { + category: "authentication", + recovery: { + startOAuthTool: "executor.coreTools.oauth.start", + }, + }, + }, + }); for (const request of (yield* server.requests).slice(recordedBeforeUserB)) { expect(request.authorization).not.toBe("Bearer token-user-a"); @@ -233,19 +232,25 @@ describe("per-user MCP auth isolation", () => { const recordedBeforeUserB = (yield* server.requests).length; const userBTools = yield* execUserB.tools.list(); const whoamiForB = userBTools.find((tool) => tool.name === "whoami")!; - const userBResult = yield* Effect.exit( - execUserB.tools.invoke( - whoamiForB.id, - { marker: "user-b-header" }, - { onElicitation: "accept-all" }, - ), + const userBResult = yield* execUserB.tools.invoke( + whoamiForB.id, + { marker: "user-b-header" }, + { onElicitation: "accept-all" }, ); - expect(Exit.isFailure(userBResult)).toBe(true); - const outer = failureError(userBResult); - expect(isToolInvocationError(outer)).toBe(true); - const inner = isToolInvocationError(outer) ? outer.cause : undefined; - expect(Predicate.isTagged(inner, "McpConnectionError")).toBe(true); + expect(userBResult).toMatchObject({ + ok: false, + error: { + code: "credential_binding_missing", + message: expect.stringContaining("Missing binding"), + details: { + category: "authentication", + recovery: { + createSecretTool: "executor.coreTools.secrets.create", + }, + }, + }, + }); for (const request of (yield* server.requests).slice(recordedBeforeUserB)) { expect(request.authorization).not.toBe("Bearer token-user-a-header"); diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index bcbf2ea41..2c06da1f0 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -21,6 +21,7 @@ import { ScopeId, SourceDetectionResult, ToolResult, + authToolFailure, defaultSourceInstallScopeId, definePlugin, tool, @@ -46,7 +47,12 @@ import { } from "./binding-store"; import { createMcpConnector, type ConnectorInput, type McpConnection } from "./connection"; import { discoverTools } from "./discover"; -import { McpConnectionError, McpInvocationError, McpToolDiscoveryError } from "./errors"; +import { + McpAuthRequiredError, + McpConnectionError, + McpInvocationError, + McpToolDiscoveryError, +} from "./errors"; import { invokeMcpTool } from "./invoke"; import { deriveMcpNamespace, type McpToolManifest, type McpToolManifestEntry } from "./manifest"; import { mcpPresets } from "./presets"; @@ -257,6 +263,30 @@ const mcpToolFailure = (code: string, message: string, details?: unknown) => ...(details === undefined ? {} : { details }), }); +const mcpAuthToolFailure = (failure: McpAuthRequiredError) => + authToolFailure({ + code: failure.code, + message: failure.message, + source: { id: failure.sourceId, scope: failure.sourceScope }, + credential: { + kind: failure.credentialKind, + ...(failure.credentialLabel ? { label: failure.credentialLabel } : {}), + ...(failure.slotKey ? { slotKey: failure.slotKey } : {}), + ...(failure.secretId ? { secretId: failure.secretId } : {}), + ...(failure.connectionId ? { connectionId: failure.connectionId } : {}), + }, + ...(failure.status !== undefined ? { status: failure.status } : {}), + ...(failure.details !== undefined + ? { + upstream: { + ...(failure.status !== undefined ? { status: failure.status } : {}), + details: failure.details, + }, + } + : {}), + recovery: { configureSourceTool: "executor.mcp.configureSource" }, + }); + const McpAddSourceInputStandardSchema = schemaToStaticToolSchema(McpAddSourceInputSchema); const McpAddSourceOutputStandardSchema = schemaToStaticToolSchema(McpAddSourceOutputSchema); const McpProbeEndpointInputStandardSchema = schemaToStaticToolSchema(McpProbeEndpointInputSchema); @@ -637,7 +667,7 @@ const resolveMcpBindingValueMap = ( readonly targetScope?: string; readonly missingLabel: string; }, -): Effect.Effect | undefined, McpConnectionError | StorageFailure> => +): Effect.Effect | undefined, McpAuthRequiredError | StorageFailure> => Effect.gen(function* () { if (!values) return undefined; const resolved: Record = {}; @@ -653,20 +683,33 @@ const resolveMcpBindingValueMap = ( value.slot, ); if (binding?.value.kind === "secret") { - const secret = yield* ctx.secrets.getAtScope(binding.value.secretId, binding.scopeId).pipe( + const secretBinding = binding.value; + const secret = yield* ctx.secrets.getAtScope(secretBinding.secretId, binding.scopeId).pipe( Effect.catchTag("SecretOwnedByConnectionError", () => Effect.fail( - new McpConnectionError({ - transport: "remote", + new McpAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), message: `Failed to resolve secret for ${params.missingLabel} "${name}"`, }), ), ), ); if (secret === null) { - return yield* new McpConnectionError({ - transport: "remote", - message: `Missing secret "${binding.value.secretId}" for ${params.missingLabel} "${name}"`, + return yield* new McpAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), + message: `Missing secret "${secretBinding.secretId}" for ${params.missingLabel} "${name}"`, }); } resolved[name] = value.prefix ? `${value.prefix}${secret}` : secret; @@ -676,8 +719,13 @@ const resolveMcpBindingValueMap = ( resolved[name] = value.prefix ? `${value.prefix}${binding.value.text}` : binding.value.text; continue; } - return yield* new McpConnectionError({ - transport: "remote", + return yield* new McpAuthRequiredError({ + code: "credential_binding_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, message: `Missing binding for ${params.missingLabel} "${name}"`, }); } @@ -760,24 +808,37 @@ const resolveMcpHeaderAuth = ( sourceId: string, sourceScope: string, auth: McpConnectionAuth, -): Effect.Effect, McpConnectionError | StorageFailure> => +): Effect.Effect, McpAuthRequiredError | StorageFailure> => Effect.gen(function* () { if (auth.kind !== "header") return {}; const binding = yield* resolveMcpSourceBinding(ctx, sourceId, sourceScope, auth.secretSlot); if (binding?.value.kind === "secret") { - const secret = yield* ctx.secrets.getAtScope(binding.value.secretId, binding.scopeId).pipe( + const secretBinding = binding.value; + const secret = yield* ctx.secrets.getAtScope(secretBinding.secretId, binding.scopeId).pipe( Effect.catchTag("SecretOwnedByConnectionError", () => Effect.fail( - new McpConnectionError({ - transport: "remote", + new McpAuthRequiredError({ + code: "credential_secret_missing", + sourceId, + sourceScope, + credentialKind: "secret", + credentialLabel: auth.headerName, + slotKey: auth.secretSlot, + secretId: String(secretBinding.secretId), message: `Failed to resolve header auth binding "${auth.secretSlot}"`, }), ), ), ); if (secret === null) { - return yield* new McpConnectionError({ - transport: "remote", + return yield* new McpAuthRequiredError({ + code: "credential_secret_missing", + sourceId, + sourceScope, + credentialKind: "secret", + credentialLabel: auth.headerName, + slotKey: auth.secretSlot, + secretId: String(secretBinding.secretId), message: `Missing secret for header auth binding "${auth.secretSlot}"`, }); } @@ -788,8 +849,13 @@ const resolveMcpHeaderAuth = ( [auth.headerName]: auth.prefix ? `${auth.prefix}${binding.value.text}` : binding.value.text, }; } - return yield* new McpConnectionError({ - transport: "remote", + return yield* new McpAuthRequiredError({ + code: "credential_binding_missing", + sourceId, + sourceScope, + credentialKind: "secret", + credentialLabel: auth.headerName, + slotKey: auth.secretSlot, message: `Missing header auth binding "${auth.secretSlot}"`, }); }); @@ -799,13 +865,18 @@ const resolveMcpStoredOauthProvider = ( sourceId: string, sourceScope: string, auth: McpConnectionAuth, -): Effect.Effect => +): Effect.Effect => Effect.gen(function* () { if (auth.kind !== "oauth2") return undefined; const binding = yield* resolveMcpSourceBinding(ctx, sourceId, sourceScope, auth.connectionSlot); if (binding?.value.kind !== "connection") { - return yield* new McpConnectionError({ - transport: "remote", + return yield* new McpAuthRequiredError({ + code: "oauth_connection_missing", + sourceId, + sourceScope, + credentialKind: "connection", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, message: `Missing OAuth connection binding for MCP source "${sourceId}"`, }); } @@ -813,13 +884,73 @@ const resolveMcpStoredOauthProvider = ( const accessToken = yield* ctx.connections .accessTokenAtScope(connectionId, binding.scopeId) .pipe( - Effect.mapError( - ({ message }) => - new McpConnectionError({ - transport: "remote", - message: `Failed to resolve OAuth connection "${connectionId}": ${message}`, - }), - ), + Effect.catchTags({ + ConnectionReauthRequiredError: ({ message, connectionId: failedConnectionId }) => + Effect.fail( + new McpAuthRequiredError({ + code: "oauth_reauth_required", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(failedConnectionId), + message: `OAuth connection "${failedConnectionId}" needs re-authentication: ${message}`, + }), + ), + ConnectionNotFoundError: ({ connectionId: failedConnectionId }) => + Effect.fail( + new McpAuthRequiredError({ + code: "oauth_connection_missing", + sourceId, + sourceScope, + credentialKind: "connection", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(failedConnectionId), + message: `OAuth connection "${failedConnectionId}" was not found for MCP source "${sourceId}"`, + }), + ), + ConnectionProviderNotRegisteredError: ({ provider }) => + Effect.fail( + new McpAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(connectionId), + message: `OAuth provider "${provider}" is not registered`, + }), + ), + ConnectionRefreshNotSupportedError: ({ provider, connectionId: failedConnectionId }) => + Effect.fail( + new McpAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(failedConnectionId), + message: `OAuth provider "${provider}" cannot refresh connection "${failedConnectionId}"`, + }), + ), + ConnectionRefreshError: ({ message, connectionId: failedConnectionId }) => + Effect.fail( + new McpAuthRequiredError({ + code: "oauth_connection_failed", + sourceId, + sourceScope, + credentialKind: "oauth", + credentialLabel: "OAuth sign-in", + slotKey: auth.connectionSlot, + connectionId: String(failedConnectionId), + message: `OAuth connection "${failedConnectionId}" refresh failed: ${message}`, + }), + ), + }), ); return makeOAuthProvider(accessToken); }); @@ -834,7 +965,7 @@ const resolveConnectorInput = ( sd: McpStoredSourceData, ctx: PluginCtx, allowStdio: boolean, -): Effect.Effect => { +): Effect.Effect => { if (sd.transport === "stdio") { if (!allowStdio) { return Effect.fail( @@ -891,15 +1022,25 @@ const resolveConnectorInput = ( // --------------------------------------------------------------------------- interface McpRuntime { - readonly connectionCache: ScopedCache.ScopedCache; - readonly pendingConnectors: Map>; + readonly connectionCache: ScopedCache.ScopedCache< + string, + McpConnection, + McpAuthRequiredError | McpConnectionError + >; + readonly pendingConnectors: Map< + string, + Effect.Effect + >; readonly cacheScope: Scope.Closeable; } const makeRuntime = (): Effect.Effect => Effect.gen(function* () { const cacheScope = yield* Scope.make(); - const pendingConnectors = new Map>(); + const pendingConnectors = new Map< + string, + Effect.Effect + >(); const connectionCache = yield* ScopedCache.make({ lookup: (key: string) => Effect.acquireRelease( @@ -1350,7 +1491,10 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { initialRemote.scope, ) : undefined; - const resolved: Result.Result = + const resolved: Result.Result< + ConnectorInput, + McpAuthRequiredError | McpConnectionError | StorageFailure + > = config.transport === "remote" ? Result.succeed({ transport: "remote" as const, @@ -1381,7 +1525,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { // the caller at the end. const discovery: Result.Result< McpToolManifest, - McpToolDiscoveryError | McpConnectionError | StorageFailure + McpAuthRequiredError | McpToolDiscoveryError | McpConnectionError | StorageFailure > = Result.isSuccess(resolved) ? yield* discoverTools(createMcpConnector(resolved.success)).pipe( Effect.mapError( @@ -1478,6 +1622,12 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { } if (Result.isFailure(discovery)) { + if (Predicate.isTagged(discovery.failure, "McpAuthRequiredError")) { + return yield* new McpConnectionError({ + transport: sd.transport, + message: discovery.failure.message, + }); + } return yield* Effect.fail(discovery.failure); } return { toolCount: manifest.tools.length, namespace }; @@ -1532,6 +1682,9 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { } const ci = yield* resolveConnectorInput(namespace, scope, sd, ctx, allowStdio).pipe( + Effect.catchTag("McpAuthRequiredError", ({ message }) => + Effect.fail(new McpConnectionError({ transport: sd.transport, message })), + ), Effect.withSpan("mcp.plugin.resolve_connector", { attributes: { "mcp.source.namespace": namespace, @@ -1959,6 +2112,9 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { } return ToolResult.ok(raw); }).pipe( + Effect.catchTag("McpAuthRequiredError", (error) => + Effect.succeed(mcpAuthToolFailure(error)), + ), Effect.withSpan("mcp.plugin.invoke_tool", { attributes: { "mcp.tool.name": toolRow.id, diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index cf2fb6730..1a2d8d24e 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -1,5 +1,6 @@ import { Data, Schema } from "effect"; import type { Option } from "effect"; +import type { AuthToolFailureCode } from "@executor-js/sdk/core"; // HTTP status lives on the class declaration so HttpApiBuilder's error // encoder (which reads `ast.annotations` off the schema it stored on @@ -37,3 +38,18 @@ export class OpenApiOAuthError extends Schema.TaggedErrorClass {} diff --git a/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts b/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts index 59523c9f4..bffe2f961 100644 --- a/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts +++ b/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts @@ -31,7 +31,6 @@ import { SetSecretInput, SetSourceCredentialBindingInput, RemoveSourceCredentialBindingInput, - ToolInvocationError, type InvokeOptions, type SecretProvider, } from "@executor-js/sdk"; @@ -627,15 +626,21 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }); expect(bindingsAfterReadd).toEqual([]); - const error = yield* Effect.flip( - aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), - ); - expect(error).toBeInstanceOf(ToolInvocationError); - expect(error).toEqual( - expect.objectContaining({ + const result = yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove); + expect(result).toMatchObject({ + ok: false, + error: { + code: "credential_binding_missing", message: expect.stringContaining('Missing binding for header "Authorization"'), - }), - ); + details: { + category: "authentication", + credential: { + label: "Authorization", + slotKey: "auth:token", + }, + }, + }, + }); }), ); diff --git a/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts b/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts index 98d1dd7cc..2e0213d10 100644 --- a/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts +++ b/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts @@ -351,10 +351,25 @@ describe("OpenAPI oauth refresh", () => { }); yield* bindOAuthConnection(executor, scopeId, "conn-refresh-dead", auth); - // Tool invocation currently wraps connection errors in a - // generic Error (see openapi invokeTool), so we assert against - // the `accessToken` call directly too — that's the surface - // the UI bridges use to trigger re-auth. + const invocation = yield* executor.tools.invoke( + "petstore.items.echoHeaders", + {}, + autoApprove, + ); + expect(invocation).toMatchObject({ + ok: false, + error: { + code: "oauth_reauth_required", + message: expect.stringContaining("needs re-authentication"), + details: { + category: "authentication", + recovery: { + startOAuthTool: "executor.coreTools.oauth.start", + }, + }, + }, + }); + const flipped = yield* executor.connections.accessToken("conn-refresh-dead").pipe( Effect.flip, Effect.flatMap((error) => diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 36af87cb7..9d2bfe7d5 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -769,13 +769,21 @@ describe("OpenAPI Plugin", () => { ); secretStore.delete(key(TEST_SCOPE, "missing-token")); - const error = yield* Effect.flip( - executor.tools.invoke("noauth.items.listItems", {}, autoApprove), - ); - - expect(Predicate.isTagged(error, "ToolInvocationError")).toBe(true); - expect(error).toMatchObject({ - message: expect.stringContaining("missing-token"), + const result = yield* executor.tools.invoke("noauth.items.listItems", {}, autoApprove); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "credential_secret_missing", + message: expect.stringContaining("missing-token"), + details: { + category: "authentication", + recovery: { + createSecretTool: "executor.coreTools.secrets.create", + secretsUrl: "https://executor.sh/secrets", + }, + }, + }, }); }), ), diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index 2dd322e70..26743509e 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -10,6 +10,7 @@ import { SourceDetectionResult, StorageError, ToolResult, + authToolFailure, defaultSourceInstallScopeId, definePlugin, tool, @@ -27,7 +28,12 @@ import { type OpenApiSourceConfig, } from "@executor-js/config"; -import { OpenApiExtractionError, OpenApiOAuthError, OpenApiParseError } from "./errors"; +import { + OpenApiAuthRequiredError, + OpenApiExtractionError, + OpenApiOAuthError, + OpenApiParseError, +} from "./errors"; import { parse, resolveSpecText } from "./parse"; import { extract } from "./extract"; import { compileToolDefinitions, type ToolDefinition } from "./definitions"; @@ -440,6 +446,30 @@ const openApiToolFailure = (code: string, message: string, details?: unknown) => ...(details === undefined ? {} : { details }), }); +const openApiAuthToolFailure = (failure: OpenApiAuthRequiredError) => + authToolFailure({ + code: failure.code, + message: failure.message, + source: { id: failure.sourceId, scope: failure.sourceScope }, + credential: { + kind: failure.credentialKind, + ...(failure.credentialLabel ? { label: failure.credentialLabel } : {}), + ...(failure.slotKey ? { slotKey: failure.slotKey } : {}), + ...(failure.secretId ? { secretId: failure.secretId } : {}), + ...(failure.connectionId ? { connectionId: failure.connectionId } : {}), + }, + ...(failure.status !== undefined ? { status: failure.status } : {}), + ...(failure.details !== undefined + ? { + upstream: { + ...(failure.status !== undefined ? { status: failure.status } : {}), + details: failure.details, + }, + } + : {}), + recovery: { configureSourceTool: "executor.openapi.configureSource" }, + }); + const staticPreviewOutput = (preview: SpecPreview): StaticPreviewSpecOutput => ({ title: Option.getOrNull(preview.title), version: Option.getOrNull(preview.version), @@ -941,7 +971,7 @@ const resolveConfiguredValueMap = ( readonly values: Record; readonly missingLabel: string; }, -): Effect.Effect, OpenApiOAuthError | StorageFailure> => +): Effect.Effect, OpenApiAuthRequiredError | StorageFailure> => Effect.gen(function* () { const resolved: Record = {}; for (const [name, value] of Object.entries(params.values)) { @@ -956,20 +986,35 @@ const resolveConfiguredValueMap = ( value.slot, ); if (binding?.value.kind === "secret") { + const secretBinding = binding.value; const secret = yield* ctx.secrets - .getAtScope(binding.value.secretId, binding.value.secretScopeId ?? binding.scopeId) + .getAtScope(secretBinding.secretId, secretBinding.secretScopeId ?? binding.scopeId) .pipe( Effect.catchTag("SecretOwnedByConnectionError", () => Effect.fail( - new OpenApiOAuthError({ - message: `Secret not found for header "${name}"`, + new OpenApiAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), + message: `Secret not found for ${params.missingLabel} "${name}"`, }), ), ), ); if (secret === null) { - return yield* new OpenApiOAuthError({ - message: `Missing secret "${binding.value.secretId}" for ${params.missingLabel} "${name}"`, + return yield* new OpenApiAuthRequiredError({ + code: "credential_secret_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, + secretId: String(secretBinding.secretId), + message: `Missing secret "${secretBinding.secretId}" for ${params.missingLabel} "${name}"`, }); } resolved[name] = value.prefix ? `${value.prefix}${secret}` : secret; @@ -979,7 +1024,13 @@ const resolveConfiguredValueMap = ( resolved[name] = value.prefix ? `${value.prefix}${binding.value.text}` : binding.value.text; continue; } - return yield* new OpenApiOAuthError({ + return yield* new OpenApiAuthRequiredError({ + code: "credential_binding_missing", + sourceId: params.sourceId, + sourceScope: params.sourceScope, + credentialKind: "secret", + credentialLabel: name, + slotKey: value.slot, message: `Missing binding for ${params.missingLabel} "${name}"`, }); } @@ -993,7 +1044,7 @@ const resolveConfiguredHeaders = ( readonly sourceScope: string; readonly headers: Record; }, -): Effect.Effect, OpenApiOAuthError | StorageFailure> => +): Effect.Effect, OpenApiAuthRequiredError | StorageFailure> => resolveConfiguredValueMap(ctx, { sourceId: params.sourceId, sourceScope: params.sourceScope, @@ -1089,7 +1140,11 @@ const resolveStoredSpecFetchCredentials = ( missingLabel: "spec fetch query parameter", }), }; - }); + }).pipe( + Effect.catchTag("OpenApiAuthRequiredError", ({ message }) => + Effect.fail(new OpenApiOAuthError({ message })), + ), + ); // --------------------------------------------------------------------------- // OAuth2 token exchange / refresh is owned by `ctx.oauth`, which registers @@ -1548,25 +1603,109 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { // Authorization header (wins over a manually-set one). All the // refresh complexity lives in the SDK — the plugin just asks. if (config.oauth2) { + const oauth2 = config.oauth2; const connection = yield* resolveOAuthConnectionId(ctx, { sourceId: op.sourceId, sourceScope: effective.oauth2Source.scope, - oauth2: config.oauth2, + oauth2, }); if (!connection) { - return yield* new OpenApiOAuthError({ + return yield* new OpenApiAuthRequiredError({ + code: "oauth_connection_missing", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "connection", + credentialLabel: + oauth2.flow === "clientCredentials" ? "OAuth client connection" : "OAuth sign-in", + slotKey: oauth2.connectionSlot, message: `OAuth configuration for "${op.sourceId}" is missing a connection binding`, }); } const accessToken = yield* ctx.connections .accessTokenAtScope(connection.connectionId, connection.scopeId) .pipe( - Effect.mapError( - () => - new OpenApiOAuthError({ - message: "OAuth connection resolution failed", - }), - ), + Effect.catchTags({ + ConnectionReauthRequiredError: ({ message, connectionId }) => + Effect.fail( + new OpenApiAuthRequiredError({ + code: "oauth_reauth_required", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "oauth", + credentialLabel: + oauth2.flow === "clientCredentials" + ? "OAuth client connection" + : "OAuth sign-in", + slotKey: oauth2.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" needs re-authentication: ${message}`, + }), + ), + ConnectionNotFoundError: ({ connectionId }) => + Effect.fail( + new OpenApiAuthRequiredError({ + code: "oauth_connection_missing", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "connection", + credentialLabel: + oauth2.flow === "clientCredentials" + ? "OAuth client connection" + : "OAuth sign-in", + slotKey: oauth2.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" was not found for "${op.sourceId}"`, + }), + ), + ConnectionProviderNotRegisteredError: ({ provider }) => + Effect.fail( + new OpenApiAuthRequiredError({ + code: "oauth_connection_failed", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "oauth", + credentialLabel: + oauth2.flow === "clientCredentials" + ? "OAuth client connection" + : "OAuth sign-in", + slotKey: oauth2.connectionSlot, + connectionId: connection.connectionId, + message: `OAuth provider "${provider}" is not registered`, + }), + ), + ConnectionRefreshNotSupportedError: ({ provider, connectionId }) => + Effect.fail( + new OpenApiAuthRequiredError({ + code: "oauth_connection_failed", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "oauth", + credentialLabel: + oauth2.flow === "clientCredentials" + ? "OAuth client connection" + : "OAuth sign-in", + slotKey: oauth2.connectionSlot, + connectionId: String(connectionId), + message: `OAuth provider "${provider}" cannot refresh connection "${connectionId}"`, + }), + ), + ConnectionRefreshError: ({ message, connectionId }) => + Effect.fail( + new OpenApiAuthRequiredError({ + code: "oauth_connection_failed", + sourceId: op.sourceId, + sourceScope: effective.oauth2Source.scope, + credentialKind: "oauth", + credentialLabel: + oauth2.flow === "clientCredentials" + ? "OAuth client connection" + : "OAuth sign-in", + slotKey: oauth2.connectionSlot, + connectionId: String(connectionId), + message: `OAuth connection "${connectionId}" refresh failed: ${message}`, + }), + ), + }), ); resolvedHeaders.authorization = `Bearer ${accessToken}`; } @@ -1582,6 +1721,17 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { const ok = result.status >= 200 && result.status < 300; if (!ok) { + if (result.status === 401 || result.status === 403) { + return authToolFailure({ + code: "credential_rejected", + status: result.status, + message: `Upstream rejected credentials for "${op.sourceId}" with HTTP ${result.status}. Re-authenticate or update the source credentials before retrying this tool.`, + source: { id: op.sourceId, scope: toolScope }, + credential: { kind: "upstream", label: "Upstream authorization" }, + upstream: { status: result.status, details: result.error }, + recovery: { configureSourceTool: "executor.openapi.configureSource" }, + }); + } return ToolResult.fail({ code: "upstream_http_error", status: result.status, @@ -1594,7 +1744,11 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { headers: result.headers, data: result.data, }); - }), + }).pipe( + Effect.catchTag("OpenApiAuthRequiredError", (error) => + Effect.succeed(openApiAuthToolFailure(error)), + ), + ), resolveAnnotations: ({ ctx, sourceId, toolRows }) => Effect.gen(function* () { diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index 3a8e16cce..2a094d5e1 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -224,6 +224,34 @@ describe("OpenAPI upstream failure modes", () => { }), ); + it.effect("upstream 401 is classified as credential_rejected", () => + Effect.gen(function* () { + const server = yield* startScriptedServer(() => ({ + status: 401, + headers: { "content-type": "application/json" }, + body: '{"error":{"message":"invalid token"}}', + })); + const executor = yield* buildExecutorForOpenApiServer(server); + + const result = yield* executor.tools.invoke("f.things.listThings", {}, autoApprove); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "credential_rejected", + status: 401, + message: expect.stringContaining("Upstream rejected credentials"), + details: { + category: "authentication", + upstream: { + status: 401, + }, + }, + }, + }); + }), + ); + it.effect("upstream returns malformed JSON despite Content-Type: application/json", () => Effect.gen(function* () { const server = yield* startScriptedServer(() => ({ From 2c034184e58813cda035368fdc85398c99068050 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 20 May 2026 10:18:29 -0700 Subject: [PATCH 2/2] add app auth failure boundary tests --- .../services/auth-tool-failures.node.test.ts | 94 ++++++++ .../src/server/auth-tool-failures.test.ts | 219 ++++++++++++++++++ packages/plugins/mcp/src/sdk/plugin.ts | 6 + 3 files changed, 319 insertions(+) create mode 100644 apps/cloud/src/services/auth-tool-failures.node.test.ts create mode 100644 apps/local/src/server/auth-tool-failures.test.ts diff --git a/apps/cloud/src/services/auth-tool-failures.node.test.ts b/apps/cloud/src/services/auth-tool-failures.node.test.ts new file mode 100644 index 000000000..37f1b8a48 --- /dev/null +++ b/apps/cloud/src/services/auth-tool-failures.node.test.ts @@ -0,0 +1,94 @@ +// --------------------------------------------------------------------------- +// Cloud app auth failure propagation +// --------------------------------------------------------------------------- +// +// Exercises the cloud HTTP API boundary: +// +// test -> HttpApiClient -> ProtectedCloudApi -> execution engine +// -> sandbox code -> OpenAPI tool invocation +// +// The assertion is intentionally on the final execution payload, not the +// plugin facade, so reviewers can see that model-visible tool results carry +// auth guidance instead of an opaque internal tool error. +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Schema } from "effect"; +import { HttpApi, HttpApiClient, HttpApiEndpoint, HttpApiGroup } from "effect/unstable/httpapi"; + +import { ScopeId } from "@executor-js/sdk"; +import { makeOpenApiHttpApiTestAddSpecPayload } from "@executor-js/plugin-openapi/testing"; + +import { ProtectedCloudApi, asOrg } from "./__test-harness__/api-harness"; + +const PingGroup = HttpApiGroup.make("default", { topLevel: true }).add( + HttpApiEndpoint.get("ping", "/ping", { success: Schema.Unknown }), +); + +const MissingAuthSourceApi = HttpApi.make("cloudAuthFailureSource").add(PingGroup); + +type CloudApiShape = HttpApiClient.ForApi; +type EffectSuccess = T extends Effect.Effect ? A : never; +type ExecuteResult = EffectSuccess>; + +const expectModelVisibleAuthFailure = (execution: ExecuteResult) => { + expect(execution.status).toBe("completed"); + if (execution.status !== "completed") return; + expect(execution.isError).toBe(false); + expect(JSON.stringify(execution.structured)).not.toContain("Internal tool error"); + expect(JSON.stringify(execution.structured)).not.toContain("Internal Tool Error"); + expect(execution.structured).toMatchObject({ + status: "completed", + result: { + ok: false, + error: { + code: "credential_binding_missing", + details: { + category: "authentication", + recovery: { + createSecretTool: "executor.coreTools.secrets.create", + secretsUrl: "https://executor.sh/secrets", + }, + }, + }, + }, + }); +}; + +describe("cloud auth tool failures", () => { + it.effect("cloud propagates missing credential binding as model-visible auth failure", () => + Effect.gen(function* () { + const org = `org_${crypto.randomUUID()}`; + const namespace = `auth_${crypto.randomUUID().replace(/-/g, "_")}`; + const scopeId = ScopeId.make(org); + + yield* asOrg(org, (client) => + client.openapi.addSpec({ + params: { scopeId }, + payload: { + ...makeOpenApiHttpApiTestAddSpecPayload(MissingAuthSourceApi, { + namespace, + headers: { + Authorization: { kind: "secret", prefix: "Bearer " }, + }, + }), + baseUrl: "https://api.example.test", + }, + }), + ); + + const execution = yield* asOrg(org, (client) => + client.executions.execute({ + payload: { + code: [ + `const result = await tools.${namespace}.default.ping({});`, + "return result;", + ].join("\n"), + }, + }), + ); + + expectModelVisibleAuthFailure(execution); + }), + ); +}); diff --git a/apps/local/src/server/auth-tool-failures.test.ts b/apps/local/src/server/auth-tool-failures.test.ts new file mode 100644 index 000000000..c779d6141 --- /dev/null +++ b/apps/local/src/server/auth-tool-failures.test.ts @@ -0,0 +1,219 @@ +// --------------------------------------------------------------------------- +// Local app auth failure propagation +// --------------------------------------------------------------------------- +// +// Exercises the local HTTP API boundary: +// +// test -> HttpApiClient -> in-process LocalApi -> execution engine +// -> sandbox code -> OpenAPI tool invocation +// +// The assertion is intentionally on the final execution payload, not the +// plugin facade, so reviewers can see that model-visible tool results carry +// auth guidance instead of an opaque internal tool error. +// --------------------------------------------------------------------------- + +import { afterAll, beforeAll, describe, expect, it } from "@effect/vitest"; +import { randomBytes } from "node:crypto"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { Effect, Layer, Schema } from "effect"; +import { FetchHttpClient, HttpRouter, HttpServer } from "effect/unstable/http"; +import { + HttpApi, + HttpApiBuilder, + HttpApiClient, + HttpApiEndpoint, + HttpApiGroup, +} from "effect/unstable/httpapi"; + +import { addGroup, observabilityMiddleware } from "@executor-js/api"; +import { CoreHandlers, ExecutionEngineService, ExecutorService } from "@executor-js/api/server"; +import { createExecutionEngine } from "@executor-js/execution"; +import { fileSecretsPlugin } from "@executor-js/plugin-file-secrets"; +import { openApiPlugin } from "@executor-js/plugin-openapi"; +import { + OpenApiExtensionService, + OpenApiGroup, + OpenApiHandlers, +} from "@executor-js/plugin-openapi/api"; +import { makeOpenApiHttpApiTestAddSpecPayload } from "@executor-js/plugin-openapi/testing"; +import { makeQuickJsExecutor } from "@executor-js/runtime-quickjs"; +import { Scope, ScopeId, collectTables, createExecutor } from "@executor-js/sdk"; + +import { ErrorCaptureLive } from "./observability"; +import { createSqliteFumaDb } from "./sqlite-fumadb"; + +const TEST_BASE_URL = "http://local.test"; + +const PingGroup = HttpApiGroup.make("default", { topLevel: true }).add( + HttpApiEndpoint.get("ping", "/ping", { success: Schema.Unknown }), +); + +const MissingAuthSourceApi = HttpApi.make("localAuthFailureSource").add(PingGroup); + +const TestApi = addGroup(OpenApiGroup); +type TestApiShape = + typeof TestApi extends HttpApi.HttpApi + ? HttpApiClient.Client + : never; + +interface Harness { + readonly fetch: typeof globalThis.fetch; + readonly scopeId: ScopeId; + readonly dispose: () => Promise; +} + +const startHarness = async (tmpDir: string): Promise => { + const scopeId = ScopeId.make(`test-${randomBytes(4).toString("hex")}`); + const plugins = [ + openApiPlugin({ httpClientLayer: FetchHttpClient.layer }), + fileSecretsPlugin({ directory: tmpDir }), + ] as const; + const sqlite = await createSqliteFumaDb({ + tables: collectTables(plugins), + namespace: "executor_local_auth_tool_failures_test", + path: join(tmpDir, "data.db"), + }); + + const executor = await Effect.runPromise( + createExecutor({ + scopes: [ + Scope.make({ + id: scopeId, + name: "test", + createdAt: new Date(), + }), + ], + db: sqlite.db, + plugins, + onElicitation: "accept-all", + }), + ); + + const engine = createExecutionEngine({ + executor, + codeExecutor: makeQuickJsExecutor(), + }); + + const TestObservability = observabilityMiddleware(TestApi); + const TestApiBase = HttpApiBuilder.layer(TestApi).pipe( + Layer.provide(CoreHandlers), + Layer.provide(OpenApiHandlers), + Layer.provide(TestObservability), + Layer.provide(ErrorCaptureLive), + ); + + const { handler: webHandler, dispose: disposeHandler } = HttpRouter.toWebHandler( + TestApiBase.pipe( + Layer.provideMerge(Layer.succeed(OpenApiExtensionService)(executor.openapi)), + Layer.provideMerge(Layer.succeed(ExecutorService)(executor)), + Layer.provideMerge(Layer.succeed(ExecutionEngineService)(engine)), + Layer.provideMerge(HttpServer.layerServices), + Layer.provideMerge(Layer.succeed(HttpRouter.RouterConfig)({ maxParamLength: 1000 })), + ), + ); + + return { + fetch: ((input: RequestInfo | URL, init?: RequestInit) => + webHandler( + input instanceof Request ? input : new Request(input, init), + )) as typeof globalThis.fetch, + scopeId, + dispose: async () => { + await Effect.runPromise(Effect.ignore(Effect.tryPromise(() => disposeHandler()))); + await Effect.runPromise( + Effect.ignore(Effect.tryPromise(() => Effect.runPromise(executor.close()))), + ); + await sqlite.close(); + }, + }; +}; + +const run = (body: (client: TestApiShape) => Effect.Effect): Effect.Effect => + Effect.gen(function* () { + const client = yield* HttpApiClient.make(TestApi, { baseUrl: TEST_BASE_URL }); + return yield* body(client); + }).pipe( + Effect.provide( + FetchHttpClient.layer.pipe( + Layer.provide(Layer.succeed(FetchHttpClient.Fetch)(harness.fetch)), + ), + ), + ) as Effect.Effect; + +type EffectSuccess = T extends Effect.Effect ? A : never; +type ExecuteResult = EffectSuccess>; + +const expectModelVisibleAuthFailure = (execution: ExecuteResult) => { + expect(execution.status).toBe("completed"); + if (execution.status !== "completed") return; + expect(execution.isError).toBe(false); + expect(JSON.stringify(execution.structured)).not.toContain("Internal tool error"); + expect(JSON.stringify(execution.structured)).not.toContain("Internal Tool Error"); + expect(execution.structured).toMatchObject({ + status: "completed", + result: { + ok: false, + error: { + code: "credential_binding_missing", + details: { + category: "authentication", + recovery: { + createSecretTool: "executor.coreTools.secrets.create", + secretsUrl: "https://executor.sh/secrets", + }, + }, + }, + }, + }); +}; + +let tmpDir: string; +let harness: Harness; + +beforeAll(async () => { + tmpDir = mkdtempSync(join(tmpdir(), "executor-local-auth-tool-failures-")); + harness = await startHarness(tmpDir); +}); + +afterAll(async () => { + await harness.dispose(); + rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe("local auth tool failures", () => { + it.effect("local propagates missing credential binding as model-visible auth failure", () => + Effect.gen(function* () { + const namespace = `auth_${randomBytes(4).toString("hex")}`; + yield* run((client) => + client.openapi.addSpec({ + params: { scopeId: harness.scopeId }, + payload: { + ...makeOpenApiHttpApiTestAddSpecPayload(MissingAuthSourceApi, { + namespace, + headers: { + Authorization: { kind: "secret", prefix: "Bearer " }, + }, + }), + baseUrl: "https://api.example.test", + }, + }), + ); + + const execution = yield* run((client) => + client.executions.execute({ + payload: { + code: [ + `const result = await tools.${namespace}.default.ping({});`, + "return result;", + ].join("\n"), + }, + }), + ); + + expectModelVisibleAuthFailure(execution); + }), + ); +}); diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 2c06da1f0..f1c44e490 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -1516,6 +1516,12 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { ); if (Result.isFailure(resolved) && sd.transport === "stdio") { + if (Predicate.isTagged(resolved.failure, "McpAuthRequiredError")) { + return yield* new McpConnectionError({ + transport: sd.transport, + message: resolved.failure.message, + }); + } return yield* Effect.fail(resolved.failure); }