fix(agent-server): use MCP OAuth credentials passed inline on the agent - #5078
Merged
Merged
Conversation
MCPSettingsOAuthTokenStore resolves FastMCP's token keys against this server's own settings store, matching on the MCP server URL. A deployment that passes the user's mcp_config inline on the agent, rather than persisting it into the sandbox's settings first, therefore gets nothing back: an OAuth server's auth.state is ignored, FastMCP finds no tokens and falls into an interactive authorization flow that cannot complete inside a sandbox, and the server ends up contributing no tools. Tokens refreshed during the conversation hit the same lookup and are discarded with "Could not persist MCP OAuth state: no configured MCP server matches". Seed the store from the config it is built for. Servers found in settings keep taking precedence, so a local agent-server behaves exactly as before. For servers absent from settings the seeded state is served instead, and puts and deletes update it in memory for the lifetime of the store rather than being dropped. SettingsBackedMCPToolProvider gains a cipher and passes the agent's mcp_config as the seed, so state that arrives encrypted at rest is read back with the server's own key.
Collaborator
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
6 tasks
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
Coverage Report •
|
||||||||||||||||||||
Collaborator
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
I verified the changes.
AGENT:
Why
MCPSettingsOAuthTokenStoreresolves FastMCP's token-storage keys against the agent server's own settings store, matching a stored server by MCP URL (_find_matching_oauth_server). That is the right source when the agent server owns the settings, which is the case when you run one locally.It is the only source, though, and a hosted deployment does not work that way: the control plane holds the user's settings and passes the resulting
mcp_configinline on the agent when it starts a conversation, without writing anything into the sandbox's settings store. Every lookup then misses, and an OAuth server'sauth.stateis ignored even though the agent is holding valid tokens. FastMCP sees an empty store, starts an interactive authorization it cannot finish inside a sandbox, and the server contributes no tools. The write side misses the same way, so a token refreshed mid-conversation is discarded with:Servers using a static credential are unaffected, because a bearer or header credential never goes through the token store.
Summary
MCPSettingsOAuthTokenStoretakes an optionalseed_mcp_config(plus thecipherneeded to read state that arrives encrypted at rest) and builds an in-memory overlay from each seeded OAuth server'sinitial_oauth_state. Settings still win whenever they hold the server, so a local agent server behaves exactly as before; the overlay only answers for servers settings do not know about.SettingsBackedMCPToolProvidercarries the cipher and seeds the store with themcp_configit is creating tools for, which is the config the agent was started with.Issue Number
None. Found while making OAuth MCP installs work against a hosted control plane, which is what produces the inline-config shape described above.
How to Test
Three tests cover the behaviour, each against a real settings store on a temp dir:
SettingsBackedMCPToolProviderhands FastMCP a store that already knows the agent's tokens.Reproducing the original failure needs no new code: construct
MCPSettingsOAuthTokenStore()with an empty settings store and ask for any key, and it returnsNoneonmainwhile returning the seeded token here.End-to-end confirmation that a hosted conversation reaches an OAuth MCP server with these credentials needs an agent server release and a sandbox image carrying it, so it is not something I can show from this repo. That run is pending and I would not treat this as verified until it is done; the change is a no-op for anyone whose settings store already holds the server.
Video/Screenshots
None: the change has no UI and the failure is a log line plus absent tools, quoted under "Why".
Design Doc
Not included. The change adds one optional constructor argument and a fallback branch in three methods of a single class; the "Why" and "Summary" above cover the design.
Type
Notes
seed_mcp_configandcipherboth default toNone,MCPSettingsOAuthTokenStore()keeps working, and settings-backed lookups are unchanged and still take precedence.check-forbidden-dynamic-attributeshook fails on this machine against a stale, gitignoredopenhands-sdk/build/tree from a previous local build. It scans that directory regardless of gitignore and flags no file in this change, so I skipped that one hook for the commit and ran every other hook normally.🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimnikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:6b31a33-pythonRun
All tags pushed for this build
About Multi-Architecture Support
6b31a33-python) is a multi-arch manifest supporting both amd64 and arm646b31a33-python-amd64) are also available if neededJev-Fast-Audit
⚡ Jev fast audit · estimates · 0.61s · commit 6b31a33
Strongest signal: No primary concern selected.
Evidence: No primary concern to locate.
Coverage: complete supplied coverage; 10/10 hunks, 2/2 files.
All estimates and evidence