[fix]: refresh stale per-user OAuth MCP tool catalogs

[zachgersh]

Summary

Refresh the global cached tool catalog for per-user OAuth MCP clients after their setup-time discovery becomes stale. Previously these clients had no persistent connection, so the periodic tool syncer skipped them and their catalogs remained unchanged until the client was recreated.

Changes

• Start an asynchronous global tools/list refresh after resolving a valid per-user OAuth token.
• Coalesce concurrent refreshes and throttle failed attempts using the configured tool sync interval.
• Apply refresh results through immutable client config snapshots and reject results that raced with client updates or removal.
• Persist only the discovered tools, name mapping, and last-sync timestamp through a conditional config-store update.
• Add the discovered_tools_last_sync migration and restore the timestamp across restarts.
• Document that the catalog is global while tool execution remains scoped to each caller OAuth token.
• Add focused race and config-store regression tests.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

Also affects framework/configstore persistence and migrations.

How to test

cd core
go test -race -cover ./mcp

cd ../framework
go test -race -cover ./configstore

cd ../transports
go test -race -run '^TestMergeMCPConfig_HashReconciliationUpdatesAndCreates$' ./bifrost-http/lib
go test -race -run '^$' ./bifrost-http/server

Expected: all commands pass. The UI is not changed by this PR.

Screenshots/Recordings

Not applicable; there are no UI changes.

Breaking changes

• Yes
• No

Related issues

None.

Security considerations

The resolved bearer token is used only by the temporary upstream MCP connection and is not persisted or exposed to plugins. Refreshed tool definitions are global by design, while every tool execution continues to use the calling user OAuth token.

Checklist

• I read the contribution guidelines and followed them
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified affected Go builds succeed
• I verified the full CI pipeline passes locally if applicable

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved per-user OAuth MCP tool-catalog refreshes with background synchronization, throttling, retry handling, and stale-result protection.
  • Refreshed tool catalogs now persist across restarts without interrupting tool execution.
  • Client updates preserve discovered tools and safely handle renamed clients.

• Documentation

  • Added documentation describing global tool-catalog refresh behavior for per-user OAuth connections.

• Chores

  • Added storage support for tracking catalog synchronization times.

Walkthrough

The PR adds coordinated per-user OAuth MCP tool-catalog refreshes, stale-result protection, immutable client configuration updates, persistence of discovered tools and sync timestamps, database migration support, bootstrap wiring, tests, and documentation.

Changes

Per-user OAuth MCP tool catalog synchronization

Layer / File(s)	Summary
Catalog persistence contract and storage core/schemas/mcp.go, framework/configstore/..., transports/bifrost-http/...	The MCP configuration exposes a persistence callback. The config store persists discovered tools, name mappings, and DiscoveredToolsLastSync. Updates require the expected client name and preserve unrelated fields.
OAuth refresh coordination core/mcp/..., docs/mcp/auth/per-user-oauth.mdx	Per-user OAuth calls capture bearer tokens and start coalesced, timeout-bounded refreshes. Successful results update and optionally persist the shared catalog. Stale or failed refreshes do not replace current tools.
Immutable client state updates core/mcp/clientmanager.go, core/mcp/clientmanager_test.go	Disable, enable, and update operations copy configuration snapshots. Client updates preserve discovered-tool metadata and synchronize renamed tool keys.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• maximhq/bifrost#5717: Provides the retained admin-credential discovery flow used by this synchronization behavior.
• maximhq/bifrost#5727: Also changes MCP tool synchronization and stale-result handling.
• maximhq/bifrost#5255: Changes validation and overflow handling for ToolSyncInterval.

Suggested reviewers: pratham-mishra04, akshaydeo, bearts

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant MCPManager
  participant MCPServer
  participant ConfigStore
  Caller->>MCPManager: Authorized MCP tool call
  MCPManager->>MCPManager: Coalesce and throttle refresh
  MCPManager->>MCPServer: Request tools/list with OAuth token
  MCPServer-->>MCPManager: Return discovered tools
  MCPManager->>ConfigStore: Persist tools, mappings, and sync time

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the primary fix: refreshing stale per-user OAuth MCP tool catalogs.
Description check	✅ Passed	The description covers the purpose, changes, testing, affected areas, security, breaking changes, documentation, and checklist items.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

core/mcp/clientmanager_test.go (1)

40-82: 🚀 Performance & Scalability | 🔵 Trivial

Consider adding coverage for the empty-tools refresh scenario.

maybeRefreshDiscoveredToolsAsync (the orchestration function that calls VerifyPerUserOAuthConnection then applyDiscoveredToolsRefresh) is not directly unit-tested here — only its constituent pieces (toolSyncFlight, applyDiscoveredToolsRefresh) are. Once the empty-tools guard proposed in core/mcp/clientmanager.go is added, add a test that exercises a successful-but-empty tools result and asserts the previous non-empty catalog is retained.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/mcp/clientmanager_test.go` around lines 40 - 82, Add a unit test for
maybeRefreshDiscoveredToolsAsync covering a successful refresh that returns an
empty tools map, with VerifyPerUserOAuthConnection succeeding; assert
applyDiscoveredToolsRefresh is not allowed to replace the existing non-empty
catalog and the prior tools remain intact, while preserving the existing
stale-result and immutable-config coverage.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/mcp/clientmanager.go`:
- Around line 287-306: Update the async refresh flow around
VerifyPerUserOAuthConnection and applyDiscoveredToolsRefresh to reject empty or
nil tools results when the client already has a non-empty shared catalog.
Preserve the existing catalog, set retryOnNextRequest so the next request
retries, and return without applying the empty tools or mapping; continue
applying empty results only when no prior catalog exists.
- Around line 259-275: Protect all reads of state.ExecutionConfig in
maybeRefreshDiscoveredToolsAsync, including the reads used by the dispatched
refresh and existing AcquireClientConn logic, with the same atomic pointer
snapshot mechanism used when DisableClient, EnableClient, UpdateClient, and
applyDiscoveredToolsRefresh replace it. Ensure the goroutine operates on the
captured configuration snapshot rather than an unsynchronized pointer.

---

Nitpick comments:
In `@core/mcp/clientmanager_test.go`:
- Around line 40-82: Add a unit test for maybeRefreshDiscoveredToolsAsync
covering a successful refresh that returns an empty tools map, with
VerifyPerUserOAuthConnection succeeding; assert applyDiscoveredToolsRefresh is
not allowed to replace the existing non-empty catalog and the prior tools remain
intact, while preserving the existing stale-result and immutable-config
coverage.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 594893e1-bf64-4717-a775-5cf81768c554

📥 Commits

Reviewing files that changed from the base of the PR and between 29e49c9 and 3b87b6a.

📒 Files selected for processing (16)

• core/changelog.md
• core/mcp/clientmanager.go
• core/mcp/clientmanager_test.go
• core/mcp/mcp.go
• core/schemas/mcp.go
• docs/mcp/auth/per-user-oauth.mdx
• framework/changelog.md
• framework/configstore/migrations.go
• framework/configstore/rdb.go
• framework/configstore/rdb_test.go
• framework/configstore/store.go
• framework/configstore/tables/mcp.go
• transports/bifrost-http/lib/config.go
• transports/bifrost-http/lib/config_test.go
• transports/bifrost-http/server/server.go
• transports/changelog.md

The merge-base changed after approval.