fix(mcp): pass OAuthClientProvider to call_mcp_tool for automatic token refresh

[Fizza-Mukhtar]

Summary

OAuth MCP tool calls fail with 401 Unauthorized after the access token
expires (~10 min). MCPTool.run() was passing only static
connection_headers (with the Bearer token) to call_mcp_tool(), but
never the OAuthClientProvider — so the MCP SDK had no way to refresh
the token automatically.

The admin tool discovery path (_list_mcp_tools_by_id) already constructs
an OAuthClientProvider via make_oauth_provider() and passes it through.
This fix brings tool execution into parity with tool discovery.

Root Cause

In MCPTool.run():

tool_result = call_mcp_tool(
    ...
    connection_headers=headers,  # static Bearer token, expires in ~10 min
    # auth= never passed — token refresh impossible
)

Fix

• MCPTool.__init__: add user_id parameter
• MCPTool.run(): construct OAuthClientProvider for OAUTH servers and
  pass auth= to call_mcp_tool(), enabling automatic token refresh via
  the MCP SDK
• tool_constructor.py: pass user_id=str(user.id) when constructing
  MCPTool

Testing

• Manually verified by tracing the OAuth path mirrors _list_mcp_tools_by_id
• No new tests added — OAuth integration requires a live MCP OAuth server

Related

Closes #9237

Additional Options

• [Optional] Please cherry-pick this PR to the latest release version.
• [Optional] Override Linear Check

---

Summary by cubic

Fixes OAuth MCP tool calls failing with 401s after token expiry by passing an OAuthClientProvider to call_mcp_tool, enabling automatic token refresh. Aligns tool execution with the discovery path and closes #9237.

• Bug Fixes
  • For OAUTH servers, pass auth (OAuthClientProvider) to call_mcp_tool for automatic token refresh; on SSE transport, log a warning and skip the provider.
  • Add user_id to MCPTool and wire it from tool_constructor.py.
  • Expand auth error detection to include "please reconnect to the server".

^{Written for commit 75b24ff. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

[greptile-apps]

Greptile Summary

This PR fixes OAuth MCP tool calls silently failing with 401 errors after the access token expires by passing an auth provider to call_mcp_tool, enabling the MCP SDK to automatically refresh tokens. The fix mirrors the existing pattern already used in the discovery path, bringing tool execution into parity.

Key changes:

• MCPTool gains a new constructor parameter to receive the requesting user's identifier
• MCPTool.run() constructs an auth provider for OAUTH servers on STREAMABLE_HTTP transport and passes it to call_mcp_tool
• For SSE transport combined with OAUTH, a warning is logged explaining that automatic refresh is unsupported due to an httpx/infinite-stream incompatibility
• "please reconnect to the server" is added to auth_error_indicators so that the ValueError raised when both the access token and refresh token are expired is surfaced as an actionable re-authentication message
• tool_constructor.py wires the authenticated user's identifier into MCPTool at construction time

Confidence Score: 4/5

• Safe to merge; the fix is a targeted, well-scoped change that mirrors an already-working code path.
• The implementation faithfully mirrors the _list_mcp_tools_by_id pattern that already works in production for OAuth token refresh during tool discovery. The STREAMABLE_HTTP / SSE transport distinction is correctly handled. The deferred imports (circular-import concern) and the layering violation are pre-existing issues flagged in prior review rounds. No regressions are expected for non-OAuth auth types since the auth parameter defaults to None and the existing headers-based path is unchanged.
• No files require special attention beyond what was already raised in prior review threads.

Important Files Changed

Filename	Overview
backend/onyx/tools/tool_implementations/mcp/mcp_tool.py	Adds user_id param and constructs OAuthClientProvider in MCPTool.run() for OAUTH servers on non-SSE transport, enabling automatic token refresh; correctly mirrors the discovery path pattern.
backend/onyx/tools/tool_constructor.py	One-line addition passing user_id=str(user.id) to MCPTool constructor; wires the new parameter from the authenticated User object already present in scope.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MCPTool.run called] --> B[Build connection headers]
    B --> C{auth_type == OAUTH?}
    C -- No --> G[call_mcp_tool without auth]
    C -- Yes --> D{transport == SSE?}
    D -- Yes --> E[Log warning: refresh unsupported]
    E --> G
    D -- No --> F[Construct OAuthClientProvider]
    F --> H[call_mcp_tool with auth provider]
    H --> I{Token valid?}
    I -- Yes --> J[Request sent with current token]
    I -- No --> K[SDK attempts token refresh]
    K -- Success --> L[Store new tokens, retry request]
    K -- Refresh expired --> M[redirect_handler raises ValueError]
    M --> N[Caught as auth error, prompt user to reconnect]
    G --> O[Return ToolResponse]
    J --> O
    L --> O
    N --> O

Loading

_{Last reviewed commit: 75b24ff}

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/onyx/tools/tool_implementations/mcp/mcp_tool.py">

