diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9638e8edf..5b9a72063 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,13 +1,12 @@ # Changelog ## [Unreleased] -### Breaking Changes +### Breaking Changes - The `vim` edit mode option is no longer available; configurations using `edit.mode: vim` will be automatically mapped to `hashline` mode - Hashline payload semantics are now strictly inline-first: the first payload line is whatever follows the sigil on the op line itself, and subsequent lines append after it. A newline immediately after `↑`/`↓`/`:` is no longer a free separator — it produces a blank first payload line. Use `LINE↓content` for a one-line insert, `LINE↓firstline\nsecondline` for two lines; bare `LINE↓` / `LINE↑` / `LINE:` (no inline payload) still insert/replace with one blank line as before. ### Added - - Added `irc.timeoutMs` setting to configure IRC message timeout duration with a default of 120 seconds - Added timeout enforcement for IRC send operations to prevent indefinite hangs when recipients are unresponsive - Added evaluator state inheritance for `task`-spawned subagents so JavaScript and Python variables are visible between a parent agent and its child sessions @@ -20,9 +19,11 @@ - Added file-read snapshot caching with multi-snapshot ring per path for recovery from agent's own writes - Added delete operation (`!`) support to hashline grammar for explicit line deletion - Added structural bracket/brace balance warnings when deleting lines with unclosed constructs +- Added resource metadata URL (RFC 9728) support to OAuth discovery for chaining authorization server resolution from protected-resource metadata ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) +- Added path-prefixed well-known URL fallback in OAuth discovery to support authorization servers behind gateways with sub-path routing ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) +- Added relative URL resolution for `Mcp-Auth-Server` header values against the server URL ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) ### Changed - - Changed Python tool bridge to use per-run identifiers alongside session IDs for correct routing of tool responses and output in concurrent evaluations - Changed JavaScript and Python `eval` execution to allow overlapping asynchronous cells on the same session ID to run concurrently instead of being strictly queued - Updated the edit mode option set to support `replace`, `patch`, `hashline`, and `apply_patch` variants @@ -44,9 +45,14 @@ - Modified hashline grammar to accept optional file hash in headers and removed hash requirements from line anchors - Changed hashline diff preview format to use `LINE:content` instead of `LINE+HASH|content` - Updated prompt documentation to reflect new `¶PATH#HASH` header and bare line-number syntax +- Updated `discoverOAuthEndpoints` to accept `resourceMetadataUrl` parameter and prioritize the resource-metadata chain ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) +- Updated `parseMcpAuthServerUrl` and `extractMcpAuthServerUrl` to accept optional `serverUrl` for relative URL resolution ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) +- Updated `MCPOAuthFlow.#resolveRegistrationEndpoint` to try origin-root well-known first, then fall back to path-prefixed well-known ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) + +### Fixed +- Fixed missing `await` on `#tryWellKnownForRegistration` call in `#resolveRegistrationEndpoint` that caused path-prefixed well-known fallback to never actually execute, returning the unresolved Promise object instead of the registration endpoint ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) ### Removed - - Removed the `installH2Fetch()` activation from CLI startup; HTTPS fetches now use Bun's default transport - Removed the `vim` edit mode along with the `VimTool` module, prompt, and supporting buffer/engine/renderer stack - Removed per-line hash anchors (2-letter bigram hashes) from hashline format @@ -57,11 +63,9 @@ - Removed `HashMismatch` type and hash mismatch error reporting; replaced with file-level validation ### Fixed - - Fixed JavaScript `eval` imports to preserve module-level singletons across re-imports of unchanged local files and reload them only after edits - Fixed concurrent Python evaluator tool calls to use per-run identifiers so tool responses and output are routed to the correct execution - Fixed the `search` tool argument validation to accept a single string `paths` value as a one-path search. - ## [15.4.0] - 2026-05-26 ### Breaking Changes diff --git a/packages/coding-agent/src/mcp/oauth-discovery.ts b/packages/coding-agent/src/mcp/oauth-discovery.ts index 65e330c06..1224d1abd 100644 --- a/packages/coding-agent/src/mcp/oauth-discovery.ts +++ b/packages/coding-agent/src/mcp/oauth-discovery.ts @@ -17,22 +17,23 @@ export interface AuthDetectionResult { authType?: "oauth" | "apikey" | "unknown"; oauth?: OAuthEndpoints; authServerUrl?: string; + resourceMetadataUrl?: string; message?: string; } -function parseMcpAuthServerUrl(errorMessage: string): string | undefined { +function parseMcpAuthServerUrl(errorMessage: string, serverUrl?: string): string | undefined { const match = errorMessage.match(/Mcp-Auth-Server:\s*([^;\]\s]+)/i); if (!match?.[1]) return undefined; try { - return new URL(match[1]).toString(); + return new URL(match[1], serverUrl).toString(); } catch { return undefined; } } -export function extractMcpAuthServerUrl(error: Error): string | undefined { - return parseMcpAuthServerUrl(error.message); +export function extractMcpAuthServerUrl(error: Error, serverUrl?: string): string | undefined { + return parseMcpAuthServerUrl(error.message, serverUrl); } /** @@ -189,12 +190,15 @@ export function extractOAuthEndpoints(error: Error): OAuthEndpoints | null { * Analyze an error to determine authentication requirements. * Returns structured info about what auth is needed. */ -export function analyzeAuthError(error: Error): AuthDetectionResult { +export function analyzeAuthError(error: Error, serverUrl?: string): AuthDetectionResult { if (!detectAuthError(error)) { return { requiresAuth: false }; } - const authServerUrl = extractMcpAuthServerUrl(error); + const authServerUrl = extractMcpAuthServerUrl(error, serverUrl); + // Extract resource_metadata URL from challenge entries in error message + const resourceMetaMatch = error.message.match(/resource_metadata\s*=\s*"([^"]+)"/i); + const resourceMetadataUrl = resourceMetaMatch?.[1]; // Try to extract OAuth endpoints const oauth = extractOAuthEndpoints(error); @@ -205,6 +209,7 @@ export function analyzeAuthError(error: Error): AuthDetectionResult { authType: "oauth", oauth, authServerUrl, + resourceMetadataUrl, message: "Server requires OAuth authentication. Launching authorization flow...", }; } @@ -221,6 +226,7 @@ export function analyzeAuthError(error: Error): AuthDetectionResult { requiresAuth: true, authType: "apikey", authServerUrl, + resourceMetadataUrl, message: "Server requires API key authentication.", }; } @@ -230,6 +236,7 @@ export function analyzeAuthError(error: Error): AuthDetectionResult { requiresAuth: true, authType: "unknown", authServerUrl, + resourceMetadataUrl, message: "Server requires authentication but type could not be determined.", }; } @@ -241,6 +248,7 @@ export function analyzeAuthError(error: Error): AuthDetectionResult { export async function discoverOAuthEndpoints( serverUrl: string, authServerUrl?: string, + resourceMetadataUrl?: string, ): Promise { const wellKnownPaths = [ "/.well-known/oauth-authorization-server", @@ -250,9 +258,44 @@ export async function discoverOAuthEndpoints( "/.mcp/auth", "/authorize", // Some MCP servers expose OAuth config here ]; - const urlsToQuery = [authServerUrl, serverUrl].filter((value): value is string => Boolean(value)); + const urlsToQuery: string[] = []; const visitedAuthServers = new Set(); + // Step 1: If a resource_metadata URL was provided, fetch it to discover auth servers. + // This follows the RFC 9728 chain: resource_metadata → authorization_servers. + if (resourceMetadataUrl && !visitedAuthServers.has(resourceMetadataUrl)) { + visitedAuthServers.add(resourceMetadataUrl); + try { + const metaResp = await fetch(resourceMetadataUrl, { + method: "GET", + headers: { Accept: "application/json" }, + redirect: "follow", + }); + if (metaResp.ok) { + const meta = (await metaResp.json()) as Record; + const authServers = Array.isArray(meta.authorization_servers) + ? meta.authorization_servers.filter((entry): entry is string => typeof entry === "string") + : []; + for (const s of authServers) { + if (!visitedAuthServers.has(s)) { + urlsToQuery.push(s); + visitedAuthServers.add(s); + } + } + } + } catch { + // Ignore errors, continue to try explicit URLs + } + } + + // Step 2: Add explicit authServerUrl and serverUrl (deduped against visited) + for (const url of [authServerUrl, serverUrl].filter((v): v is string => Boolean(v))) { + if (!visitedAuthServers.has(url)) { + urlsToQuery.push(url); + visitedAuthServers.add(url); + } + } + const findEndpoints = (metadata: Record): OAuthEndpoints | null => { if (metadata.authorization_endpoint && metadata.token_endpoint) { const scopesSupported = Array.isArray(metadata.scopes_supported) @@ -311,39 +354,61 @@ export async function discoverOAuthEndpoints( }; for (const baseUrl of urlsToQuery) { - visitedAuthServers.add(baseUrl); for (const path of wellKnownPaths) { - try { - const url = new URL(path, baseUrl); - const response = await fetch(url.toString(), { - method: "GET", - headers: { Accept: "application/json" }, - }); + // Try each well-known path at both the absolute origin and relative + const urlsToTry = buildWellKnownUrls(path, baseUrl); + for (const url of urlsToTry) { + try { + const response = await fetch(url.toString(), { + method: "GET", + headers: { Accept: "application/json" }, + redirect: "follow", + }); - if (response.ok) { - const metadata = (await response.json()) as Record; - const endpoints = findEndpoints(metadata); - if (endpoints) return endpoints; + if (response.ok) { + const metadata = (await response.json()) as Record; + const endpoints = findEndpoints(metadata); + if (endpoints) return endpoints; - if (path === "/.well-known/oauth-protected-resource") { - const authServers = Array.isArray(metadata.authorization_servers) - ? metadata.authorization_servers.filter((entry): entry is string => typeof entry === "string") - : []; + if (path === "/.well-known/oauth-protected-resource") { + const authServers = Array.isArray(metadata.authorization_servers) + ? metadata.authorization_servers.filter((entry): entry is string => typeof entry === "string") + : []; - for (const discoveredAuthServer of authServers) { - if (visitedAuthServers.has(discoveredAuthServer)) { - continue; + for (const discoveredAuthServer of authServers) { + if (visitedAuthServers.has(discoveredAuthServer)) { + continue; + } + const discovered = await discoverOAuthEndpoints(serverUrl, discoveredAuthServer); + if (discovered) return discovered; } - const discovered = await discoverOAuthEndpoints(serverUrl, discoveredAuthServer); - if (discovered) return discovered; } } + } catch { + // Ignore errors, try next path } - } catch { - // Ignore errors, try next path } } } return null; } + +function buildWellKnownUrls(wellKnownPath: string, baseUrl: string): URL[] { + let parsed: URL; + try { + parsed = new URL(baseUrl); + } catch { + return []; + } + + const absUrl = new URL(wellKnownPath, parsed); + if (!wellKnownPath.startsWith("/")) return [absUrl]; + + const normalizedPath = parsed.pathname.replace(/\/$/, ""); + const lastSlash = normalizedPath.lastIndexOf("/"); + if (lastSlash <= 0) return [absUrl]; + + const relUrl = new URL(wellKnownPath.slice(1), `${parsed.origin}${normalizedPath.slice(0, lastSlash)}/`); + return relUrl.href !== absUrl.href ? [absUrl, relUrl] : [absUrl]; +} diff --git a/packages/coding-agent/src/mcp/oauth-flow.ts b/packages/coding-agent/src/mcp/oauth-flow.ts index 71ae572d9..72d8c44bc 100644 --- a/packages/coding-agent/src/mcp/oauth-flow.ts +++ b/packages/coding-agent/src/mcp/oauth-flow.ts @@ -324,23 +324,39 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { } async #resolveRegistrationEndpoint(): Promise { + const authorizationUrl = new URL(this.config.authorizationUrl); + + // origin-root well-known; most servers serve metadata here. + const rootUrl = new URL("/.well-known/oauth-authorization-server", authorizationUrl.origin).toString(); + const endpoint = await this.#tryWellKnownForRegistration(rootUrl); + if (endpoint) return endpoint; + + // path-prefixed well-known for gateways (e.g. https://gateway.example.com/my-service/). + const normalizedPath = authorizationUrl.pathname.replace(/\/$/, ""); + const lastSlash = normalizedPath.lastIndexOf("/"); + if (lastSlash <= 0) return null; + + const prefixedUrl = new URL( + ".well-known/oauth-authorization-server", + `${authorizationUrl.origin}${normalizedPath.slice(0, lastSlash)}/`, + ).toString(); + return await this.#tryWellKnownForRegistration(prefixedUrl); + } + + async #tryWellKnownForRegistration(wellKnownUrl: string): Promise { try { - const authorizationEndpoint = new URL(this.config.authorizationUrl); - const metadataUrl = new URL("/.well-known/oauth-authorization-server", authorizationEndpoint.origin); - const response = await fetch(metadataUrl.toString(), { + const response = await fetch(wellKnownUrl, { method: "GET", headers: { Accept: "application/json" }, }); - if (!response.ok) return null; const metadata = (await response.json()) as { registration_endpoint?: string }; if (metadata.registration_endpoint && metadata.registration_endpoint.trim() !== "") { return metadata.registration_endpoint; } } catch { - // Ignore metadata discovery failures. + // Ignore fetch/parse failures. } - return null; } diff --git a/packages/coding-agent/src/modes/components/mcp-add-wizard.ts b/packages/coding-agent/src/modes/components/mcp-add-wizard.ts index 45420ad72..b7d4db071 100644 --- a/packages/coding-agent/src/modes/components/mcp-add-wizard.ts +++ b/packages/coding-agent/src/modes/components/mcp-add-wizard.ts @@ -965,14 +965,18 @@ export class MCPAddWizard extends Container { }, 1000); } catch (error) { // Connection failed - check if it's an auth error - const authResult = analyzeAuthError(error as Error); + const authResult = analyzeAuthError(error as Error, this.#state.url); if (authResult.requiresAuth) { // Prefer OAuth first: use error metadata, then well-known discovery fallback. let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; if (!oauth && this.#state.transport !== "stdio" && this.#state.url) { try { - oauth = await discoverOAuthEndpoints(this.#state.url, authResult.authServerUrl); + oauth = await discoverOAuthEndpoints( + this.#state.url, + authResult.authServerUrl, + authResult.resourceMetadataUrl, + ); } catch { // Ignore discovery failures and fallback to manual auth. } diff --git a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts index 717682096..8ad770813 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -401,12 +401,16 @@ export class MCPCommandController { ); return; } - const authResult = analyzeAuthError(error as Error); + const authResult = analyzeAuthError(error as Error, finalConfig.url); if (authResult.requiresAuth) { let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; if (!oauth && finalConfig.url) { try { - oauth = await discoverOAuthEndpoints(finalConfig.url, authResult.authServerUrl); + oauth = await discoverOAuthEndpoints( + finalConfig.url, + authResult.authServerUrl, + authResult.resourceMetadataUrl, + ); } catch { // Ignore discovery error and handle below. } @@ -742,11 +746,11 @@ export class MCPCommandController { } // Analyze the connection error to extract OAuth endpoints - const authResult = analyzeAuthError(connectionError!); + const authResult = analyzeAuthError(connectionError!, "url" in config ? config.url : undefined); let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; if (!oauth && (config.type === "http" || config.type === "sse") && config.url) { - oauth = await discoverOAuthEndpoints(config.url, authResult.authServerUrl); + oauth = await discoverOAuthEndpoints(config.url, authResult.authServerUrl, authResult.resourceMetadataUrl); } if (!oauth) { diff --git a/packages/coding-agent/test/oauth-discovery.test.ts b/packages/coding-agent/test/oauth-discovery.test.ts index 63d261b96..a9c7de08f 100644 --- a/packages/coding-agent/test/oauth-discovery.test.ts +++ b/packages/coding-agent/test/oauth-discovery.test.ts @@ -50,3 +50,146 @@ describe("mcp oauth discovery", () => { expect(calls[0]).toBe("https://www.figma.com/.well-known/oauth-authorization-server"); }); }); + +describe("path-prefixed auth servers", () => { + it("discovers endpoints via relative well-known path when server URL has a sub-path", async () => { + const calls: string[] = []; + using _hook = hookFetch(input => { + const url = String(input); + calls.push(url); + + // Absolute well-known fails (at origin root) + if (url === "https://gateway.example.com/.well-known/oauth-authorization-server") { + return new Response("not found", { status: 404 }); + } + // Relative well-known succeeds (under /my-service/) + if (url === "https://gateway.example.com/my-service/.well-known/oauth-authorization-server") { + return new Response( + JSON.stringify({ + authorization_endpoint: "https://gateway.example.com/my-service/oauth/authorize", + token_endpoint: "https://gateway.example.com/my-service/oauth/token", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + return new Response("not found", { status: 404 }); + }); + + const oauth = await discoverOAuthEndpoints("https://gateway.example.com/my-service/mcp"); + + expect(oauth).toEqual({ + authorizationUrl: "https://gateway.example.com/my-service/oauth/authorize", + tokenUrl: "https://gateway.example.com/my-service/oauth/token", + }); + // Absolute well-known was tried first (existing behavior) + expect(calls[0]).toBe("https://gateway.example.com/.well-known/oauth-authorization-server"); + // Relative well-known was tried as fallback + expect(calls).toContain("https://gateway.example.com/my-service/.well-known/oauth-authorization-server"); + }); + + it("prefers absolute well-known when it succeeds (origin-root servers still work)", async () => { + const calls: string[] = []; + using _hook = hookFetch(input => { + const url = String(input); + calls.push(url); + + if (url === "https://auth.example.com/.well-known/oauth-authorization-server") { + return new Response( + JSON.stringify({ + authorization_endpoint: "https://auth.example.com/oauth", + token_endpoint: "https://auth.example.com/token", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + return new Response("not found", { status: 404 }); + }); + + const oauth = await discoverOAuthEndpoints("https://mcp.example.com", "https://auth.example.com"); + + expect(oauth).toEqual({ + authorizationUrl: "https://auth.example.com/oauth", + tokenUrl: "https://auth.example.com/token", + }); + // Only the absolute path was needed + expect(calls).toHaveLength(1); + expect(calls[0]).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + }); +}); + +describe("resource_metadata chain", () => { + it("extracts resourceMetadataUrl from error message", () => { + const error = new Error( + 'HTTP 401: WWW-Authenticate: Bearer resource_metadata="https://gateway.example.com/my-service/.well-known/oauth-protected-resource"', + ); + + const auth = analyzeAuthError(error); + expect(auth.requiresAuth).toBe(true); + expect(auth.resourceMetadataUrl).toBe( + "https://gateway.example.com/my-service/.well-known/oauth-protected-resource", + ); + }); + + it("follows resource_metadata URL to discover authorization servers", async () => { + const calls: string[] = []; + using _hook = hookFetch(input => { + const url = String(input); + calls.push(url); + + // resource_metadata URL returns authorization_servers + if (url === "https://gateway.example.com/my-service/.well-known/oauth-protected-resource") { + return new Response( + JSON.stringify({ + authorization_servers: ["https://gateway.example.com/my-service"], + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + // Well-known at the discovered auth server (absolute fails, relative succeeds) + if (url === "https://gateway.example.com/.well-known/oauth-authorization-server") { + return new Response("not found", { status: 404 }); + } + if (url === "https://gateway.example.com/my-service/.well-known/oauth-authorization-server") { + return new Response( + JSON.stringify({ + authorization_endpoint: "https://gateway.example.com/my-service/oauth", + token_endpoint: "https://gateway.example.com/my-service/token", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + return new Response("not found", { status: 404 }); + }); + + const oauth = await discoverOAuthEndpoints( + "https://gateway.example.com/my-service/mcp", + undefined, + "https://gateway.example.com/my-service/.well-known/oauth-protected-resource", + ); + + expect(oauth).toEqual({ + authorizationUrl: "https://gateway.example.com/my-service/oauth", + tokenUrl: "https://gateway.example.com/my-service/token", + }); + // resource_metadata fetched first + expect(calls[0]).toBe("https://gateway.example.com/my-service/.well-known/oauth-protected-resource"); + }); +}); + +describe("relative Mcp-Auth-Server URL", () => { + it("resolves relative Mcp-Auth-Server against server URL", () => { + const error = new Error("HTTP 401: WWW-Authenticate: Bearer; Mcp-Auth-Server: /my-service/oauth"); + + // Without serverUrl, relative URL returns undefined + expect(extractMcpAuthServerUrl(error)).toBeUndefined(); + + // With serverUrl, relative URL is resolved + expect(extractMcpAuthServerUrl(error, "https://gateway.example.com/my-service/mcp")).toBe( + "https://gateway.example.com/my-service/oauth", + ); + }); +});