fix(mcp/oauth): preserve advertised path-scoped resource indicators

Review on PR #3503 caught that the broad same-origin filter broke valid
protected-resource discovery shapes such as
`https://gateway.example.com/my-service/mcp`: same host as the authorization
server, but a distinct MCP service identified by path. Those advertised
resources must be preserved for audience selection.

- Replaced the unconditional same-origin filter with a provenance-aware policy: exact auth-server-origin resources are always stripped, path-scoped same-origin resources are preserved by default, and only OMP-synthesized fallback resources opt into same-origin path stripping.
- Added `stripSameOriginResource` to `MCPOAuthConfig` and `RefreshMCPOAuthTokenOptions`; quick-add/reauth set it only when the resource came from `config.url` / `runtimeBaseConfig.url` fallback rather than `oauth.resource` or an existing auth resource.
- Refresh uses the same flag when `MCPManager.prepareConfig` falls back to `config.url`, and no longer persists fallback resources into the credential as if they were provider-advertised material.
- Updated RFC 8707 tests to cover both sides: gateway path resource preserved, Plane-style fallback `/http/mcp` stripped, refresh path mirrors the same distinction.

Fixes #3502

packages/coding-agent/CHANGELOG.md

@@ -13,7 +13,7 @@
 - Fixed ollama-cloud task/subagent fan-out exceeding the provider's three-request concurrency cap by adding a provider-specific subagent limiter, and let configured task/smol/advisor model roles inherit the default retry fallback chain when they do not define their own chain. ([#3464](https://github.com/can1357/oh-my-pi/issues/3464))
 - Fixed the per-provider subagent concurrency limiter (e.g. `providers.ollama-cloud.maxConcurrency`) being replaced with a fresh semaphore whenever the configured limit changed, which orphaned the in-flight slots on the old instance and let a runtime or mixed limit value exceed the cap. The limiter now resizes a single shared semaphore in place — raising the ceiling admits queued waiters immediately, lowering it drains in-flight holders without admitting past the new cap. ([#3464](https://github.com/can1357/oh-my-pi/issues/3464))
 - Fixed a background-task spawn slot leaking from the `task.maxConcurrency` limiter when progress reporting threw between acquiring the slot and entering the guarded run: `markRunning`/`reportProgress` now run inside the try whose `finally` releases the semaphore, so a failed progress report can no longer permanently shrink subagent concurrency. ([#3464](https://github.com/can1357/oh-my-pi/issues/3464))
-- Fixed MCP OAuth authorization failing with `Authorization failed: An unexpected error occurred` against authorization servers (Plane is the live example) that reject same-origin `resource` indicators, including path-bearing MCP endpoint URLs such as `https://mcp.plane.so/http/mcp`. Per RFC 8707 §2 the resource indicator distinguishes *other* resource servers from the authorization server, so OMP's `MCPOAuthFlow` now drops same-origin resource values across the authorize URL, the matching token-exchange request, **and** the refresh-token request — filtered against the authorization-server origin (persisted on the credential as `authorizationUrl`), with `tokenUrl`'s origin as the legacy fallback so the filter stays correct even when RFC 8414's optional authorize/token-endpoint origin split is in play. ([#3502](https://github.com/can1357/oh-my-pi/issues/3502))
+- Fixed MCP OAuth authorization failing with `Authorization failed: An unexpected error occurred` against authorization servers (Plane is the live example) that reject redundant `resource` indicators. OMP now drops exact authorization-server-origin resources for every MCP OAuth flow, and also drops same-origin path resources only when OMP synthesized them from the server URL fallback (e.g. `https://mcp.plane.so/http/mcp`). Provider-advertised path-scoped resources from protected-resource discovery are preserved so gateway-hosted MCP services can still request the audience they advertised. The refresh-token path uses the same policy, filtered against the authorization-server origin persisted on the credential as `authorizationUrl`, with `tokenUrl`'s origin as the legacy fallback when that field is absent. ([#3502](https://github.com/can1357/oh-my-pi/issues/3502))
 
 ## [16.1.19] - 2026-06-25
 

packages/coding-agent/src/mcp/manager.ts

@@ -1235,8 +1235,9 @@ export class MCPManager {
 				// token endpoints sit on different origins (issue #3502 review
 				// follow-up).
 				const authorizationUrl = material && "authorizationUrl" in material ? material.authorizationUrl : undefined;
-				const resource =
-					material?.resource ?? (config.type === "http" || config.type === "sse" ? config.url : undefined);
+				const resourceIsFallback =
+					!material?.resource && (config.type === "http" || config.type === "sse") && Boolean(config.url);
+				const resource = material?.resource ?? (resourceIsFallback ? config.url : undefined);
 				// Proactive refresh: 5-minute buffer before expiry
 				// Force refresh: on 401/403 auth errors (revoked tokens, clock skew, missing expires)
 				const REFRESH_BUFFER_MS = 5 * 60_000;
@@ -1250,7 +1251,7 @@ export class MCPManager {
 							clientId,
 							clientSecret,
 							resource,
-							{ authorizationUrl },
+							{ authorizationUrl, stripSameOriginResource: resourceIsFallback },
 						);
 						// Spread the old credential first so embedded refresh material survives rotation.
 						const refreshedCredential: MCPStoredOAuthCredential = {
@@ -1259,7 +1260,7 @@ export class MCPManager {
 							tokenUrl,
 							clientId,
 							clientSecret,
-							resource,
+							resource: resourceIsFallback ? undefined : resource,
 							authorizationUrl,
 						};
 						await this.#authStorage.set(credentialId, refreshedCredential);

packages/coding-agent/src/mcp/oauth-flow.ts

@@ -177,26 +177,38 @@ function resolveResourceUri(resource: string | undefined): string | undefined {
 	return trimmed;
 }
 
+interface ResourceIndicatorFilterOptions {
+	/** Strip any resource URL on the same origin as the authorization server. */
+	stripSameOriginResource?: boolean;
+}
+
 /**
- * Drop a resource indicator hosted on the same origin as {@link serverUrl}.
+ * Drop a redundant resource indicator relative to {@link serverUrl}.
  *
- * Some authorization servers (Plane is the live example, see issue #3502)
- * reject same-origin resource indicators with `server_error` before the
- * consent screen, including path-bearing values such as
- * `https://mcp.plane.so/http/mcp`. Per RFC 8707 §2 the indicator
- * distinguishes *other* resource servers from the authorization server, so
- * same-origin values are redundant for these MCP servers — silently strip
- * them to stay compatible with strict implementations.
+ * Exact auth-server-origin values are always redundant: they don't identify a
+ * distinct resource server. Path-bearing same-origin resources are different:
+ * protected-resource discovery can advertise them for gateway-hosted MCP
+ * services (`https://gateway.example.com/my-service/mcp`), and those values
+ * must be preserved because they identify the service audience by path.
  *
- * New credentials pass the original authorization URL so refresh filters
- * against the same issuer origin as the initial grant. Legacy credentials
- * fall back to the token URL's origin.
+ * Plane is stricter for OMP-synthesized fallback resources (e.g. using the
+ * configured server URL `https://mcp.plane.so/http/mcp` as `resource`), so
+ * fallback callers opt into `stripSameOriginResource` while provider-advertised
+ * `oauth.resource` values keep the path-preserving default.
  */
-function filterSameOriginResource(resource: string | undefined, serverUrl: string): string | undefined {
+function filterResourceIndicator(
+	resource: string | undefined,
+	serverUrl: string,
+	options: ResourceIndicatorFilterOptions = {},
+): string | undefined {
 	if (!resource) return undefined;
 	try {
 		const origin = new URL(serverUrl).origin;
-		if (new URL(resource).origin === origin) return undefined;
+		const parsedResource = new URL(resource);
+		if (parsedResource.origin !== origin) return resource;
+		if (options.stripSameOriginResource || (parsedResource.pathname === "/" && parsedResource.search === "")) {
+			return undefined;
+		}
 	} catch {
 		// Malformed serverUrl will fail elsewhere; fall through.
 	}
@@ -231,6 +243,13 @@ export interface MCPOAuthConfig {
 	callbackPath?: string;
 	/** MCP resource URI for RFC 8707 resource indicators */
 	resource?: string;
+	/**
+	 * True when `resource` was synthesized from the server URL fallback rather
+	 * than advertised by OAuth/protected-resource metadata. Fallback resources
+	 * are stripped when same-origin with the authorization server; advertised
+	 * path-scoped resources are preserved.
+	 */
+	stripSameOriginResource?: boolean;
 	/** Fetch implementation for token exchange and discovery requests. */
 	fetch?: FetchImpl;
 }
@@ -450,13 +469,14 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
 	}
 
 	/**
-	 * Drop resource indicators hosted on the authorization-server origin.
-	 * Delegates to {@link filterSameOriginResource}; the refresh-token leg uses
-	 * the same helper with the persisted authorization URL so initial grant and
-	 * refresh stay in lock-step (RFC 8707 §2.2 requires matching indicators).
+	 * Drop redundant resource indicators for this authorization server.
+	 * Provider-advertised path-scoped values are preserved; fallback server-URL
+	 * values opt into same-origin stripping via `stripSameOriginResource`.
 	 */
 	#filterResourceIndicator(resource: string | undefined): string | undefined {
-		return filterSameOriginResource(resource, this.config.authorizationUrl);
+		return filterResourceIndicator(resource, this.config.authorizationUrl, {
+			stripSameOriginResource: this.config.stripSameOriginResource,
+		});
 	}
 
 	/**
@@ -583,6 +603,12 @@ export interface RefreshMCPOAuthTokenOptions {
 	 * origin when omitted for legacy credentials.
 	 */
 	authorizationUrl?: string;
+	/**
+	 * True when the refresh `resource` was synthesized from the server URL
+	 * fallback because the credential/auth material carried no resource.
+	 * Preserved advertised resources leave this false/undefined.
+	 */
+	stripSameOriginResource?: boolean;
 }
 
 /**
@@ -610,9 +636,11 @@ export async function refreshMCPOAuthToken(
 		refresh_token: refreshToken,
 	});
 	if (clientId) params.set("client_id", clientId);
-	// Drop same-origin indicators so refresh stays consistent with the initial
-	// grant; see {@link filterSameOriginResource} for context.
-	const resolvedResource = filterSameOriginResource(resolveResourceUri(resource), filterAnchor);
+	// Drop redundant indicators so refresh stays consistent with the initial
+	// grant; see {@link filterResourceIndicator} for context.
+	const resolvedResource = filterResourceIndicator(resolveResourceUri(resource), filterAnchor, {
+		stripSameOriginResource: optsFromTrailing?.stripSameOriginResource,
+	});
 	if (resolvedResource) params.set("resource", resolvedResource);
 	if (clientSecret) params.set("client_secret", clientSecret);
 

packages/coding-agent/src/modes/controllers/mcp-command-controller.ts

@@ -496,6 +496,7 @@ export class MCPCommandController {
 
 						try {
 							const oauthResource = oauth.resource ?? finalConfig.url;
+							const oauthResourceIsFallback = !oauth.resource;
 							const oauthResult = await this.#handleOAuthFlow(
 								oauth.authorizationUrl,
 								oauth.tokenUrl,
@@ -509,11 +510,13 @@ export class MCPCommandController {
 									prompt: finalConfig.oauth?.prompt,
 									serverUrl: finalConfig.url,
 									resource: oauthResource,
+									stripSameOriginResource: oauthResourceIsFallback,
 								},
 							);
 							finalConfig = this.#persistOAuthResult(finalConfig, oauthResult, {
 								tokenUrl: oauth.tokenUrl,
 								resource: oauthResource,
+								stripSameOriginResource: oauthResourceIsFallback,
 								clientId: oauth.clientId,
 								userClientSecret: finalConfig.oauth?.clientSecret,
 							});
@@ -583,6 +586,7 @@ export class MCPCommandController {
 			prompt?: string;
 			serverUrl?: string;
 			resource?: string;
+			stripSameOriginResource?: boolean;
 		},
 	): Promise<OAuthFlowResult> {
 		const authStorage = this.ctx.session.modelRegistry.authStorage;
@@ -624,6 +628,7 @@ export class MCPCommandController {
 					callbackPort: opts?.callbackPort,
 					callbackPath: opts?.callbackPath,
 					resource: opts?.resource,
+					stripSameOriginResource: opts?.stripSameOriginResource,
 				},
 				{
 					onAuth: (info: { url: string; instructions?: string }) => {
@@ -752,10 +757,17 @@ export class MCPCommandController {
 	#persistOAuthResult(
 		config: MCPServerConfig,
 		result: OAuthFlowResult,
-		opts: { tokenUrl: string; resource?: string; clientId?: string; userClientSecret?: string },
+		opts: {
+			tokenUrl: string;
+			resource?: string;
+			stripSameOriginResource?: boolean;
+			clientId?: string;
+			userClientSecret?: string;
+		},
 	): MCPServerConfig {
 		const clientId = result.clientId ?? opts.clientId ?? config.oauth?.clientId;
-		const resource = result.resource ?? opts.resource ?? config.auth?.resource;
+		const resource =
+			result.resource ?? (opts.stripSameOriginResource ? undefined : opts.resource) ?? config.auth?.resource;
 		return {
 			...config,
 			auth: {
@@ -1559,6 +1571,7 @@ export class MCPCommandController {
 			const currentAuthResource = currentAuth?.resource ? expandEnvVarsDeep(currentAuth.resource) : undefined;
 			const oauthResource =
 				oauth.resource ?? currentAuthResource ?? ("url" in runtimeBaseConfig ? runtimeBaseConfig.url : undefined);
+			const oauthResourceIsFallback = !oauth.resource && !currentAuthResource;
 
 			const oauthResult = await this.#handleOAuthFlow(
 				oauth.authorizationUrl,
@@ -1573,6 +1586,7 @@ export class MCPCommandController {
 					prompt: found.config.oauth?.prompt,
 					serverUrl,
 					resource: oauthResource,
+					stripSameOriginResource: oauthResourceIsFallback,
 				},
 			);
 
@@ -1593,6 +1607,7 @@ export class MCPCommandController {
 					clientId: oauth.clientId,
 					userClientSecret,
 					resource: oauthResource,
+					stripSameOriginResource: oauthResourceIsFallback,
 				});
 				await updateMCPServer(found.filePath, name, updated);
 			}

packages/coding-agent/test/mcp-manager-oauth-refresh.test.ts

@@ -84,7 +84,7 @@ describe("MCPManager OAuth refresh failure", () => {
 			undefined,
 			undefined,
 			"https://logfire.example.com/mcp",
-			{ authorizationUrl: undefined },
+			{ authorizationUrl: undefined, stripSameOriginResource: true },
 		);
 		// The poisoned Bearer must not be re-injected — that is the loop the user
 		// reported (#1908).

packages/coding-agent/test/oauth-flow.test.ts

@@ -551,11 +551,10 @@ describe("mcp oauth flow", () => {
 		expect(tokenParams.get("resource")).toBeNull();
 	});
 	describe("RFC 8707 resource indicator", () => {
-		// return `server_error` when the resource indicator is same-origin,
-		// including path-bearing resources such as
-		// `https://mcp.plane.so/http/mcp`. Per RFC 8707 §2 the indicator
-		// distinguishes *other* resource servers, so same-origin values are
-		// redundant for these MCP servers.
+		// Exact auth-server-origin values are redundant, but path-scoped
+		// same-host values from protected-resource discovery identify a distinct
+		// MCP service and must be preserved. Plane's fallback-resource case opts
+		// into same-origin path stripping separately.
 
 		const REDIRECT_URI = "http://127.0.0.1:14580/callback";
 
@@ -563,6 +562,7 @@ describe("mcp oauth flow", () => {
 			authorizationUrl: string;
 			resource?: string;
 			onTokenBody?: (body: string) => void;
+			stripSameOriginResource?: boolean;
 		}): Promise<MCPOAuthFlow> {
 			return new MCPOAuthFlow(
 				{
@@ -570,6 +570,7 @@ describe("mcp oauth flow", () => {
 					tokenUrl: "https://provider.example/token",
 					clientId: "client-id",
 					resource: config.resource,
+					stripSameOriginResource: config.stripSameOriginResource,
 					callbackPort: 14580,
 					fetch: mockProviderTokenEndpoint(body => config.onTokenBody?.(body)),
 				},
@@ -601,7 +602,7 @@ describe("mcp oauth flow", () => {
 			expect(flow.resource).toBeUndefined();
 		});
 
-		it("strips a self-referential resource that was pre-populated on the authorization URL", async () => {
+		it("strips an origin-only resource that was pre-populated on the authorization URL", async () => {
 			const flow = await buildFlow({
 				authorizationUrl: "https://mcp.plane.so/authorize?resource=https%3A%2F%2Fmcp.plane.so",
 			});
@@ -632,11 +633,31 @@ describe("mcp oauth flow", () => {
 			expect(tokenParams.get("resource")).toBeNull();
 		});
 
-		it("strips the resource when it points at a path under the auth-server origin", async () => {
+		it("keeps a discovered path-scoped resource under the auth-server origin", async () => {
+			let tokenRequestBody = "";
+			const flow = await buildFlow({
+				authorizationUrl: "https://gateway.example.com/authorize",
+				resource: "https://gateway.example.com/my-service/mcp",
+				onTokenBody: body => {
+					tokenRequestBody = body;
+				},
+			});
+
+			const { url } = await flow.generateAuthUrl("state-x", REDIRECT_URI);
+			await flow.exchangeToken("test-code", "state-x", REDIRECT_URI);
+			const tokenParams = new URLSearchParams(tokenRequestBody);
+
+			expect(new URL(url).searchParams.get("resource")).toBe("https://gateway.example.com/my-service/mcp");
+			expect(flow.resource).toBe("https://gateway.example.com/my-service/mcp");
+			expect(tokenParams.get("resource")).toBe("https://gateway.example.com/my-service/mcp");
+		});
+
+		it("strips a fallback server URL resource when it points at a path under the auth-server origin", async () => {
 			let tokenRequestBody = "";
 			const flow = await buildFlow({
 				authorizationUrl: "https://mcp.plane.so/authorize",
 				resource: "https://mcp.plane.so/http/mcp",
+				stripSameOriginResource: true,
 				onTokenBody: body => {
 					tokenRequestBody = body;
 				},
@@ -666,13 +687,12 @@ describe("mcp oauth flow", () => {
 
 	describe("RFC 8707 resource indicator (refresh)", () => {
 		// Regression for the review on PR #3503: the initial grant stores
-		// `resource: undefined` for same-origin Plane resources, but
-		// `MCPManager.prepareConfig` (manager.ts:1232-1233) falls back to
-		// `config.url` when the stored material has no resource, which would
-		// re-introduce the same value at refresh time. `refreshMCPOAuthToken`
-		// must apply the same-origin filter against the original
-		// authorization-server origin (falling back to `tokenUrl` for legacy
-		// credentials) so initial grant and refresh stay in lock-step.
+		// `resource: undefined` for fallback same-origin Plane resources, but
+		// `MCPManager.prepareConfig` falls back to `config.url` when the stored
+		// material has no resource. `refreshMCPOAuthToken` must apply the
+		// fallback same-origin filter against the original authorization-server
+		// origin (falling back to `tokenUrl` for legacy credentials) while
+		// preserving advertised path-scoped resources.
 
 		function mockArbitraryTokenEndpoint(targetUrl: string, onBody: (body: string) => void): FetchImpl {
 			return async (input, init) => {
@@ -732,7 +752,27 @@ describe("mcp oauth flow", () => {
 			expect(tokenParams.get("resource")).toBeNull();
 		});
 
-		it("strips a refresh resource that points at a different path under the token-server origin", async () => {
+		it("keeps an advertised refresh resource that points at a path under the token-server origin", async () => {
+			let tokenRequestBody = "";
+
+			await refreshMCPOAuthToken(
+				"https://gateway.example.com/token",
+				"refresh-token",
+				"client-id",
+				undefined,
+				"https://gateway.example.com/my-service/mcp",
+				{
+					fetch: mockArbitraryTokenEndpoint("https://gateway.example.com/token", body => {
+						tokenRequestBody = body;
+					}),
+				},
+			);
+			const tokenParams = new URLSearchParams(tokenRequestBody);
+
+			expect(tokenParams.get("resource")).toBe("https://gateway.example.com/my-service/mcp");
+		});
+
+		it("strips a fallback refresh resource that points at a path under the token-server origin", async () => {
 			let tokenRequestBody = "";
 
 			await refreshMCPOAuthToken(
@@ -742,6 +782,7 @@ describe("mcp oauth flow", () => {
 				undefined,
 				"https://mcp.plane.so/http/mcp",
 				{
+					stripSameOriginResource: true,
 					fetch: mockArbitraryTokenEndpoint("https://mcp.plane.so/token", body => {
 						tokenRequestBody = body;
 					}),
@@ -797,11 +838,10 @@ describe("mcp oauth flow", () => {
 			expect(tokenParams.get("resource")).toBe("https://api.example.com");
 		});
 
-		it("falls back to tokenUrl-anchored filtering for legacy credentials without authorizationUrl", async () => {
-			// Documents the legacy path: credentials minted before this fix
-			// don't carry `authorizationUrl`, so refresh still filters against
-			// `tokenUrl`'s origin — preserves the same-origin behavior of
-			// commit 1.
+		it("falls back to tokenUrl-anchored exact-origin filtering for legacy credentials without authorizationUrl", async () => {
+			// Documents the legacy path: credentials minted before this fix don't
+			// carry `authorizationUrl`, so refresh still strips exact origin-only
+			// resources against `tokenUrl`.
 			let tokenRequestBody = "";
 
 			await refreshMCPOAuthToken(