Skip to content

Fix OAuth token refresh for servers with root-based discovery (fixes #3)#6

Closed
kingsleyzhong wants to merge 2 commits intoapify:mainfrom
kingsleyzhong:fix-oauth-token-refresh-discovery
Closed

Fix OAuth token refresh for servers with root-based discovery (fixes #3)#6
kingsleyzhong wants to merge 2 commits intoapify:mainfrom
kingsleyzhong:fix-oauth-token-refresh-discovery

Conversation

@kingsleyzhong
Copy link
Copy Markdown

Problem

The discoverTokenEndpoint() function in oauth-utils.ts only tries path-based OAuth discovery URLs:

  • https://example.com/mcp/.well-known/oauth-authorization-server
  • https://example.com/mcp/.well-known/openid-configuration

However, some servers like Notion host their OAuth metadata at the root:

  • https://mcp.notion.com/.well-known/oauth-authorization-server

This causes token refresh to fail for these servers, even though initial login works (because the MCP SDK's discoverAuthorizationServerMetadata() tries both path-based and root-based discovery).

Solution

Add root-based fallback URLs to discoverTokenEndpoint(), matching the behavior of the MCP SDK:

const serverUrlObj = new URL(serverUrl);
const base = \`\${serverUrlObj.protocol}//\${serverUrlObj.host}\`;
if (serverUrl !== base && serverUrl !== \`\${base}/\`) {
  discoveryUrls.push(
    \`\${base}/.well-known/oauth-authorization-server\`,
    \`\${base}/.well-known/openid-configuration\`,
  );
}

Testing

  • All unit tests pass (335/335)
  • Manual verification: discoverTokenEndpoint('https://mcp.notion.com/mcp') now correctly returns https://mcp.notion.com/token

Fixes #3

kingsleyzhong and others added 2 commits February 1, 2026 10:11
Fixes apify#3

The discoverTokenEndpoint() function only tried path-based OAuth discovery
URLs (e.g., https://mcp.notion.com/mcp/.well-known/oauth-authorization-server),
but some servers like Notion host their OAuth metadata at the root
(e.g., https://mcp.notion.com/.well-known/oauth-authorization-server).

Initial login worked because discoverAuthorizationServerMetadata() in the MCP
SDK tries both path-based and root-based discovery. However, token refresh
failed because discoverTokenEndpoint() only tried path-based URLs.

This fix adds root-based fallback URLs to discoverTokenEndpoint(), matching
the behavior of the MCP SDK's discovery function.
Will regenerate this later
@jancurn
Copy link
Copy Markdown
Member

jancurn commented Feb 23, 2026

Great catch, thanks for the fix - I was unable to revert the changes to package-lock.json, so I recreated the changes in c69008a

@jancurn jancurn closed this Feb 23, 2026
jancurn pushed a commit that referenced this pull request Apr 15, 2026
On Windows, `isProcessAlive()` uses a 2-second cached `tasklist` snapshot
populated when `stopBridge()` runs. When `restartSession` subsequently
spawns a new bridge, the new PID is not in the cache, causing
`ensureBridgeReady()` to misclassify the freshly spawned bridge as dead
and trigger a spurious double-restart via `restartBridge()`. Unlike the
explicit `restartSession` path, `restartBridge` passes `mcpSessionId` for
resumption, so the second bridge silently resumes the old MCP session
and `sessions.json` never records a new session ID.

This caused two E2E tests to fail on Windows:

  - sessions/mcp-session #6: mcpSessionId unchanged after explicit restart
  - sessions/restart #9: `.capabilities` missing from `restart --json`

Fix by resetting the tasklist cache immediately after spawning the bridge
so the next `isProcessAlive()` check runs a fresh `tasklist`. The only
`spawn()` call in src/ is in `startBridge`, so one invalidation site
covers every bridge startup path.

https://claude.ai/code/session_014w5QYdwZKHA5REd2pTEXwP
jancurn added a commit that referenced this pull request Apr 15, 2026
…167)

On Windows, `isProcessAlive()` uses a 2-second cached `tasklist` snapshot
populated when `stopBridge()` runs. When `restartSession` subsequently
spawns a new bridge, the new PID is not in the cache, causing
`ensureBridgeReady()` to misclassify the freshly spawned bridge as dead
and trigger a spurious double-restart via `restartBridge()`. Unlike the
explicit `restartSession` path, `restartBridge` passes `mcpSessionId` for
resumption, so the second bridge silently resumes the old MCP session
and `sessions.json` never records a new session ID.

This caused two E2E tests to fail on Windows:

  - sessions/mcp-session #6: mcpSessionId unchanged after explicit restart
  - sessions/restart #9: `.capabilities` missing from `restart --json`

Fix by resetting the tasklist cache immediately after spawning the bridge
so the next `isProcessAlive()` check runs a fresh `tasklist`. The only
`spawn()` call in src/ is in `startBridge`, so one invalidation site
covers every bridge startup path.

https://claude.ai/code/session_014w5QYdwZKHA5REd2pTEXwP

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth token refresh fails for MCP servers with non-root OAuth discovery endpoints (e.g., Notion)

3 participants