<violation number="1" location="backend/onyx/tools/tool_implementations/mcp/mcp_tool.py:274">
P2: Generic `"reconnect"` substring matching can misclassify transport/runtime failures as authentication failures and return misleading credential-update guidance.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[oedokumaci]

Thanks for the fix! I can confirm the root cause analysis is correct.

One thing I noticed while working around this on my end: this PR may not fully solve the problem for OAuth providers whose token refresh responses omit the refresh_token field.

The MCP SDK's _handle_refresh_response() in oauth2.py stores the new token as-is:

self.context.current_tokens = token_response
await self.context.storage.set_tokens(token_response)

If a refresh response only contains access_token (no refresh_token), this overwrites the previously stored refresh_token. After the first successful refresh, can_refresh_token() returns False and subsequent refreshes fail.

A fix would be to preserve the old refresh_token when the new response doesn't include one:

if not token_response.refresh_token and self.context.current_tokens and self.context.current_tokens.refresh_token:
    token_response = OAuthToken(
        access_token=token_response.access_token,
        token_type=token_response.token_type,
        expires_in=token_response.expires_in,
        scope=token_response.scope,
        refresh_token=self.context.current_tokens.refresh_token,
    )

This might belong in the MCP SDK itself rather than in Onyx though.

[Fizza-Mukhtar]

Thanks for the fix! I can confirm the root cause analysis is correct.

One thing I noticed while working around this on my end: this PR may not fully solve the problem for OAuth providers whose token refresh responses omit the refresh_token field.

The MCP SDK's _handle_refresh_response() in oauth2.py stores the new token as-is:

self.context.current_tokens = token_response
await self.context.storage.set_tokens(token_response)

If a refresh response only contains access_token (no refresh_token), this overwrites the previously stored refresh_token. After the first successful refresh, can_refresh_token() returns False and subsequent refreshes fail.

A fix would be to preserve the old refresh_token when the new response doesn't include one:

if not token_response.refresh_token and self.context.current_tokens and self.context.current_tokens.refresh_token:
    token_response = OAuthToken(
        access_token=token_response.access_token,
        token_type=token_response.token_type,
        expires_in=token_response.expires_in,
        scope=token_response.scope,
        refresh_token=self.context.current_tokens.refresh_token,
    )

This might belong in the MCP SDK itself rather than in Onyx though.

Thanks for confirming and for the detailed follow-up! You're right — this edge case is separate from the fix here. I agree it likely belongs in the MCP SDK itself. Happy to follow up if the team thinks a workaround should live in Onyx too!

[oedokumaci]

I've filed the upstream fix for the refresh_token preservation issue in the MCP Python SDK:

• Issue: _handle_refresh_response discards existing refresh_token when server omits it modelcontextprotocol/python-sdk#2270
• PR: fix(oauth): preserve existing refresh_token when refresh response omits it modelcontextprotocol/python-sdk#2271

If that's merged and released, Onyx will need to bump its mcp package version to pick up the fix. Without it, this PR's refresh logic will work for the first token expiry but fail on subsequent ones for providers that omit refresh_token in refresh responses.

[evan-onyx]

LGTM, I'll make a mirror to run our CI

[Fizza-Mukhtar]

Hi @evan-onyx 👋
Could you please merge this when the CI checks pass? All review concerns have been addressed.

[evan-onyx]

Hi @evan-onyx 👋 Could you please merge this when the CI checks pass? All review concerns have been addressed.

some CI is failing #9414 that looks like it might be real issues (most accurately, a test or two that needs to be updated). @Fizza-Mukhtar if you're willing to make those fixes it would be fantastic, otherwise I'll get to it when I have time (hopefully later today, but might be a few days at worst)

[Fizza-Mukhtar]

Hi @evan-onyx 👋 Could you please merge this when the CI checks pass? All review concerns have been addressed.

some CI is failing #9414 that looks like it might be real issues (most accurately, a test or two that needs to be updated). @Fizza-Mukhtar if you're willing to make those fixes it would be fantastic, otherwise I'll get to it when I have time (hopefully later today, but might be a few days at worst)

Sure, I'll take a look at the failing tests and push a fix!

[evan-onyx]

Hi @evan-onyx 👋 Could you please merge this when the CI checks pass? All review concerns have been addressed.

some CI is failing #9414 that looks like it might be real issues (most accurately, a test or two that needs to be updated). @Fizza-Mukhtar if you're willing to make those fixes it would be fantastic, otherwise I'll get to it when I have time (hopefully later today, but might be a few days at worst)

Sure, I'll take a look at the failing tests and push a fix!

actually sorry I just pushed one 😅 it ended up being pretty simple

[evan-onyx]

@Fizza-Mukhtar I'll merge the mirror once it passes, which should be soon!

[evan-onyx]

The mirror PR #9414 has been merged!