fix(mcp): detect live tool catalog changes on connected servers - #1771
Open
Aaronontheweb wants to merge 1 commit into
Open
fix(mcp): detect live tool catalog changes on connected servers#1771Aaronontheweb wants to merge 1 commit into
Aaronontheweb wants to merge 1 commit into
Conversation
The daemon enumerated an MCP server's tool catalog exactly once per client lifetime (at connect/reconnect). A server that added, removed, or changed tools mid-session stayed invisible to the model until a disconnect + reconnect. Reuse the reconnection service's 30s health tick to also re-list healthy servers' catalogs on the live client, throttled to once per 5 minutes per server. A SHA-256 content fingerprint over the sorted, schema-canonicalized tool surface (name, description, input schema, return schema) detects any add, remove, rename, or schema edit without a reconnect. A changed catalog republishes the snapshot with a bumped generation and re-publishes the tool registry; a failed or empty refresh keeps the last good catalog (never wipes the index). The re-list is bounded by a 15s per-refresh timeout so one hung server cannot stall the poll loop, block a reconnect on the per-server gate, or delay shutdown. A failed refresh rolls back its throttle claim so the next 30s tick retries rather than waiting out the full interval. Addresses #1769.
![github-code-quality[bot]](https://avatars.github.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+331
to
+341
| catch (Exception ex) | ||
| { | ||
| // Invariant: a failed refresh must never empty the catalog. Roll back the | ||
| // throttle claim so the next 30s tick retries instead of waiting 5 minutes. | ||
| lifecycle.RollbackCatalogRefreshClaim(previousRefreshMs); | ||
| _logger.LogWarning(ex, | ||
| "MCP server '{Name}' catalog refresh failed; keeping generation {Generation} unchanged", | ||
| current.Name.Value, | ||
| current.Generation); | ||
| return false; | ||
| } |
Comment on lines
+154
to
+161
| catch (Exception ex) | ||
| { | ||
| // A refresh failure is not a connection failure — the manager keeps the | ||
| // last good catalog and the next tick retries. Log and move on. | ||
| _logger.LogDebug(ex, | ||
| "MCP server '{Name}' catalog refresh threw an exception", | ||
| serverName.Value); | ||
| } |
Aaronontheweb
added
bug
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
1 participant
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.
What
Closes #1769. The daemon enumerated an MCP server's tool catalog exactly once per client lifetime — inside
McpClientRuntime.InitializeAsync, only reached at connect/reconnect. A server that added, removed, renamed, or edited tools mid-session stayed invisible to the model until a disconnect + reconnect.This reuses the reconnection service's existing 30s health tick to also re-list healthy servers' catalogs on the live client (no client rebuild, no OAuth re-run), throttled to once per 5 minutes per server.
How
McpClientManager.TryRefreshCatalogAsync(serverName, ct)— re-lists tools via a newIMcpClientRuntime.ListToolsAsyncseam, computes a SHA-256 content fingerprint over the sorted, schema-canonicalized tool surface (name, description, input schema, return schema), and republishesMcpServerSnapshotwithGeneration + 1+ aToolRegistryupdate only when the fingerprint changed. Same "connection first, tools second" publish ordering as the connect path.McpReconnectionService— the 30s tick's connected branch callsTryRefreshCatalogAsync. Cadence/throttle lives in the manager (McpServerLifecycle.TryClaimCatalogRefresh), so the service stays dumb.IMcpReconnectable.TryRefreshCatalogAsync— new interface method so the service can reach the refresh through its existing abstraction.Safety invariants
CatalogRefreshTimeout) — one hung server cannot stall the poll loop, block a reconnect on the per-server gate, or delay daemon shutdown.Design notes
notifications/tools/list_changed, so the notification path was deliberately omitted (fewest moving parts). The refresh mechanism is the exact seam prompts/resources support will hang off later.Tests
New
McpCatalogRefreshTests(10 tests): change republishes generation; no-change doesn't bump; throttle within interval; failed refresh keeps last good; failed refresh rolls back throttle so next tick retries; empty catalog keeps last good; unknown server no-op; fingerprint order-stability; fingerprint changes on description/tool-add; canonical schema ignores key order/whitespace.All 144 MCP tests in
Netclaw.Daemon.Testspass. Full diff reviewed by three adversarial sub-agents (concurrency, security, simplification) — their findings (throttle leak, unbounded gate hold, empty-catalog wipe, schema churn, dead code) are all addressed in this commit.