fix(mcp): own discovery by connection generation - #2663
Conversation
4bbc2fd to
a0e4a94
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Codex automated review
I reviewed exact head a0e4a944874e4d561730c4c7751f8d6c0ca1dc0c across initial discovery, explicit refresh, tools/list_changed, reconnect/disconnect, stale-client closure, schema preparation, snapshot/binding publication, and status/capability listeners. I found no specific, reproducible P0–P3 issue.
The single generation-owned discovery state is a real simplification: epoch coalescing suppresses superseded lists, ownership is checked before and after publication, and invalid preparation leaves the previous callable snapshot intact. The three-file change is one cohesive transaction and I did not find a useful split or low-value test block to remove. Current checks are green.
Disclosure: This is an automated review performed by Codex using delegated adversarial review passes and a final evidence check. It has not been independently verified by Astro-Han or another human reviewer, does not constitute human approval, and does not represent the final judgment of a human reviewer.
a0e4a94 to
6bf555d
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Following up my earlier Codex pass with a deeper look, since that one reported no findings and this PR carries a behaviour change. The concurrency design holds up — that part of the earlier conclusion stands, and I now have evidence for it rather than an absence of findings. What it missed is a blast-radius regression and the fact that the PR's own headline invariant is not pinned by its tests.
Concurrency: verified, not just unrefuted. 90 seeded fuzz iterations mixing refreshTools / list_changed / reconnect / disconnect / sync produced zero stale publishes, zero snapshot-status divergences, zero hangs. Specifically confirmed correct: epoch coalescing (10 notifications during a held list collapse to exactly one follow-up), single-flight (3 concurrent refreshes → 2 lists, all three get the newest result), stale rounds always rejecting during disconnect/reconnect, the post-publish ownership recheck catching a synchronous listener that retires the generation, and ABA being impossible since ownership compares discoveryState by object identity. Preparation failure keeping the previous snapshot is atomic and real — snapshot identity unchanged, revision unchanged, old bindings still callable.
One thing blocks: a single bad outputSchema now takes down the whole server. Same probe against both trees:
origin/main |
this PR | |
|---|---|---|
| refresh with one bad tool among three | ok, ["echo","bad","other"] |
whole refresh rejects |
| fresh connect with one bad tool | connected, 3 tools usable |
error, 0 tools, transport closed |
The bar is low. {type:"string",pattern:"["} does it, and so does any tool declaring draft-04 — the SDK's _engineFor throws for any $schema outside 2020-12 / 2019-09 / draft-07 / draft-06, which is common in shipped MCP servers. On refresh the tool list then freezes permanently with status.error stuck, because every subsequent list_changed fails identically.
"Prepare validation fully before publishing" is the right goal and I don't want to lose it. The implementation is all-or-nothing, which is the opposite failure from silently dropping bad tools but not obviously better: one malformed advertisement disables every healthy tool on that server, with no recovery path short of the server changing what it advertises. Dropping just the offending tool from the published snapshot and surfacing a per-tool diagnostic keeps the guarantee for everything callable. Detail inline.
Note that the deleted rejects a malformed output schema before invoking the Tool was the test pinning the old blast radius, so this is a deliberate replacement rather than an oversight — it just deserves to be an explicit decision in the description rather than a side effect of the new preparation order.
The headline invariant is only half-pinned. The PR states that a candidate may publish only while its connection generation and change epoch are current. Mutation testing says the epoch half is solid — four separate mutations all go red. The generation half does not: deleting the entire generation fence on the tools/list_changed handler changes nothing, and the three identity clauses in ownsToolDiscovery can be removed together while every test stays green (so can the closing/isClosed pair, in the other direction). Only turning the whole guard into true reddens anything. So the tests pin "some ownership check exists", not "ownership is by connection generation". Inline.
Unrelated to this PR, worth its own issue. A server that emits tools/list_changed after every tools/list response livelocks discovery — I measured ~2000 list round trips in 4 seconds with sync() never settling and the manager unable to even remove that server. I initially took this for a regression from installing the handler before initial discovery; it is not. origin/main produces the same 2000-odd loop under the same probe. Pre-existing, and it recovers as soon as the server stops notifying, but a bound on the retry loop would be worth having, and this PR is a reasonable place to note it rather than fix it.
Remaining P2/P3 inline. Nothing else blocks.
Review assistance: Claude Code (Opus) ran three independent fresh-eye passes — concurrency/interleaving, state machine and failure paths, and mutation-based test effectiveness — each blind to the others, in isolated worktrees. I reproduced the blast-radius table and the livelock comparison against both trees myself, and discarded one reported P0 whose origin/main baseline did not hold up. Per AGENTS.md this is AI-assisted review and does not constitute the independent human review this change still needs.
中文
接着我之前那次 Codex 评审再深入看了一轮——那次报告没有发现，而这个 PR 带有行为变更。并发设计经得起检验，之前结论的这一部分成立，而且现在我有正面证据而不只是「没找到问题」。它漏掉的是一处爆炸半径回归，以及这个 PR 自己的核心不变量并没有被它的测试钉住。
并发：是验证过的，不只是没被推翻。 90 次 seeded fuzz，混合 refreshTools / list_changed / reconnect / disconnect / sync，零次陈旧发布、零次快照与状态分歧、零次挂起。明确确认正确的有：epoch 合并（一次 list 期间来 10 条通知，恰好合并成 1 次跟进）、single-flight（3 个并发 refresh → 2 次 list，三者都拿到最新结果）、disconnect/reconnect 期间陈旧轮次总是拒绝、发布后的所有权复检能抓住在同步监听器里退休 generation 的情况、以及 ABA 不可能（所有权按对象同一性比较 discoveryState）。准备失败时保留旧快照是原子且真实的——快照同一性不变、revision 不变、旧 binding 仍可调用。
有一处阻塞：一个坏的 outputSchema 现在会打死整台 server。 同一探针对两个源码树：
origin/main |
本 PR | |
|---|---|---|
| 三个工具中一个坏 schema 的 refresh | ok，["echo","bad","other"] |
整次 refresh 拒绝 |
| 含一个坏工具的新建连接 | connected，3 个工具可用 |
error，0 个工具，传输被关闭 |
触发门槛很低。{type:"string",pattern:"["} 就够，任何声明 draft-04 的工具也一样——SDK 的 _engineFor 对 2020-12 / 2019-09 / draft-07 / draft-06 之外的任何 $schema 都直接抛错，而这在已发布的 MCP server 里很常见。走 refresh 路径时，工具列表会被永久冻结，status.error 卡住，因为之后每一次 list_changed 都以同样方式失败。
「发布前完整准备校验」这个目标是对的，我不希望丢掉它。但实现是全有全无，这是「静默丢弃坏工具」的反面，却不见得更好：一条畸形通告会让那台 server 上所有健康的工具全部不可用，而且除非 server 改变它通告的内容，否则没有恢复路径。只把出问题的那个工具从发布的快照里剔除、并给出 per-tool 诊断，就能对所有可调用的工具保住这个保证。细节在行内。
顺带说明：被删掉的 rejects a malformed output schema before invoking the Tool 正是钉住旧爆炸半径的那个测试，所以这是有意替换而非疏漏——只是它值得成为描述里的一个显式决定，而不是新的准备顺序带来的副作用。
核心不变量只钉住了一半。 PR 声明：候选只能在其 connection generation 与 change epoch 仍然当前时才能发布。Mutation testing 表明 epoch 那一半是扎实的——四处不同的 mutation 全部变红。generation 那一半则不然：把 tools/list_changed handler 上整段 generation fence 删掉，什么都不变；ownsToolDiscovery 里三个 identity 子句可以一起删掉而所有测试仍绿（反过来删掉 closing/isClosed 那一对也一样）。只有把整个 guard 变成 true 才会红。所以测试钉住的是「存在某种所有权判定」，而不是「所有权按 connection generation 判定」。细节在行内。
与本 PR 无关，值得单独开 issue。 一台在每次 tools/list 响应之后都发 tools/list_changed 的 server 会让 discovery 活锁——我实测 4 秒内约 2000 次 list 往返，sync() 永不 settle，manager 连移除这台 server 都做不到。我起初以为这是「把 handler 装在初次 discovery 之前」引入的回归，并不是：同一探针在 origin/main 上产生同样约 2000 次的循环。这是既有问题，而且 server 一停止发通知就会恢复；但给重试循环加一个上界是值得的，本 PR 是个适合记录（而非修复）它的地方。
其余 P2/P3 在行内。没有别的阻塞项。
评审协助说明：Claude Code (Opus) 跑了三轮相互隔离的 fresh-eye 审查——并发与交错、状态机与失败路径、基于 mutation 的测试有效性——彼此不知道对方的发现，各自在独立 worktree 中进行。爆炸半径那张表和活锁的双树对比是我自己复现的，另有一条被报告的 P0 因其 origin/main 基线站不住而被我剔除。按 AGENTS.md，这属于 AI 辅助评审，不构成这个改动仍然需要的独立人类评审。
6bf555d to
bf35afa
Compare
📝 WalkthroughSummaryThis PR makes MCP tool discovery owned by the connection generation. It installs the generation-fenced The PR preserves the last callable tool snapshot when refresh fails or refresh notifications exceed the retry budget. It validates each tool output schema during its first invocation. A malformed schema affects only that tool call. The changes extend the existing discovery and snapshot paths. They do not create a parallel discovery path. The shared refresh owner handles initial discovery, explicit refreshes, and notification-triggered refreshes. The solution is the smallest coherent design shown by the diff. The shared owner prevents duplicate requests. Generation checks prevent retired connections from publishing state. Rechecks after synchronous status listeners prevent retired generations from continuing. Per-call validator creation prevents The tests cover:
No safe deletion or simplification is evident. The removed Complexity delta
Total maintenance complexity appears justified and likely decreases because the PR removes parallel revision state and consolidates discovery ownership. ValidationThe PR reports successful workspace tests, MCP tests, type checking, linting, formatting checks, third-party notice checks, and five consecutive runs of the race-focused tests. The reported MCP test result is 66/66. Direct evidence for the final check results is not available in the provided context, so required checks remain unverified here. Review-relevant risksThe PR changes user-visible MCP behavior by changing tool discovery timing, refresh coalescing, cancellation, failure recovery, notification suppression, and output-schema validation. Material changes in these areas require independent human review under repository policy. No public API declaration changes, security changes, licensing changes, release changes, or governance changes were identified in the current diff. The person performing the merge reviews the final diff, and a maintainer makes the final determination. WalkthroughThe PR unifies initial MCP tool discovery with refresh handling, adds cancellation and race recovery, and publishes current snapshots after queued refreshes. It also creates output-schema validators per prepared tool and adds regression coverage. ChangesMCP refresh and validation updates
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The MCP discovery changes are merge-ready after normal checks and review; the only identified concern is optional defensive hardening for an unreachable refresh path, so no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant startToolRefresh
participant refreshToolLoop
participant ToolServer
participant ToolSnapshot
MCPClient->>startToolRefresh: Start initial discovery
startToolRefresh->>refreshToolLoop: Pass discovery and abort options
refreshToolLoop->>ToolServer: Request tools/list
ToolServer-->>MCPClient: Send tools/list_changed
MCPClient->>startToolRefresh: Queue notification refresh
ToolServer-->>refreshToolLoop: Return tool descriptors
refreshToolLoop->>ToolSnapshot: Publish the latest snapshot
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed exact head bf35afa3602d36ba9ce1f48390f4104633d8a895, the complete diff, CI, all previous review threads and author replies, and the current MCP discovery lifecycle.
The output-schema blast radius, refresh-gate settlement window, failed-refresh release, and cross-tool $id collision findings are fixed on this head. Per-tool call preparation and isolated Ajv compilation are sensible corrections.
One P2 remains in the PR’s core claim: initial discovery is still not owned by the same notification/refresh transaction.
connectEntry() awaits the first tools/list before installing the manager’s notifications/tools/list_changed handler. I reproduced this with a real SSE MCP transport:
- the server begins the initial
tools/list; - it sends
tools/list_changed; - it returns the old list;
- the client receives the notification;
- only then does the manager install its handler.
Both the base and this head perform one list request and publish ["old"]; no follow-up discovery occurs. This is not a new regression, but the PR explicitly claims to install the generation-fenced handler before initial discovery and route initial discovery, explicit refresh, and notifications through one owner. The latest implementation still leaves initial discovery as a parallel path.
The minimal root fix is to install the generation-owned handler first and make initial discovery enter the same coalescing/single-flight owner as later refreshes. Merely moving the handler earlier while retaining two concurrent discovery paths would leave another race.
I would hold approval until that stated invariant is actually true and covered by a response-before-notification transport regression. No PR split is needed.
Disclosure: Codex coordinated independent read-only review passes and a final real-transport reproduction on the exact head. The human contributor remains responsible for verifying the evidence and deciding whether to merge.
中文
既有 schema、gate 和 $id 问题已修。仍有一条 P2：初次 discovery 依然先 list、后安装通知 handler。真实 SSE 探针证明响应前通知会被丢弃并发布旧 snapshot。应让初次 discovery 真正进入同一个 generation-owned transaction。
bf35afa to
32bea79
Compare
|
The remaining P2 is fixed at head Root fix, as suggested: the generation-fenced
Regression (real SSE transport, response-after-notification): the server holds the first Evidence: @maka/mcp 66/66 (was 65; +1 regression), the race-focused tests pass 5 consecutive runs, biome clean. 中文初次 discovery 不再绕过事务：handler 先于初次 tools/list 安装，初次 list 进入与显式 refresh/通知相同的 single-flight owner；响应前到达的 list_changed 会合入当前 pass 并恰好补一次 list，不再被丢弃。连接期取消语义不变（signal 穿透到首个 list）。真实 SSE 回归已新增并对未修复行为验证失败。mcp 66/66，竞态用例 5 连跑全绿。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/mcp/src/index.ts (1)
332-343: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAssign
entry.refreshStatebefore invoking the loop. Disposition: optional.
refreshToolLoopis async, so its prologue (Lines 708-728) runs synchronously until the firstawait. If that prologue throws, thefinallyat Line 791 executes before Line 342, so theentry.refreshState === stateguard fails and Line 342 then installs an already-rejected state as the active gate. Every later join for the same client and generation would receive that rejected promise permanently.The path is not reachable today:
refreshToolsAfterNotificationvalidates the notification state,suppressed, and the pass budget before it callsstartToolRefresh, so the prologue cannot throw on the only path that starts withpendingNotification: true. The ordering is still fragile against future prologue checks, and the correction is one statement swap.♻️ Assign the gate first
- state.promise = this.refreshToolLoop(serverId, entry, state); entry.refreshState = state; + state.promise = this.refreshToolLoop(serverId, entry, state); return state.promise;packages/mcp/src/__tests__/manager.test.ts (1)
151-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
waitForweakens the claim this test makes. Disposition: optional.The behavior under test is that the notification joins the initial pass, so the replacement snapshot must already be published when
syncPromiseresolves.waitForat Line 152 also accepts a snapshot that lands after connect. Thetools/listcount of 2 still rejects a dropped notification, so coverage is not lost; asserting directly afterawait syncPromisepins the ownership claim more precisely.♻️ Assert synchronously after connect resolves
await syncPromise; - await waitFor(() => manager.toolSnapshot().tools[0]?.descriptor.name === 'replacement'); assert.deepEqual( manager.toolSnapshot().tools.map(({ descriptor }) => descriptor.name), ['replacement'], );
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c07dbcc-cdbd-4df9-9de0-c0e341a7ac82
📒 Files selected for processing (4)
packages/mcp/src/__tests__/manager.test.tspackages/mcp/src/__tests__/tool-output-validation.test.tspackages/mcp/src/index.tspackages/mcp/src/tool-output-validation.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for consolidating initial discovery and notification refresh under one connection-generation owner. I reviewed this four-file change with an independent @reviewer pass plus a read-only ollama-cloud/deepseek-v4-flash:high pass. Installing the generation-fenced notification handler before initial discovery and committing the initial snapshot together with connected are both the right invariants.
The remaining issue is in the post-connect branch: the loop publishes each candidate snapshot before checking whether a list-changed notification arrived during the awaited tools/list. That briefly advertises exactly the snapshot the server has declared stale, and callTool can resolve a binding from it before the next pass finishes.
The first-principles/Occam model is one refresh transaction: collect candidates privately, observe pending/generation/connection ownership, and publish once only when the pass is quiescent (or at the explicit suppression boundary). Initial and later refreshes can share that same commit rule instead of having separate publication timing.
Current CI is green. No local test suite was run during this review; conclusions are based on source, test, and current CI inspection. Codex performed the final adjudication; external-model output was treated as unverified until checked against the code.
中文摘要
感谢把 initial discovery 和 notification refresh 收敛到同一个 connection-generation owner。提前安装 generation-fenced notification handler，并把 initial snapshot 与 connected 一起提交，都是正确不变量。
剩余问题在 post-connect 分支：tools/list await 期间若收到 list-changed，当前 loop 会先发布这份已被服务器声明过期的 candidate，再检查 pending 并进入下一轮；这段窗口里 callTool 可能解析到 stale binding。
最小模型是一笔 refresh transaction：candidate 保持私有，先核对 pending/generation/connection ownership，只有 quiescent 或明确 suppression boundary 才一次发布。initial 与 later refresh 可以共享同一 commit rule，不需要不同的 publication timing。
当前 CI 全绿。本次未运行本地测试套件；结论来自源码、测试与 CI 检查。外部模型输出在核对代码前均视为未验证输入。
The shared SDK validator cache resolved schemas by $id, so two tools that reused an $id shared whichever validator was registered first, across connection generations and server trust boundaries. Compile per preparation so each published tool owns its validator.
A promise .finally cleared the refresh state one microtask after the loop settled, so a refresh requested from a subscriber queued during publication joined a settled promise and received descriptors from a list that started before it asked. Clear the gate in the loop's finally so every exit releases it before the promise settles, and pin the failure-then-success release.
The generation-fenced list-changed handler was installed only after the first tools/list completed, and initial discovery ran as a direct list outside the single-flight refresh owner. A tools/list_changed that arrived while the first response was still in flight was dropped, so the connection settled on a snapshot the server had already declared stale, and no follow-up discovery occurred. Install the handler before initial discovery and run the first list through startToolRefresh/refreshToolLoop with an initial-scoped state: the connected status guard is relaxed only for that state, the connect abort signal is threaded into the in-flight first list so cancelling an installation still settles promptly, and a notification received during the pass joins the same coalescing owner as later refreshes. Regression: a real SSE transport holds the first tools/list, changes its list, sends the notification, and only then answers with the stale list; the manager must publish the replacement through exactly one follow-up list. The test fails against the previous behavior (notification dropped, snapshot stays stale).
Prepare initial discovery snapshots without exposing them while the client is still connecting. Commit the callable snapshot with the connected status, and degrade notification storms to the last valid initial result instead of tearing down a live server.
18d9127 to
85e39fb
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The problem and owner are correct: initial discovery, explicit refresh, and notifications should share one generation-fenced publication transaction. Two concrete P2 gaps remain in that state machine. The notification budget is connection-lifetime rather than burst-scoped, and synchronous onChange re-entry happens after the final pending/authority check, so a queued refresh can be lost or a retired generation can still resolve successfully. Both belong in the existing refresh loop; no second authority is needed.
AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 85e39fb372a3a5b1338377318a73187eee013724, the synchronous listener seam, refresh/generation state, and current CI. No local tests were run.
中文审查
问题定义和责任层正确：initial discovery、显式 refresh 与 notification 应共享同一个 generation-fenced 发布事务。当前还有两个 P2：notification budget 实际绑定整个连接生命周期，而不是一次 burst；同步 onChange 重入发生在最后一次 pending/authority 检查之后，会丢掉排队刷新，或让已退休 generation 仍成功返回。两项都应在现有 refresh loop 内修复，不需要第二套权威。
本次为 AI 辅助审查，已核验精确 head、同步 listener seam、刷新/generation 状态和当前 CI；未运行本地测试。
|
/agentic_review |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mcp/src/index.ts (1)
342-353: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAssign
entry.refreshStatebefore you start the loop.
refreshToolLoopis called at Line 351, andentry.refreshState = stateis set at Line 352. The loop body runs synchronously up to the firstawaitat Line 775. If that prefix throws (notification state mismatch or budget exhaustion at Lines 751-769), thefinallyat Line 842 runs before Line 352 executes. The guardentry.refreshState === stateis then false, so the gate is not cleared. Line 352 afterwards installs a state whose promise is already rejected, and every laterrefreshToolsor notification joins that settled rejection for the life of the connection.The current callers make that prefix unreachable:
refreshToolsAfterNotificationre-checks suppression and the pass budget synchronously before it callsstartToolRefresh, and non-notification passes never enter that branch. So this is not a reachable defect today. Disposition: optional. The reordering removes the dependency on that caller-side invariant at no cost.♻️ Install the gate before the loop can settle
- state.promise = this.refreshToolLoop(serverId, entry, state); entry.refreshState = state; + state.promise = this.refreshToolLoop(serverId, entry, state); return state.promise;
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ddadba0-7865-4513-89b8-2372f76479fc
📒 Files selected for processing (2)
packages/mcp/src/__tests__/manager.test.tspackages/mcp/src/index.ts
Included review availability: Your plan provides up to 3 included reviews per hour; 1 remains after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The generation-owned single-flight design is now coherent across initial discovery, explicit refresh, and tools/list_changed. Reentrant callbacks, synchronous disconnect, failed-refresh gate release, and notification bursts all have focused coverage, with no parallel discovery authority left behind.
No remaining P0-P3 findings on this exact head. The current Desktop E2E failure is in the unrelated /side command menu spec rather than this packages/mcp change; it still needs a successful rerun before merge.
AI-assisted review disclosure: Codex reviewed exact head 7da2d17, the concurrency/error paths, focused tests, current CI, and existing thread state.
中文说明
初始 discovery、显式 refresh 和 tools/list_changed 现在统一由 connection generation 的 single-flight 管理；同步重入、disconnect、失败后的 gate 释放和 notification burst 都有覆盖。当前 head 没有剩余 finding。现有 Desktop E2E 失败来自无关的 /side 菜单测试，合并前仍需重跑通过。
|
Before merging, Could we add the AI tool used in this PR so the merge commits would contain this information? |
|
Added the repository-template AI use section to the PR body, naming OpenAI Codex (Maka), its implementation/test/review scope, and the |
2 participants
Summary
Complete the generation-owned MCP Tool discovery transaction left as a follow-up to #1661.
tools/list_changedhandler before initial discoveryThis keeps discovery, callable bindings, and snapshot publication under one manager-owned transaction without adding another notification queue or public interface.
Refs #1650
Verification
npm test— all workspace tests passednpm --workspace @maka/mcp test— 67 passednpm run typechecknpm run lintnpm run format:checknpm run check:third-party-noticesReview focus
The key invariant is that a candidate can publish only while its connection generation and change epoch are still current. Tests cover a notification during initial discovery, a synchronous listener retiring the generation, a malformed output schema that fails only its own tool call before any wire round trip, and a refresh queued by a synchronous status listener during publication.
Checklist
Does this PR entail a change in behavior?
AI use
Tool(s) and scope: OpenAI Codex (Maka) contributed implementation, regression tests, review remediation, and verification under the author’s direction.
Final squash trailer:
Generated-by: Maka