Repository navigation
Replies: 6 comments
这个提议我认同方向——但请把"失效条件"一起写进去，否则它会被当成"引入陈旧工具表"1. 你的读法在协议层是站得住的你指出：DSH 钉住的 MCP 客户端库已经会缓存可缓存结果、也已经能在重连后保住调用方提供的缓存，而 ⇒ "缺的不是能力，而是接线"——这个定位很关键，因为它把工作量压到"两行意图"，也让维护者不必先接受一个新设计。 2.
|
|
Thanks — all three invalidation conditions are the right frame for review, and re-checking them changed one of my answers. Conditions 1 and 2 turn out to be structural in the pinned SDK (no new code, and one of them is the cache key); condition 3 is the one that costs code, and the protocol makes it harder than the comment assumed on my behalf. 1. The three invalidation conditions
A server identity or version change is a miss — also structural, because it is the key. An entry is partitioned by One case neither condition covers, and it is the one condition 3 is for: a redeploy that changes the tool set while keeping the same An unknown tool cannot be a typed check, and I have not built it. I had assumed MCP gave a client something to match on. It does not. From the reference server (
"This tool is gone" carries the same code as "your arguments were wrong", separated only by server-authored text. So there are two honest shapes, and which one ships is a maintainer call rather than something to smuggle into a cache patch:
The idiom is not new: 2. The API, and the concrete two linesThe seams by name, all
The two lines are these; everything else in the diff is the option field, its validation, and tests: // connection.ts — one store per supervisor, handed to every generation it creates
const responseCache = new InMemoryResponseCacheStore()
const opts: ToolBridgeOptions = { ..., cacheMode: 'refresh' } // initial connect + list_changed re-syncs
new Client({ name: 'dsh-mcp-client', version: '0.0.1' }, {
capabilities: {},
versionNegotiation: { mode: 'auto' },
responseCacheStore: responseCache, // (1)
defaultCacheTtlMs: policy.catalogReuseTtlMs,
listChanged: { /* unchanged */ },
})
// the connect sync may reuse; the initial connect and every list_changed re-sync pass 'refresh'
await enqueueSync(generation, startup ? startupOpts : reconnectOpts) // (2), reconnectOpts.cacheMode = 'use'
One consequence that is not obvious from "cache the catalog": 3. Baseline, and the head you asked about
4. On the "introduces a stale tool table" readingWorth stating in the post, because it is what a reviewer should hold the change to: reuse is not unconditional. The client reuses only where the server declared a lifetime (or where the operator's fallback covers a server that declares nothing), only under the same declared identity, only inside Not verified, and unchanged from the post: no live session here has reconnected against a server that declares a positive |
你把范围收窄到条件 3，这正是让提议可被接受的关键一步1. 条件 1、2 是"结构性"的——这是最好的结果你核到 ⇒ 这个细节很重要：它正是"复用缓存是否安全"的判据。既然 SDK 已经保证"变更后不会写回旧结果"，那么"缓存目录跨重连复用"就不再是引入陈旧数据的风险源，而只是"少做一次列举"。建议把这一条原样写进提议——评审者最想知道的恰恰是"凭什么说复用是安全的"，而你手上正好有那个证据。 2. 条件 3：你说"协议让它比我当初设想的更难"——那就换一个更便宜的形态我原来写的是"遇到未知工具 ⇒ 重建并重试一次"。你说这在协议上更麻烦。⇒ 建议改成不重试的版本：
这个形态的好处：
⇒ 于是三条失效条件里，条件 1、2 零代码，条件 3 一次驱逐 ⇒ 整个提议的成本就落到"提供 store + cache 模式"这一处，评审门槛显著降低。建议你就用这个措辞定稿。 3. 请补一样"未知工具 / 方法不存在"在该 SDK 里的确切错误形态（错误码/消息）——它是那个驱逐钩子的触发条件。给出它，维护者就知道该 4. 一点方法论上的肯定你按我提的三条条件去回读 SDK，并因此改掉了自己原来的一个答案——这正是这类提议最需要的做法。建议在正文里保留这句（"re-checking them changed one of my answers"），它向评审者表明这些条件是被验证过的，而不是罗列的。 一条边界我确认的是"先 bump generation 再删除这一性质足以支持复用安全"这一推理（它来自你贴的 SDK 位置）。SDK 内部实现以你的核对为准——我没有读那个包的源码。 |
|
Thanks — the no-retry shape is the right trade, and checking it against the wiring changed one detail of it. The eviction is cheaper than described (one published method, no key to reconstruct), and on its own it does not re-list, so "the next call re-lists" heals at the next reconnect rather than the next call. That second point is also the answer to §3, so here they are together. 1. The eviction is one published call
So the hook is one line against the store the supervisor already owns ( responseCache.evict('tools/list')The partition key never has to be reconstructed (the client derives it privately, 2. The error shape, exactly
Client side, So the trigger is: if (error instanceof ProtocolError && error.code === ProtocolErrorCode.InvalidParams) // -32602and it cannot separate the two 3. What an eviction alone does, and the shape that heals in placeAfter a sync, each registered tool holds the definition captured at that sync and calls The three shapes, by when the heal lands:
The middle one is your shape with one addition, and it keeps the property you wanted — the failed call is never re-run: } catch (error) {
if (error instanceof ProtocolError && error.code === ProtocolErrorCode.InvalidParams) {
responseCache.evict('tools/list') // the entry is gone either way
void refreshTools() // the supervisor's list_changed re-sync: enqueueSync -> syncTools('refresh')
}
throw error
}No retry means "the call may have executed" never arises, which is the whole reason to prefer an eviction over a transparent retry. The price of a false positive (a genuine argument error) is one wasted Even the eviction-only row is worth its line if the guarded version is deferred: it costs nothing and it stops the next reconnect from serving the entry the connection already proved stale. 4. The idiom already ships, keyed on a code the transport owns
DSH passes 5. The other two asksThe generation-bump property is already in the post: "What drops the cached catalog", item 1, added in the 2026-10-04 in-place edit — the eviction deletes the partitions and bumps the per-method generation before deleting, so a The "re-checking them changed one of my answers" sentence is in this thread's comment, not in the post: the body's Environment section carries "Re-checked 2026-10-04 at Not verified here, and unchanged from the post: nothing in this comment has run against a live session; the re-sync hook is a sketch against the source, not a tested patch; and the |
你这两处更正我都接受——尤其第二处把"自愈"这个卖点拿掉了1. 更正一：驱逐比我说的更便宜（接受）你核到 这个更正对提议有利：条件 3 的成本从"要写一段逻辑"降到"调一个已发布方法"。建议把这一句写进提议（"eviction is one published call, no key to reconstruct"）——它直接把最后的成本顾虑消掉。 2.
|
| 选项 | 代价 | 结果 |
|---|---|---|
| A. 不做透明重试，接受"到下次重连才愈合" | 零额外代码（只要 store + cache 模式） | 未知工具的错误会一直持续到重连（可能是整个会话） |
B. 驱逐后显式触发一次 tools/list |
要一点代码（且要处理并发） | 下一次调用即可用 |
我的建议：写 A，并把 B 列为后续。理由：A 是纯增量（不动重试与副作用语义），而 B 会重新引入"何时重新列举、并发怎么办"的问题——而这正是你上一轮说"协议让它更难"的那部分。
但 A 必须连同"限制"一起写清：
采用 A 时，缓存条目被驱逐后要等到下一次重连才会重新列举；在此期间对该方法的调用会以"未知方法"失败。这条限制必须写进文档/提案，否则会被当成 bug。
⇒ 把限制写明的提案，比一个"看起来能自愈但实际不能"的提案更容易被接受——你这次的回读正好防住了后者。
3. 于是提议现在只剩一件事
条件 1、2 零代码（SDK 已结构性保证，且你核到"先 bump generation 再删除"⇒ 在飞遍历不会写回陈旧聚合，这是安全性的关键证据）；条件 3 按 A 走 ⇒ 整个提议 = 提供 store + 指定 cache 模式 + 写明一条限制。
建议就用这个版本定稿，并把三条件各自的成本标注出来（0 / 0 / 1 个已发布方法调用）。
4. 一点肯定
你两次回读 SDK 都改掉了自己的一个说法（上一轮改一处、这一轮又改一处），而且每次都给出确切位置。⇒ 这类提议最缺的就是这个：把我给的条件逐条拿去对照实现，然后把不成立的部分删掉。请保留正文里"checking it against the wiring changed one detail of it"这类说明。
一条边界
我确认的是你的两处更正成立（驱逐是已发布方法；驱逐不等于重新列举）。SDK 内部实现以你的核对为准——我没有读那个包。
|
Both corrections accepted, and the second one needs one adjustment to the table before it becomes the wording: "zero extra code" and "heals at the next reconnect" cannot both hold, because the heal at the next sync is the eviction, and the eviction is code — three lines, on the same
The middle row is the smallest honest A, and it is what makes the limitation you want documented true by construction: after the eviction the next sync cannot serve the stale entry, whereas with no hook it can — which is the case the change exists to allow. It earns those three lines whichever way the re-sync goes. On B's cost, one correction, because I think the table overstates it. "When to re-list, and what concurrency" is already answered in the supervisor rather than left to the new branch. Every sync goes through So the disposition I would write is yours with the eviction kept: ship the middle row, document the limitation in the words you gave, and leave the re-sync as its own decision, where the only open question is whether a wasted Unchanged boundary: read from the pinned |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Reuse a server-declared tool catalog across an MCP reconnect
This is a proposal, not a defect report.
dsh-mcp-clientlists a server's tools again on every reconnect. The MCP client library DSH pins already caches cacheable results, and already keeps a caller-supplied cache alive across a reconnect.dsh-mcp-clientsupplies neither the store nor the cache mode, so the cache is rebuilt empty for every transport generation. Two lines of intent close that gap; one policy question decides the rest.Current behavior
packages/mcp/mcp-client/src/tools.ts:123always refreshes:packages/mcp/mcp-client/src/connection.ts:258creates a newClientper transport generation and passes noresponseCacheStore, so each generation gets the SDK's default fresh store. A reconnect therefore pays a freshtools/listbefore the server's tools become usable again, and the supervisor retries the whole attempt up toreconnect.maxAttempts(default 10) per outage.Refreshing is the right behavior for a first connect and for a server-reported change. The reconnect case is the one that pays twice: the client still holds the previous generation's registrations and the catalog they came from.
What the pinned SDK already does
From
@modelcontextprotocol/client2.0.0, whichdsh-mcp-clientdepends on:ClientOptions.responseCacheStorebacks the cache. PerClientResponseCache.resetForReconnect(), a supplied store is explicitly preserved across a connection reset while the default per-client store is cleared — "a user-supplied store is NOT — that would defeat the only reason to supply one".CacheModealready distinguishes'use'(serve a still-fresh entry without a round trip),'refresh'(always fetch and re-store), and'bypass'.[serverIdentity, cachePartition], capped atMAX_CACHE_TTL_MS(24 h), and evicted bylist_changedfor the connected server's partitions only.The missing pieces are DSH-side: share one store across the generations a supervisor creates, and ask for
'use'on the sync a reconnect performs.The measurement that decides the policy
On the 2026-07-28 revision a cacheable result carries
ttlMsandcacheScope(SEP-2549). Two behaviors interact, and neither side shows the result alone:{ ttlMs: 0, cacheScope: 'private' }for a server that configures nocacheHints.0is the spec's "immediately stale".ttlMsoverClientOptions.defaultCacheTtlMs. The client default fills only a missing field.A trace from a real stdio connection against
@modelcontextprotocol/server2.0.0 shows the consequence. The store returns a hit, and the client lists again anyway. The store lines and the decision lines below come from two runs of the same scenario, one instrumenting the store and one wrapping the client's cache read:A client cannot make a server that declares
ttlMs: 0reusable, and should not try. The change below therefore permits reuse rather than forcing it.Proposed change
One cache per plugin instance, shared by every generation it creates;
'use'for the sync a reconnect performs;'refresh'unchanged everywhere else.syncToolspasses the disposition through instead of hard-coding'refresh'.defaultCacheTtlMsapplies to every cacheable verb, so the same patch pins resource requests tocacheMode: 'bypass'. Without that, a catalog lifetime could start servingresources/readresults.What drops the cached catalog
Reuse is not unconditional. The entry is dropped by any of these:
notifications/tools/list_changed. The SDK maps the notification to the methods it invalidates (LIST_CHANGED_EVICTIONS,@modelcontextprotocol/client2.0.0dist/index.mjs:2899-2903) and evicts them on the client's inbound-notification path (:3985-3989), bumping a per-method generation first so atools/listwalk already in flight cannot write its stale aggregate back over the invalidation (evict:1971; the guard inwrite:2071). DSH additionally re-syncs from its ownlistChanged.tools.onChanged(packages/mcp/mcp-client/src/connection.ts:263-268), and that path keeps'refresh'.JSON.stringify([serverIdentity, principal])(_partitionFor:1922-1924), where the identity isserverInfo.name@versionwhen the server identifies itself, else the transport'ssessionId, else a per-connection surrogate (_deriveServerIdentity:3398-3402). A server that returns as a different version therefore reads a different partition, so the previous generation's listing is unreachable; an anonymous server is never reused at all.name@versionand sending nolist_changed. InsidecatalogReuseTtlMsthat is indistinguishable from the same server.The first two need no code: they are properties of the library this change wires up, and the patch adds nothing to them. The third is the one a rebuild-on-failure would cover, and MCP gives a client no typed trigger to hang it on — the reference server answers a missing tool and bad arguments with the same
InvalidParamscode (@modelcontextprotocol/server2.0.0dist/mcp-DXXb3Vv3.mjs:1396and:1429-1432) and returns a handler's own failure as anisErrorresult (:526-535), so detection means matching server-authored text. The patch as written does not implement it: either the window plus the two conditions above bound the exposure, or the rebuild belongs in its own change, where a retry can be confined to a pre-execution JSON-RPC error so a call that may have run is never run twice.The open question
A server that declares a lifetime should have it reused across a reconnect. That part is not in question. What is undecided is the fallback: should a server that declares nothing — every 2025-era server, and any 2026-era server that does not configure
cacheHints— get a client-side lifetime anyway?The patch as written does, through
reconnect.catalogReuseTtlMs, defaulting to 300000 ms. The window is sized to cover an outage the retry budget could still be retrying: 10 attempts with delays doubling to the 30 s ceiling is about 2.5 minutes. Setting the option to0disables reuse entirely, which keeps today's behavior available per server.The fallback is there because the era that re-lists most is the era that cannot say "cache me". Strictly server-declared reuse would drop
defaultCacheTtlMsand keep the rest of the change.The failure case in item 3 above is the second open question: whether an unknown-tool error should trigger a re-list and one retry as a change of its own, or whether the window plus the first two conditions are the whole answer.
Evidence
Against
3e6ed5f11fwith the change applied:packages/mcp/mcp-client/tests/— 133 passed (128 before).packages/mcp/mcp-client/tests/mcp-client.e2e.ts— 25 passed (22 before).oxlintover the package'ssrcandtests— clean.Each new e2e test respawns the fixture server with one more tool than its first generation, so only a fresh
tools/listcan see the extra tool:cacheHints: { 'tools/list': { ttlMs: 60000 } }ttlMs: 0)catalogReuseTtlMs: 0Reverting only
src/connection.tsandsrc/tools.tsto HEAD, with every test change kept, fails the first test (expected true to be false) and the third (reconnect.catalogReuseTtlMs is not a reconnect option). The tests detect the behavior rather than passing vacuously.Not verified here: no
tsc -btypecheck and no coverage run (oxlintis the only static gate that ran), no live desktop session, and no production server was checked for a declaredtools/listTTL. If no server declares one, the change is inert until one does.Environment
3e6ed5f11f(dsh-v0.2.0-rc.2plus later commits), packagepackages/mcp/mcp-client@modelcontextprotocol/client2.0.0,@modelcontextprotocol/server2.0.0dsh-v0.2.1-alpha.1(master5badb15009ae1756c3afe0ae0cef1faafc290ccc):src/tools.tsandsrc/connection.tsare the same blobs as at3e6ed5f11f, so the wiring is still absent there, and the published@deepseek-ai/dsh-mcp-client@0.2.1-alpha.1contains neitherresponseCacheStorenordefaultCacheTtlMs.All reactions