Przeglądaj źródła

fix(httpapi): preserve mcp oauth error parity (#24706)

Kit Langton 3 miesięcy temu
rodzic
commit
796b652d2b

+ 12 - 4
packages/opencode/src/server/routes/instance/httpapi/mcp.ts

@@ -20,6 +20,10 @@ const AuthCallbackPayload = Schema.Struct({
 const AuthRemoveResponse = Schema.Struct({
   success: Schema.Literal(true),
 }).annotate({ identifier: "McpAuthRemoveResponse" })
+class UnsupportedOAuthError extends Schema.ErrorClass<UnsupportedOAuthError>("McpUnsupportedOAuthError")(
+  { error: Schema.String },
+  { httpApiStatus: 400 },
+) {}
 
 export const McpPaths = {
   status: "/mcp",
@@ -57,7 +61,7 @@ export const McpApi = HttpApi.make("mcp")
         HttpApiEndpoint.post("authStart", McpPaths.auth, {
           params: { name: Schema.String },
           success: AuthStartResponse,
-          error: HttpApiError.BadRequest,
+          error: UnsupportedOAuthError,
         }).annotateMerge(
           OpenApi.annotations({
             identifier: "mcp.auth.start",
@@ -80,7 +84,7 @@ export const McpApi = HttpApi.make("mcp")
         HttpApiEndpoint.post("authAuthenticate", McpPaths.authAuthenticate, {
           params: { name: Schema.String },
           success: MCP.Status,
-          error: HttpApiError.BadRequest,
+          error: UnsupportedOAuthError,
         }).annotateMerge(
           OpenApi.annotations({
             identifier: "mcp.auth.authenticate",
@@ -149,7 +153,9 @@ export const mcpHandlers = Layer.unwrap(
     })
 
     const authStart = Effect.fn("McpHttpApi.authStart")(function* (ctx: { params: { name: string } }) {
-      if (!(yield* mcp.supportsOAuth(ctx.params.name))) return yield* new HttpApiError.BadRequest({})
+      if (!(yield* mcp.supportsOAuth(ctx.params.name))) {
+        return yield* new UnsupportedOAuthError({ error: `MCP server ${ctx.params.name} does not support OAuth` })
+      }
       return yield* mcp.startAuth(ctx.params.name)
     })
 
@@ -161,7 +167,9 @@ export const mcpHandlers = Layer.unwrap(
     })
 
     const authAuthenticate = Effect.fn("McpHttpApi.authAuthenticate")(function* (ctx: { params: { name: string } }) {
-      if (!(yield* mcp.supportsOAuth(ctx.params.name))) return yield* new HttpApiError.BadRequest({})
+      if (!(yield* mcp.supportsOAuth(ctx.params.name))) {
+        return yield* new UnsupportedOAuthError({ error: `MCP server ${ctx.params.name} does not support OAuth` })
+      }
       return yield* mcp.authenticate(ctx.params.name)
     })
 

+ 19 - 2
packages/opencode/src/server/routes/instance/mcp.ts

@@ -8,6 +8,21 @@ import { lazy } from "@/util/lazy"
 import { Effect } from "effect"
 import { jsonRequest, runRequest } from "./trace"
 
+const UnsupportedOAuthError = z
+  .object({
+    error: z.string(),
+  })
+  .meta({ ref: "McpUnsupportedOAuthError" })
+
+const unsupportedOAuthErrorResponse = {
+  description: "MCP server does not support OAuth",
+  content: {
+    "application/json": {
+      schema: resolver(UnsupportedOAuthError),
+    },
+  },
+}
+
 export const McpRoutes = lazy(() =>
   new Hono()
     .get(
@@ -85,7 +100,8 @@ export const McpRoutes = lazy(() =>
               },
             },
           },
-          ...errors(400, 404),
+          400: unsupportedOAuthErrorResponse,
+          ...errors(404),
         },
       }),
       async (c) => {
@@ -157,7 +173,8 @@ export const McpRoutes = lazy(() =>
               },
             },
           },
-          ...errors(400, 404),
+          400: unsupportedOAuthErrorResponse,
+          ...errors(404),
         },
       }),
       async (c) => {

+ 83 - 2
packages/opencode/test/server/httpapi-mcp.test.ts

@@ -1,15 +1,28 @@
 import { afterEach, describe, expect, test } from "bun:test"
-import { Context } from "effect"
+import type { UpgradeWebSocket } from "hono/ws"
+import { Context, Effect, FileSystem, Layer, Path } from "effect"
+import { NodeFileSystem, NodePath } from "@effect/platform-node"
+import { Flag } from "@opencode-ai/core/flag/flag"
 import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server"
 import { McpPaths } from "../../src/server/routes/instance/httpapi/mcp"
 import { Instance } from "../../src/project/instance"
+import { InstanceRoutes } from "../../src/server/routes/instance"
 import * as Log from "@opencode-ai/core/util/log"
 import { resetDatabase } from "../fixture/db"
-import { tmpdir } from "../fixture/fixture"
+import { provideInstance, tmpdir } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
 
 void Log.init({ print: false })
 
+const original = Flag.OPENCODE_EXPERIMENTAL_HTTPAPI
 const context = Context.empty() as Context.Context<unknown>
+const websocket = (() => () => new Response(null, { status: 501 })) as unknown as UpgradeWebSocket
+const it = testEffect(Layer.mergeAll(NodeFileSystem.layer, NodePath.layer))
+
+function app(experimental: boolean) {
+  Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = experimental
+  return InstanceRoutes(websocket)
+}
 
 function request(route: string, directory: string, init?: RequestInit) {
   const headers = new Headers(init?.headers)
@@ -23,7 +36,51 @@ function request(route: string, directory: string, init?: RequestInit) {
   )
 }
 
+function withMcpProject<A, E, R>(self: (dir: string) => Effect.Effect<A, E, R>) {
+  return Effect.gen(function* () {
+    const fs = yield* FileSystem.FileSystem
+    const path = yield* Path.Path
+    const dir = yield* fs.makeTempDirectoryScoped({ prefix: "opencode-test-" })
+
+    yield* fs.writeFileString(
+      path.join(dir, "opencode.json"),
+      JSON.stringify({
+        $schema: "https://opencode.ai/config.json",
+        formatter: false,
+        lsp: false,
+        mcp: {
+          demo: {
+            type: "local",
+            command: ["echo", "demo"],
+            enabled: false,
+          },
+        },
+      }),
+    )
+    yield* Effect.addFinalizer(() =>
+      Effect.promise(() => Instance.provide({ directory: dir, fn: () => Instance.dispose() })).pipe(Effect.ignore),
+    )
+
+    return yield* self(dir).pipe(provideInstance(dir))
+  })
+}
+
+const readResponse = Effect.fnUntraced(function* (input: {
+  app: ReturnType<typeof InstanceRoutes>
+  path: string
+  headers: HeadersInit
+}) {
+  const response = yield* Effect.promise(() =>
+    Promise.resolve(input.app.request(input.path, { method: "POST", headers: input.headers })),
+  )
+  return {
+    status: response.status,
+    body: yield* Effect.promise(() => response.text()),
+  }
+})
+
 afterEach(async () => {
+  Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = original
   await Instance.disposeAll()
   await resetDatabase()
 })
@@ -107,4 +164,28 @@ describe("mcp HttpApi", () => {
     expect(removed.status).toBe(200)
     expect(await removed.json()).toEqual({ success: true })
   })
+
+  it.live(
+    "matches legacy unsupported OAuth error responses",
+    withMcpProject((dir) =>
+      Effect.gen(function* () {
+        const headers = { "x-opencode-directory": dir }
+        const legacy = app(false)
+        const httpapi = app(true)
+
+        yield* Effect.forEach(["/mcp/demo/auth", "/mcp/demo/auth/authenticate"], (path) =>
+          Effect.gen(function* () {
+            const legacyResponse = yield* readResponse({ app: legacy, path, headers })
+            const httpapiResponse = yield* readResponse({ app: httpapi, path, headers })
+
+            expect(legacyResponse).toEqual({
+              status: 400,
+              body: JSON.stringify({ error: "MCP server demo does not support OAuth" }),
+            })
+            expect(httpapiResponse).toEqual(legacyResponse)
+          }),
+        )
+      }),
+    ),
+  )
 })

+ 8 - 4
packages/sdk/js/src/v2/gen/types.gen.ts

@@ -2129,6 +2129,10 @@ export type McpStatus =
   | McpStatusNeedsAuth
   | McpStatusNeedsClientRegistration
 
+export type McpUnsupportedOAuthError = {
+  error: string
+}
+
 export type Path = {
   home: string
   state: string
@@ -4907,9 +4911,9 @@ export type McpAuthStartData = {
 
 export type McpAuthStartErrors = {
   /**
-   * Bad request
+   * MCP server does not support OAuth
    */
-  400: BadRequestError
+  400: McpUnsupportedOAuthError
   /**
    * Not found
    */
@@ -4985,9 +4989,9 @@ export type McpAuthAuthenticateData = {
 
 export type McpAuthAuthenticateErrors = {
   /**
-   * Bad request
+   * MCP server does not support OAuth
    */
-  400: BadRequestError
+  400: McpUnsupportedOAuthError
   /**
    * Not found
    */