From 8b9e886f604faec639969acc113824c7064115fa Mon Sep 17 00:00:00 2001 From: masnwilliams <43387599+masnwilliams@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:08:56 +0000 Subject: [PATCH 1/4] Return 401 instead of 500 when the Kernel API rejects a credential --- src/app/[transport]/route.test.ts | 32 ++++++ src/app/[transport]/route.ts | 52 ++++++++- src/lib/mcp/analytics.ts | 38 +++++++ src/lib/mcp/auth-context.test.ts | 177 +++++++++++++++++++----------- src/lib/mcp/auth-context.ts | 51 ++++++--- 5 files changed, 267 insertions(+), 83 deletions(-) create mode 100644 src/app/[transport]/route.test.ts diff --git a/src/app/[transport]/route.test.ts b/src/app/[transport]/route.test.ts new file mode 100644 index 0000000..09f2265 --- /dev/null +++ b/src/app/[transport]/route.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "bun:test"; + +const { connectionScopeFailureResponse } = await import("./route"); + +describe("connectionScopeFailureResponse", () => { + test("answers a rejected credential with 401 rather than a server error", async () => { + const response = connectionScopeFailureResponse({ + status: "rejected", + statusCode: 401, + }); + + expect(response.status).toBe(401); + expect(response.headers.get("WWW-Authenticate")).toContain( + 'error="invalid_token"', + ); + expect(await response.json()).toEqual({ + error: "invalid_token", + error_description: "The Kernel API rejected this credential", + }); + }); + + test("answers an unresolvable scope with a retryable 503", async () => { + const response = connectionScopeFailureResponse({ status: "unavailable" }); + + expect(response.status).toBe(503); + expect(response.headers.get("Retry-After")).toBe("1"); + expect(await response.json()).toEqual({ + error: "temporarily_unavailable", + error_description: "Unable to resolve Kernel connection scope", + }); + }); +}); diff --git a/src/app/[transport]/route.ts b/src/app/[transport]/route.ts index cd6e224..b023948 100644 --- a/src/app/[transport]/route.ts +++ b/src/app/[transport]/route.ts @@ -7,6 +7,7 @@ import { verifyToken } from "@clerk/nextjs/server"; import { after, NextRequest } from "next/server"; import { isValidJwtFormat } from "@/lib/auth-utils"; import { + captureMcpConnectionScopeFailure, flushMcpAnalytics, instrumentMcpAnalytics, isMcpAnalyticsEnabled, @@ -14,6 +15,7 @@ import { import { connectionAnalyticsFromContext, resolveMcpConnectionContext, + type McpConnectionContextFailure, } from "@/lib/mcp/auth-context"; import { mcpAppsAuthSubject } from "@/lib/mcp-apps-marker"; import { requestUsesMcpApps } from "@/lib/mcp-apps-request"; @@ -59,6 +61,36 @@ function createAuthErrorResponse( ); } +// A credential the Kernel API rejects is the caller's problem, and a scope we +// cannot resolve right now is worth retrying. Neither is a server fault, so +// neither should reach the error handler as a thrown 500. +export function connectionScopeFailureResponse( + failure: Exclude, +): Response { + if (failure.status === "rejected") { + return createAuthErrorResponse( + "invalid_token", + "The Kernel API rejected this credential", + ); + } + return new Response( + JSON.stringify({ + error: "temporarily_unavailable", + error_description: "Unable to resolve Kernel connection scope", + }), + { + status: 503, + headers: { + "Retry-After": "1", + "Content-Type": "application/json", + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Methods": "GET, POST, OPTIONS", + "Access-Control-Allow-Headers": "Content-Type, Authorization", + }, + }, + ); +} + // Handler variants keep per-connection capabilities out of tools/list unless // the authenticated connection can use them. const serverInfo = { serverInfo: { name, version } }; @@ -83,6 +115,7 @@ async function handleMcpRequestWithIdentity({ authSubject, scopes, authInfoExtra, + credentialType, transportSessionId, connectionContextCacheIdentity, observeConnection, @@ -92,11 +125,12 @@ async function handleMcpRequestWithIdentity({ authSubject: string; scopes: string[]; authInfoExtra: AuthInfoExtra; + credentialType: "api_key" | "oauth"; transportSessionId: string | null; connectionContextCacheIdentity?: string; observeConnection: boolean; }) { - const [mcpApps, connectionContext] = await Promise.all([ + const [mcpApps, connection] = await Promise.all([ requestUsesMcpApps(req, { authSubject, transportSessionId, @@ -108,9 +142,19 @@ async function handleMcpRequestWithIdentity({ cacheIdentity: connectionContextCacheIdentity, }), ]); - if (!connectionContext) { - throw new Error("Unable to resolve Kernel connection scope"); + if (connection.status !== "ok") { + captureMcpConnectionScopeFailure({ + outcome: connection.status, + credentialType, + upstreamStatusCode: + connection.status === "rejected" ? connection.statusCode : undefined, + }); + if (connection.status === "invalid") { + throw new Error("Unable to resolve Kernel connection scope"); + } + return connectionScopeFailureResponse(connection); } + const connectionContext = connection.context; const connectionAnalytics = observeConnection && isMcpAnalyticsEnabled() ? connectionAnalyticsFromContext(connectionContext) @@ -161,6 +205,7 @@ async function handleAuthenticatedRequest( authSubject: mcpAppsAuthSubject({ token }), scopes: ["apikey"], authInfoExtra: { userId: null, clerkToken: null }, + credentialType: "api_key", transportSessionId, observeConnection, }); @@ -194,6 +239,7 @@ async function handleAuthenticatedRequest( authSubject, scopes: ["openid"], authInfoExtra: { userId, clerkToken: token }, + credentialType: "oauth", transportSessionId, connectionContextCacheIdentity: transportSessionId ? `${authSubject}\0${transportSessionId}` diff --git a/src/lib/mcp/analytics.ts b/src/lib/mcp/analytics.ts index 7eec511..cefe264 100644 --- a/src/lib/mcp/analytics.ts +++ b/src/lib/mcp/analytics.ts @@ -39,6 +39,18 @@ export type OAuthTokenExchangeAnalytics = { export const OAUTH_TOKEN_EXCHANGE_EVENT = "oauth_token_exchange"; +// Scope resolution runs before a request reaches the instrumented server, so a +// connection that never gets a scope emits no $mcp_* event. This is the only +// record of it. +export type McpConnectionScopeFailureAnalytics = { + outcome: "rejected" | "unavailable" | "invalid"; + credentialType: "api_key" | "oauth"; + upstreamStatusCode?: number; +}; + +export const MCP_CONNECTION_SCOPE_FAILURE_EVENT = + "mcp_connection_scope_failure"; + if (!projectToken && process.env.NODE_ENV !== "production") { console.error( "POSTHOG_PROJECT_TOKEN variable required by PostHog is missing or un-configured, this causes events to be silently missed. This error stops appearing once POSTHOG_PROJECT_TOKEN is configured", @@ -315,6 +327,32 @@ export function captureOAuthTokenExchange( } } +export function captureMcpConnectionScopeFailure( + failure: McpConnectionScopeFailureAnalytics, + client: PostHog | null = posthog, +) { + if (!client) return; + + const properties = { + connection_scope_outcome: failure.outcome, + connection_credential_type: failure.credentialType, + upstream_status_code: failure.upstreamStatusCode, + }; + + try { + client.capture({ + distinctId: "mcp-connection-scope", + event: MCP_CONNECTION_SCOPE_FAILURE_EVENT, + properties: { + $process_person_profile: false, + ...properties, + }, + }); + } catch (error) { + console.error("Failed to capture MCP connection scope analytics", error); + } +} + export function instrumentMcpAnalytics( server: McpServer, client: PostHog | null = posthog, diff --git a/src/lib/mcp/auth-context.test.ts b/src/lib/mcp/auth-context.test.ts index f4683db..b183212 100644 --- a/src/lib/mcp/auth-context.test.ts +++ b/src/lib/mcp/auth-context.test.ts @@ -6,6 +6,8 @@ import { connectionScopeFromAuthContext, expireMcpConnectionContextCacheForTests, resolveMcpConnectionContext, + type McpConnectionContext, + type McpConnectionContextResult, } from "@/lib/mcp/auth-context"; function response({ @@ -49,22 +51,40 @@ function dependencies(body: unknown, calls?: string[]) { }; } +function expectResolved( + result: McpConnectionContextResult, +): McpConnectionContext { + expect(result.status).toBe("ok"); + if (result.status !== "ok") throw new Error("expected a resolved scope"); + return result.context; +} + +function rejection(status: number) { + return { + createKernelClient: () => { + throw Object.assign(new Error("rejected"), { status }); + }, + }; +} + afterEach(clearMcpConnectionContextCacheForTests); describe("resolveMcpConnectionContext", () => { test("normalizes organization-wide API-key scope", async () => { - const context = await resolveMcpConnectionContext({ - token: "sk_secret", - dependencies: dependencies(response()), - }); + const context = expectResolved( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: dependencies(response()), + }), + ); - expect(context?.scope).toEqual({ + expect(context.scope).toEqual({ kind: "organization", organizationId: "org_123", projectId: null, source: "credential", }); - const analytics = connectionAnalyticsFromContext(context!); + const analytics = connectionAnalyticsFromContext(context); expect(analytics).toEqual({ authMethod: "api_key", credentialScope: "organization", @@ -77,42 +97,46 @@ describe("resolveMcpConnectionContext", () => { }); test("normalizes project-scoped API-key scope", async () => { - const context = await resolveMcpConnectionContext({ - token: "sk_secret", - dependencies: dependencies( - response({ - credentialProjectId: "project_123", - effectiveProjectId: "project_123", - }), - ), - }); + const context = expectResolved( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: dependencies( + response({ + credentialProjectId: "project_123", + effectiveProjectId: "project_123", + }), + ), + }), + ); - expect(context?.scope).toEqual({ + expect(context.scope).toEqual({ kind: "project", organizationId: "org_123", projectId: "project_123", source: "credential", }); - expect(connectionAnalyticsFromContext(context!)?.credentialScope).toBe( + expect(connectionAnalyticsFromContext(context)?.credentialScope).toBe( "project", ); }); test("uses the canonical OAuth user principal for analytics", async () => { - const context = await resolveMcpConnectionContext({ - token: "jwt.secret.value", - dependencies: dependencies( - response({ - method: "jwt", - source: "oauth", - principalType: "user", - principalId: "user_kernel_123", - }), - ), - }); + const context = expectResolved( + await resolveMcpConnectionContext({ + token: "jwt.secret.value", + dependencies: dependencies( + response({ + method: "jwt", + source: "oauth", + principalType: "user", + principalId: "user_kernel_123", + }), + ), + }), + ); - expect(connectionAnalyticsFromContext(context!)?.authMethod).toBe("oauth"); - expect(connectionAnalyticsFromContext(context!)?.userId).toBe( + expect(connectionAnalyticsFromContext(context)?.authMethod).toBe("oauth"); + expect(connectionAnalyticsFromContext(context)?.userId).toBe( "user_kernel_123", ); }); @@ -155,16 +179,20 @@ describe("resolveMcpConnectionContext", () => { test("reuses normalized scope across token refreshes in one session", async () => { const calls: string[] = []; - const first = await resolveMcpConnectionContext({ - token: "old-token", - cacheIdentity: "user_123\0session_123", - dependencies: dependencies(response(), calls), - }); - const refreshed = await resolveMcpConnectionContext({ - token: "new-token", - cacheIdentity: "user_123\0session_123", - dependencies: dependencies(response(), calls), - }); + const first = expectResolved( + await resolveMcpConnectionContext({ + token: "old-token", + cacheIdentity: "user_123\0session_123", + dependencies: dependencies(response(), calls), + }), + ); + const refreshed = expectResolved( + await resolveMcpConnectionContext({ + token: "new-token", + cacheIdentity: "user_123\0session_123", + dependencies: dependencies(response(), calls), + }), + ); expect(refreshed).toBe(first); expect(calls).toEqual(["old-token"]); @@ -186,21 +214,25 @@ describe("resolveMcpConnectionContext", () => { test("retains resolved scope during a transient refresh failure", async () => { const cacheIdentity = "user_123\0session_123"; - const first = await resolveMcpConnectionContext({ - token: "old-token", - cacheIdentity, - dependencies: dependencies(response()), - }); + const first = expectResolved( + await resolveMcpConnectionContext({ + token: "old-token", + cacheIdentity, + dependencies: dependencies(response()), + }), + ); expireMcpConnectionContextCacheForTests(); - const refreshed = await resolveMcpConnectionContext({ - token: "new-token", - cacheIdentity, - dependencies: { - createKernelClient: () => { - throw new Error("temporary outage"); + const refreshed = expectResolved( + await resolveMcpConnectionContext({ + token: "new-token", + cacheIdentity, + dependencies: { + createKernelClient: () => { + throw new Error("temporary outage"); + }, }, - }, - }); + }), + ); expect(refreshed).toBe(first); }); @@ -216,14 +248,10 @@ describe("resolveMcpConnectionContext", () => { const rejected = await resolveMcpConnectionContext({ token: "revoked-token", cacheIdentity, - dependencies: { - createKernelClient: () => { - throw Object.assign(new Error("revoked"), { status: 401 }); - }, - }, + dependencies: rejection(401), }); - expect(rejected).toBeNull(); + expect(rejected).toEqual({ status: "rejected", statusCode: 401 }); }); test("invalidates cached scope after an inconsistent response", async () => { @@ -254,8 +282,8 @@ describe("resolveMcpConnectionContext", () => { }, }); - expect(inconsistent).toBeNull(); - expect(transient).toBeNull(); + expect(inconsistent).toEqual({ status: "invalid" }); + expect(transient).toEqual({ status: "unavailable" }); }); test("fails closed when auth context is unavailable or malformed", async () => { @@ -272,7 +300,30 @@ describe("resolveMcpConnectionContext", () => { dependencies: dependencies({ authentication: {} }), }); - expect(unavailable).toBeNull(); - expect(malformed).toBeNull(); + expect(unavailable).toEqual({ status: "unavailable" }); + expect(malformed).toEqual({ status: "invalid" }); + }); + + test("separates a rejected credential from an unresolvable scope", async () => { + expect( + await resolveMcpConnectionContext({ + token: "revoked", + dependencies: rejection(401), + }), + ).toEqual({ status: "rejected", statusCode: 401 }); + expect( + await resolveMcpConnectionContext({ + token: "wrong-project", + dependencies: rejection(403), + }), + ).toEqual({ status: "rejected", statusCode: 403 }); + for (const status of [408, 429, 500, 502, 503]) { + expect( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: rejection(status), + }), + ).toEqual({ status: "unavailable" }); + } }); }); diff --git a/src/lib/mcp/auth-context.ts b/src/lib/mcp/auth-context.ts index c2e7c01..9f70584 100644 --- a/src/lib/mcp/auth-context.ts +++ b/src/lib/mcp/auth-context.ts @@ -62,17 +62,33 @@ type ResolveAuthContextOptions = { cacheIdentity?: string; }; +// Why the connection has no scope. "rejected" belongs to the caller's +// credential, "unavailable" is retryable, and "invalid" means the Kernel API +// answered with something we cannot normalize. +export type McpConnectionContextFailure = + | { status: "rejected"; statusCode: number } + | { status: "unavailable" } + | { status: "invalid" }; + +export type McpConnectionContextResult = + | { status: "ok"; context: McpConnectionContext } + | McpConnectionContextFailure; + type AuthContextResolution = - | { context: AuthContext; transientFailure: false } - | { context: null; transientFailure: boolean }; + | { context: AuthContext; failure: null } + | { context: null; failure: McpConnectionContextFailure }; -function isTransientAuthContextError(error: unknown) { +function classifyAuthContextError(error: unknown): McpConnectionContextFailure { const status = error && typeof error === "object" && "status" in error ? (error as { status?: unknown }).status : undefined; - if (typeof status !== "number") return true; - return status === 408 || status === 429 || status >= 500; + if (typeof status !== "number") return { status: "unavailable" }; + if (status === 408 || status === 429 || status >= 500) { + return { status: "unavailable" }; + } + if (status >= 400) return { status: "rejected", statusCode: status }; + return { status: "invalid" }; } async function resolveMcpAuthContext({ @@ -86,19 +102,16 @@ async function resolveMcpAuthContext({ .auth.context.retrieve({ signal }); const parsed = authContextSchema.safeParse(context); if (parsed.success) { - return { context: parsed.data, transientFailure: false }; + return { context: parsed.data, failure: null }; } console.warn("Received invalid MCP auth context", parsed.error.issues); - return { context: null, transientFailure: false }; + return { context: null, failure: { status: "invalid" } }; } catch (error) { console.warn( "Failed to resolve MCP auth context", error instanceof Error ? error.message : error, ); - return { - context: null, - transientFailure: isTransientAuthContextError(error), - }; + return { context: null, failure: classifyAuthContextError(error) }; } } @@ -187,10 +200,10 @@ export async function resolveMcpConnectionContext({ signal, dependencies, cacheIdentity, -}: ResolveAuthContextOptions): Promise { +}: ResolveAuthContextOptions): Promise { if (cacheIdentity) { const cached = readConnectionContextCache(cacheIdentity); - if (cached) return cached; + if (cached) return { status: "ok", context: cached }; } const stale = cacheIdentity ? readConnectionContextCache(cacheIdentity, true) @@ -202,10 +215,14 @@ export async function resolveMcpConnectionContext({ dependencies, }); if (!resolution.context) { - if (cacheIdentity && !resolution.transientFailure) { + const { failure } = resolution; + if (cacheIdentity && failure.status !== "unavailable") { connectionContextCache.delete(connectionContextCacheKey(cacheIdentity)); } - return resolution.transientFailure ? stale : null; + if (failure.status === "unavailable" && stale) { + return { status: "ok", context: stale }; + } + return failure; } const scope = connectionScopeFromAuthContext(resolution.context); @@ -214,12 +231,12 @@ export async function resolveMcpConnectionContext({ if (cacheIdentity) { connectionContextCache.delete(connectionContextCacheKey(cacheIdentity)); } - return null; + return { status: "invalid" }; } const context = { authContext: resolution.context, scope }; if (cacheIdentity) writeConnectionContextCache(cacheIdentity, context); - return context; + return { status: "ok", context }; } export function clearMcpConnectionContextCacheForTests() { From 3f3cc6688c1859bb49e0d4d52383b9bfd4393653 Mon Sep 17 00:00:00 2001 From: masnwilliams <43387599+masnwilliams@users.noreply.github.com> Date: Wed, 12 Aug 2026 23:56:50 +0000 Subject: [PATCH 2/4] Distinguish upstream rejection reasons when resolving connection scope --- src/app/[transport]/route.test.ts | 164 +++++++++++++++++++++++++++--- src/app/[transport]/route.ts | 90 +++++++++------- src/lib/mcp/analytics.test.ts | 42 ++++++++ src/lib/mcp/auth-context.test.ts | 101 +++++++++++++----- src/lib/mcp/auth-context.ts | 29 +++++- 5 files changed, 343 insertions(+), 83 deletions(-) diff --git a/src/app/[transport]/route.test.ts b/src/app/[transport]/route.test.ts index 09f2265..6647b41 100644 --- a/src/app/[transport]/route.test.ts +++ b/src/app/[transport]/route.test.ts @@ -1,32 +1,168 @@ -import { describe, expect, test } from "bun:test"; +import { beforeEach, describe, expect, mock, test } from "bun:test"; +import { APIConnectionError } from "@onkernel/sdk"; +import type { McpConnectionScopeFailureAnalytics } from "@/lib/mcp/analytics"; +import { + kernelClientMock, + resetKernelClientFactory, +} from "@/lib/mcp/kernel-client.test-fixtures"; -const { connectionScopeFailureResponse } = await import("./route"); +process.env.CLERK_SECRET_KEY ??= "test-clerk-secret"; -describe("connectionScopeFailureResponse", () => { - test("answers a rejected credential with 401 rather than a server error", async () => { - const response = connectionScopeFailureResponse({ - status: "rejected", - statusCode: 401, - }); +// after() needs a Next request scope only the server provides. The route uses +// it to flush analytics, which these tests observe directly instead. +const nextServer = await import("next/server"); +mock.module("next/server", () => ({ ...nextServer, after: () => {} })); + +mock.module("@/lib/redis", () => ({ + hasMcpAppsClient: async () => false, + markMcpAppsClient: async () => {}, + clearMcpAppsClient: async () => {}, +})); + +// Scope resolution runs before the instrumented McpServer exists, so this +// capture is the only record of it. Record the calls and delegate, so the +// module stays intact for anything else exercising it. +const captured: McpConnectionScopeFailureAnalytics[] = []; +const analytics = await import("@/lib/mcp/analytics"); +mock.module("@/lib/mcp/analytics", () => ({ + ...analytics, + captureMcpConnectionScopeFailure: ( + failure: McpConnectionScopeFailureAnalytics, + ) => { + captured.push(failure); + return analytics.captureMcpConnectionScopeFailure(failure); + }, +})); + +const { POST, connectionScopeFailureResponse } = await import("./route"); + +function initializeRequest(token = "sk_opaque_key") { + return new Request("https://mcp.example.test/mcp", { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-06-18", + capabilities: {}, + clientInfo: { name: "route-test", version: "0" }, + }, + }), + }) as never; +} + +function failingAuthContext(error: unknown) { + kernelClientMock.factory = () => + ({ + auth: { + context: { + retrieve: async () => { + throw error; + }, + }, + }, + }) as never; +} + +beforeEach(() => { + resetKernelClientFactory(); + captured.length = 0; +}); + +describe("connection scope failures through the handler", () => { + test("answers a refused credential with 401 rather than a server error", async () => { + failingAuthContext(Object.assign(new Error("revoked"), { status: 401 })); + + const response = await POST(initializeRequest()); expect(response.status).toBe(401); - expect(response.headers.get("WWW-Authenticate")).toContain( - 'error="invalid_token"', - ); expect(await response.json()).toEqual({ error: "invalid_token", error_description: "The Kernel API rejected this credential", }); + expect(captured).toEqual([ + { + outcome: "rejected", + credentialType: "api_key", + upstreamStatusCode: 401, + }, + ]); + }); + + test("keeps a credential scoped to another project usable", async () => { + failingAuthContext( + Object.assign(new Error("other project"), { status: 403 }), + ); + + const response = await POST(initializeRequest()); + + expect(response.status).toBe(403); + expect((await response.json()).error).toBe("insufficient_scope"); + expect(captured[0]?.upstreamStatusCode).toBe(403); }); test("answers an unresolvable scope with a retryable 503", async () => { - const response = connectionScopeFailureResponse({ status: "unavailable" }); + failingAuthContext(new APIConnectionError({ message: "socket hang up" })); + + const response = await POST(initializeRequest()); expect(response.status).toBe(503); expect(response.headers.get("Retry-After")).toBe("1"); + expect(captured).toEqual([ + { + outcome: "unavailable", + credentialType: "api_key", + upstreamStatusCode: undefined, + }, + ]); + }); + + test("still surfaces an unaccountable failure as a server error", async () => { + failingAuthContext(new TypeError("cannot read properties of undefined")); + + await expect(POST(initializeRequest())).rejects.toThrow( + "Unable to resolve Kernel connection scope", + ); + expect(captured).toEqual([ + { + outcome: "invalid", + credentialType: "api_key", + upstreamStatusCode: undefined, + }, + ]); + }); +}); + +describe("connectionScopeFailureResponse", () => { + test("names an inactive project instead of blaming the credential", async () => { + const response = connectionScopeFailureResponse({ + status: "rejected", + statusCode: 404, + }); + + expect(response.status).toBe(404); + expect(response.headers.get("WWW-Authenticate")).toBeNull(); expect(await response.json()).toEqual({ - error: "temporarily_unavailable", - error_description: "Unable to resolve Kernel connection scope", + error: "project_not_found", + error_description: + "The Kernel project for this connection was not found or is inactive", }); }); + + test("challenges a refused credential so clients re-authenticate", () => { + const response = connectionScopeFailureResponse({ + status: "rejected", + statusCode: 401, + }); + + expect(response.headers.get("WWW-Authenticate")).toContain( + 'error="invalid_token"', + ); + }); }); diff --git a/src/app/[transport]/route.ts b/src/app/[transport]/route.ts index b023948..0dba3ea 100644 --- a/src/app/[transport]/route.ts +++ b/src/app/[transport]/route.ts @@ -38,56 +38,72 @@ export async function OPTIONS(_req: NextRequest): Promise { }); } +const CORS_HEADERS = { + "Content-Type": "application/json", + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Methods": "GET, POST, OPTIONS", + "Access-Control-Allow-Headers": "Content-Type, Authorization", +}; + +function errorResponse( + status: number, + error: string, + description: string, + headers: Record = {}, +): Response { + return new Response( + JSON.stringify({ error, error_description: description }), + { status, headers: { ...CORS_HEADERS, ...headers } }, + ); +} + // Helper function to create authentication error response function createAuthErrorResponse( error: string = "invalid_token", description: string = "Missing or invalid access token", ): Response { - return new Response( - JSON.stringify({ - error, - error_description: description, - }), - { - status: 401, - headers: { - "WWW-Authenticate": `Bearer realm="OAuth", error="${error}", error_description="${description}"`, - "Content-Type": "application/json", - "Access-Control-Allow-Origin": "*", - "Access-Control-Allow-Methods": "GET, POST, OPTIONS", - "Access-Control-Allow-Headers": "Content-Type, Authorization", - }, - }, - ); + return errorResponse(401, error, description, { + "WWW-Authenticate": `Bearer realm="OAuth", error="${error}", error_description="${description}"`, + }); } // A credential the Kernel API rejects is the caller's problem, and a scope we // cannot resolve right now is worth retrying. Neither is a server fault, so -// neither should reach the error handler as a thrown 500. +// neither should reach the error handler as a thrown 500. Each rejection keeps +// the upstream meaning: only 401 tells a client its credential is bad, so only +// 401 may prompt it to discard one. export function connectionScopeFailureResponse( failure: Exclude, ): Response { if (failure.status === "rejected") { - return createAuthErrorResponse( - "invalid_token", - "The Kernel API rejected this credential", - ); + switch (failure.statusCode) { + case 403: + return errorResponse( + 403, + "insufficient_scope", + "This credential is not scoped to the requested Kernel project", + { + "WWW-Authenticate": `Bearer realm="OAuth", error="insufficient_scope"`, + }, + ); + case 404: + return errorResponse( + 404, + "project_not_found", + "The Kernel project for this connection was not found or is inactive", + ); + default: + return createAuthErrorResponse( + "invalid_token", + "The Kernel API rejected this credential", + ); + } } - return new Response( - JSON.stringify({ - error: "temporarily_unavailable", - error_description: "Unable to resolve Kernel connection scope", - }), - { - status: 503, - headers: { - "Retry-After": "1", - "Content-Type": "application/json", - "Access-Control-Allow-Origin": "*", - "Access-Control-Allow-Methods": "GET, POST, OPTIONS", - "Access-Control-Allow-Headers": "Content-Type, Authorization", - }, - }, + return errorResponse( + 503, + "temporarily_unavailable", + "Unable to resolve Kernel connection scope", + { "Retry-After": "1" }, ); } @@ -147,7 +163,7 @@ async function handleMcpRequestWithIdentity({ outcome: connection.status, credentialType, upstreamStatusCode: - connection.status === "rejected" ? connection.statusCode : undefined, + connection.status === "invalid" ? undefined : connection.statusCode, }); if (connection.status === "invalid") { throw new Error("Unable to resolve Kernel connection scope"); diff --git a/src/lib/mcp/analytics.test.ts b/src/lib/mcp/analytics.test.ts index 2088b8b..7a6bdeb 100644 --- a/src/lib/mcp/analytics.test.ts +++ b/src/lib/mcp/analytics.test.ts @@ -6,9 +6,11 @@ import { PostHogMCPAnalyticsProperty, } from "@posthog/mcp"; import { + captureMcpConnectionScopeFailure, captureOAuthTokenExchange, enrichMcpAnalyticsEvent, instrumentMcpAnalytics, + MCP_CONNECTION_SCOPE_FAILURE_EVENT, OAUTH_TOKEN_EXCHANGE_EVENT, sanitizeMcpAnalyticsEvent, } from "@/lib/mcp/analytics"; @@ -328,6 +330,46 @@ describe("captureOAuthTokenExchange", () => { }); }); +describe("captureMcpConnectionScopeFailure", () => { + test("records the outcome without the credential it was resolving", () => { + const captured: unknown[] = []; + const fakePosthog = { + capture: (event: unknown) => captured.push(event), + } as unknown as PostHog; + + captureMcpConnectionScopeFailure( + { + outcome: "rejected", + credentialType: "api_key", + upstreamStatusCode: 403, + }, + fakePosthog, + ); + + expect(captured).toEqual([ + { + distinctId: "mcp-connection-scope", + event: MCP_CONNECTION_SCOPE_FAILURE_EVENT, + properties: { + $process_person_profile: false, + connection_scope_outcome: "rejected", + connection_credential_type: "api_key", + upstream_status_code: 403, + }, + }, + ]); + }); + + test("is a no-op without a configured client", () => { + expect(() => + captureMcpConnectionScopeFailure( + { outcome: "unavailable", credentialType: "oauth" }, + null, + ), + ).not.toThrow(); + }); +}); + describe("instrumentMcpAnalytics (SDK integration)", () => { const ORG = "org_integration"; diff --git a/src/lib/mcp/auth-context.test.ts b/src/lib/mcp/auth-context.test.ts index b183212..72bab09 100644 --- a/src/lib/mcp/auth-context.test.ts +++ b/src/lib/mcp/auth-context.test.ts @@ -1,4 +1,9 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { + APIConnectionError, + APIConnectionTimeoutError, + APIUserAbortError, +} from "@onkernel/sdk"; import type { KernelClient } from "@/lib/mcp/kernel-client"; import { clearMcpConnectionContextCacheForTests, @@ -67,6 +72,14 @@ function rejection(status: number) { }; } +function throwing(error: unknown) { + return { + createKernelClient: (): KernelClient => { + throw error; + }, + }; +} + afterEach(clearMcpConnectionContextCacheForTests); describe("resolveMcpConnectionContext", () => { @@ -226,11 +239,9 @@ describe("resolveMcpConnectionContext", () => { await resolveMcpConnectionContext({ token: "new-token", cacheIdentity, - dependencies: { - createKernelClient: () => { - throw new Error("temporary outage"); - }, - }, + dependencies: throwing( + new APIConnectionError({ message: "temporary outage" }), + ), }), ); @@ -275,11 +286,9 @@ describe("resolveMcpConnectionContext", () => { const transient = await resolveMcpConnectionContext({ token: "newer-token", cacheIdentity, - dependencies: { - createKernelClient: () => { - throw new Error("temporary outage"); - }, - }, + dependencies: throwing( + new APIConnectionError({ message: "temporary outage" }), + ), }); expect(inconsistent).toEqual({ status: "invalid" }); @@ -289,11 +298,9 @@ describe("resolveMcpConnectionContext", () => { test("fails closed when auth context is unavailable or malformed", async () => { const unavailable = await resolveMcpConnectionContext({ token: "sk_secret", - dependencies: { - createKernelClient: () => { - throw new Error("API unavailable"); - }, - }, + dependencies: throwing( + new APIConnectionError({ message: "API unavailable" }), + ), }); const malformed = await resolveMcpConnectionContext({ token: "sk_secret", @@ -304,26 +311,64 @@ describe("resolveMcpConnectionContext", () => { expect(malformed).toEqual({ status: "invalid" }); }); - test("separates a rejected credential from an unresolvable scope", async () => { - expect( - await resolveMcpConnectionContext({ - token: "revoked", - dependencies: rejection(401), - }), - ).toEqual({ status: "rejected", statusCode: 401 }); - expect( - await resolveMcpConnectionContext({ - token: "wrong-project", - dependencies: rejection(403), - }), - ).toEqual({ status: "rejected", statusCode: 403 }); + test("keeps each answer the Kernel API gives about a credential distinct", async () => { + for (const status of [401, 403, 404]) { + expect( + await resolveMcpConnectionContext({ + token: "credential", + dependencies: rejection(status), + }), + ).toEqual({ status: "rejected", statusCode: status }); + } + }); + + test("treats retryable statuses as unavailable and keeps the upstream status", async () => { for (const status of [408, 429, 500, 502, 503]) { expect( await resolveMcpConnectionContext({ token: "sk_secret", dependencies: rejection(status), }), + ).toEqual({ status: "unavailable", statusCode: status }); + } + }); + + test("treats a request the API could not understand as our bug", async () => { + for (const status of [400, 409, 422]) { + expect( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: rejection(status), + }), + ).toEqual({ status: "invalid" }); + } + }); + + test("retries transport failures but surfaces unknown throws", async () => { + for (const error of [ + new APIConnectionError({ message: "socket hang up" }), + new APIConnectionTimeoutError({ message: "timed out" }), + new APIUserAbortError({ message: "client went away" }), + ]) { + expect( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: throwing(error), + }), ).toEqual({ status: "unavailable" }); } + + for (const error of [ + new TypeError("cannot read properties of undefined"), + new Error("unexpected"), + "not even an error", + ]) { + expect( + await resolveMcpConnectionContext({ + token: "sk_secret", + dependencies: throwing(error), + }), + ).toEqual({ status: "invalid" }); + } }); }); diff --git a/src/lib/mcp/auth-context.ts b/src/lib/mcp/auth-context.ts index 9f70584..82194f9 100644 --- a/src/lib/mcp/auth-context.ts +++ b/src/lib/mcp/auth-context.ts @@ -1,3 +1,4 @@ +import { APIConnectionError, APIUserAbortError } from "@onkernel/sdk"; import type { AuthContext } from "@onkernel/sdk/resources/auth/context"; import { createHash } from "crypto"; import { z } from "zod"; @@ -67,7 +68,7 @@ type ResolveAuthContextOptions = { // answered with something we cannot normalize. export type McpConnectionContextFailure = | { status: "rejected"; statusCode: number } - | { status: "unavailable" } + | { status: "unavailable"; statusCode?: number } | { status: "invalid" }; export type McpConnectionContextResult = @@ -78,16 +79,36 @@ type AuthContextResolution = | { context: AuthContext; failure: null } | { context: null; failure: McpConnectionContextFailure }; +// The Kernel API answers about a credential with 401 (unauthenticated), 403 +// (authenticated but not entitled to this project) or 404 (project missing or +// inactive). Any other 4xx means we sent a request it could not understand, +// which is our bug rather than the caller's. +const REJECTION_STATUSES = new Set([401, 403, 404]); + function classifyAuthContextError(error: unknown): McpConnectionContextFailure { const status = error && typeof error === "object" && "status" in error ? (error as { status?: unknown }).status : undefined; - if (typeof status !== "number") return { status: "unavailable" }; - if (status === 408 || status === 429 || status >= 500) { + + if (typeof status === "number") { + if (REJECTION_STATUSES.has(status)) { + return { status: "rejected", statusCode: status }; + } + if (status === 408 || status === 429 || status >= 500) { + return { status: "unavailable", statusCode: status }; + } + return { status: "invalid" }; + } + + // Only a failure to reach the API is worth retrying. Anything else thrown + // without a status is a bug on our side and must keep surfacing as one. + if ( + error instanceof APIConnectionError || + error instanceof APIUserAbortError + ) { return { status: "unavailable" }; } - if (status >= 400) return { status: "rejected", statusCode: status }; return { status: "invalid" }; } From 7502e30da46a65b22d26689e52cb99ed1b26682a Mon Sep 17 00:00:00 2001 From: masnwilliams <43387599+masnwilliams@users.noreply.github.com> Date: Thu, 13 Aug 2026 00:13:29 +0000 Subject: [PATCH 3/4] Stabilize connection scope failure tests --- src/app/[transport]/route.test.ts | 38 ++++++++++++++----------------- src/app/[transport]/route.ts | 7 +----- src/lib/mcp/auth-context.test.ts | 2 +- src/lib/mcp/auth-context.ts | 9 ++------ 4 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/app/[transport]/route.test.ts b/src/app/[transport]/route.test.ts index 6647b41..74334f1 100644 --- a/src/app/[transport]/route.test.ts +++ b/src/app/[transport]/route.test.ts @@ -1,10 +1,6 @@ -import { beforeEach, describe, expect, mock, test } from "bun:test"; -import { APIConnectionError } from "@onkernel/sdk"; +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import type { McpConnectionScopeFailureAnalytics } from "@/lib/mcp/analytics"; -import { - kernelClientMock, - resetKernelClientFactory, -} from "@/lib/mcp/kernel-client.test-fixtures"; +import { defaultMcpDependencies } from "@/lib/mcp/dependencies"; process.env.CLERK_SECRET_KEY ??= "test-clerk-secret"; @@ -13,12 +9,6 @@ process.env.CLERK_SECRET_KEY ??= "test-clerk-secret"; const nextServer = await import("next/server"); mock.module("next/server", () => ({ ...nextServer, after: () => {} })); -mock.module("@/lib/redis", () => ({ - hasMcpAppsClient: async () => false, - markMcpAppsClient: async () => {}, - clearMcpAppsClient: async () => {}, -})); - // Scope resolution runs before the instrumented McpServer exists, so this // capture is the only record of it. Record the calls and delegate, so the // module stays intact for anything else exercising it. @@ -27,17 +17,18 @@ const analytics = await import("@/lib/mcp/analytics"); mock.module("@/lib/mcp/analytics", () => ({ ...analytics, captureMcpConnectionScopeFailure: ( - failure: McpConnectionScopeFailureAnalytics, + ...args: Parameters ) => { - captured.push(failure); - return analytics.captureMcpConnectionScopeFailure(failure); + captured.push(args[0]); + return analytics.captureMcpConnectionScopeFailure(...args); }, })); +const originalCreateKernelClient = defaultMcpDependencies.createKernelClient; const { POST, connectionScopeFailureResponse } = await import("./route"); function initializeRequest(token = "sk_opaque_key") { - return new Request("https://mcp.example.test/mcp", { + return new Request("https://mcp.example.test/sse", { method: "POST", headers: { Authorization: `Bearer ${token}`, @@ -58,7 +49,7 @@ function initializeRequest(token = "sk_opaque_key") { } function failingAuthContext(error: unknown) { - kernelClientMock.factory = () => + defaultMcpDependencies.createKernelClient = () => ({ auth: { context: { @@ -71,10 +62,13 @@ function failingAuthContext(error: unknown) { } beforeEach(() => { - resetKernelClientFactory(); captured.length = 0; }); +afterEach(() => { + defaultMcpDependencies.createKernelClient = originalCreateKernelClient; +}); + describe("connection scope failures through the handler", () => { test("answers a refused credential with 401 rather than a server error", async () => { failingAuthContext(Object.assign(new Error("revoked"), { status: 401 })); @@ -107,8 +101,10 @@ describe("connection scope failures through the handler", () => { expect(captured[0]?.upstreamStatusCode).toBe(403); }); - test("answers an unresolvable scope with a retryable 503", async () => { - failingAuthContext(new APIConnectionError({ message: "socket hang up" })); + test("answers an upstream outage with a retryable 503", async () => { + failingAuthContext( + Object.assign(new Error("unavailable"), { status: 502 }), + ); const response = await POST(initializeRequest()); @@ -118,7 +114,7 @@ describe("connection scope failures through the handler", () => { { outcome: "unavailable", credentialType: "api_key", - upstreamStatusCode: undefined, + upstreamStatusCode: 502, }, ]); }); diff --git a/src/app/[transport]/route.ts b/src/app/[transport]/route.ts index 0dba3ea..e32f279 100644 --- a/src/app/[transport]/route.ts +++ b/src/app/[transport]/route.ts @@ -67,11 +67,6 @@ function createAuthErrorResponse( }); } -// A credential the Kernel API rejects is the caller's problem, and a scope we -// cannot resolve right now is worth retrying. Neither is a server fault, so -// neither should reach the error handler as a thrown 500. Each rejection keeps -// the upstream meaning: only 401 tells a client its credential is bad, so only -// 401 may prompt it to discard one. export function connectionScopeFailureResponse( failure: Exclude, ): Response { @@ -92,7 +87,7 @@ export function connectionScopeFailureResponse( "project_not_found", "The Kernel project for this connection was not found or is inactive", ); - default: + case 401: return createAuthErrorResponse( "invalid_token", "The Kernel API rejected this credential", diff --git a/src/lib/mcp/auth-context.test.ts b/src/lib/mcp/auth-context.test.ts index 72bab09..295ab93 100644 --- a/src/lib/mcp/auth-context.test.ts +++ b/src/lib/mcp/auth-context.test.ts @@ -312,7 +312,7 @@ describe("resolveMcpConnectionContext", () => { }); test("keeps each answer the Kernel API gives about a credential distinct", async () => { - for (const status of [401, 403, 404]) { + for (const status of [401, 403, 404] as const) { expect( await resolveMcpConnectionContext({ token: "credential", diff --git a/src/lib/mcp/auth-context.ts b/src/lib/mcp/auth-context.ts index 82194f9..79b264b 100644 --- a/src/lib/mcp/auth-context.ts +++ b/src/lib/mcp/auth-context.ts @@ -63,11 +63,8 @@ type ResolveAuthContextOptions = { cacheIdentity?: string; }; -// Why the connection has no scope. "rejected" belongs to the caller's -// credential, "unavailable" is retryable, and "invalid" means the Kernel API -// answered with something we cannot normalize. export type McpConnectionContextFailure = - | { status: "rejected"; statusCode: number } + | { status: "rejected"; statusCode: 401 | 403 | 404 } | { status: "unavailable"; statusCode?: number } | { status: "invalid" }; @@ -83,8 +80,6 @@ type AuthContextResolution = // (authenticated but not entitled to this project) or 404 (project missing or // inactive). Any other 4xx means we sent a request it could not understand, // which is our bug rather than the caller's. -const REJECTION_STATUSES = new Set([401, 403, 404]); - function classifyAuthContextError(error: unknown): McpConnectionContextFailure { const status = error && typeof error === "object" && "status" in error @@ -92,7 +87,7 @@ function classifyAuthContextError(error: unknown): McpConnectionContextFailure { : undefined; if (typeof status === "number") { - if (REJECTION_STATUSES.has(status)) { + if (status === 401 || status === 403 || status === 404) { return { status: "rejected", statusCode: status }; } if (status === 408 || status === 429 || status >= 500) { From 5ca48b7a8bd69d45a731be30f3962869088d0a22 Mon Sep 17 00:00:00 2001 From: masnwilliams <43387599+masnwilliams@users.noreply.github.com> Date: Thu, 13 Aug 2026 00:16:45 +0000 Subject: [PATCH 4/4] Fix analytics capture test delegation --- src/app/[transport]/route.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/[transport]/route.test.ts b/src/app/[transport]/route.test.ts index 74334f1..b4d3134 100644 --- a/src/app/[transport]/route.test.ts +++ b/src/app/[transport]/route.test.ts @@ -14,13 +14,15 @@ mock.module("next/server", () => ({ ...nextServer, after: () => {} })); // module stays intact for anything else exercising it. const captured: McpConnectionScopeFailureAnalytics[] = []; const analytics = await import("@/lib/mcp/analytics"); +const captureMcpConnectionScopeFailure = + analytics.captureMcpConnectionScopeFailure; mock.module("@/lib/mcp/analytics", () => ({ ...analytics, captureMcpConnectionScopeFailure: ( ...args: Parameters ) => { captured.push(args[0]); - return analytics.captureMcpConnectionScopeFailure(...args); + return captureMcpConnectionScopeFailure(...args); }, }));