fix(runtime): repopulate StateView per-server tools on reconnect (MCP-2094)#637
Merged
Conversation
…-2094) The connection-down handler clears the StateView per-server tool set (status.Tools = nil) on disconnect, but the reconnect handler deliberately left it empty, relying on background discovery to refill it. This left a transient/persistent window where StateView reports 0 tools for a connected server that has tools. Consumers that don't use the #635 read fallback (tray tool counts, SSE servers.changed counts, health/diagnostics) showed 0 tools after a reconnect/unquarantine. Fix the root cause so StateView stays the consistent source of truth: - On reconnect, repopulate StateView.Tools from the retained Supervisor snapshot (which keeps tools across a disconnect) instead of waiting for background discovery to re-run. - Drop the size-based guard in RefreshToolsFromDiscovery that skipped updates whenever the new set was smaller. It pinned StateView to a stale higher count when a server legitimately dropped tools, diverging from the snapshot (updated unconditionally). Servers with zero tools never reach that loop, so the guard never protected against empty discoveries anyway. - Extract toolInfosFromMetadata so reconcile, discovery refresh, and reconnect repopulation produce an identical StateView tool set. Adds reconnect->StateView-still-populated and shrinking-tool-set tests. Related: MCP-2083 (PR #635, read-layer symptom fix)
Deploying mcpproxy-docs with
|
| Latest commit: |
94c6147
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://19bbb767.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2094-stateview-tool.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27400596336 --repo smart-mcp-proxy/mcpproxy-go
|
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
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
2 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.
Summary
Root-cause fix for the StateView per-server tool repopulation race behind MCP-2083 (PR #635, which fixed the symptom at the read layer — the Tools tab falls back to the bleve index when the StateView per-server cache is empty).
The deeper problem remained: in
internal/runtime/supervisor/supervisor.go,status.Tools = nilon disconnect,RefreshToolsFromDiscoveryhad a guard that skipped the StateView update whenever the new tool set was smaller than the current one.Together these left the StateView per-server tool set transiently/persistently empty after a reconnect/unquarantine, so other StateView consumers that don't route through the #635 read fallback — tray tool counts, SSE
servers.changedcounts, health/diagnostics — could still show 0 tools for a connected server that has tools.Changes
connected=trueevent, repopulatestatus.Toolsfrom the retained Supervisor snapshot (the snapshot keepsToolsacross a disconnect; only StateView is cleared), so StateView is consistent immediately instead of waiting for background discovery to re-run. Background discovery still overwrites with fresh data afterward.RefreshToolsFromDiscovery— it pinned StateView to a stale higher count when a server legitimately dropped tools, diverging from the Supervisor snapshot (which is updated unconditionally) and the bleve index. Servers with zero discovered tools never reach that loop (absent fromtoolsByServer), so the guard never protected against empty/stale discoveries anyway. StateView now mirrors the snapshot last-writer-wins.toolInfosFromMetadataso reconcile, discovery refresh, and reconnect repopulation produce an identical StateView tool set (removes 2 copies of the conversion loop).Tests
TestSupervisor_ReconnectRepopulatesStateViewTools— disconnect clears, reconnect repopulates StateView (was 0, now 2).TestSupervisor_RefreshToolsFromDiscovery_ShrinkingToolSet— a later discovery reporting fewer (non-empty) tools now updates StateView instead of being silently skipped.Both fail on
origin/mainand pass with this change.Verification
No user-facing CLI/API/config/docs surface changes (internal runtime consistency fix), so no docs diff required.
Related: MCP-2083 (PR #635)