fix(mcp): complete the initialize handshake instead of exiting on stdio (#211) - #217
Frankie-Xu wants to merge 2 commits into
Conversation
🌱 graft blast radius3 areas changed → 3 areas can be affected. 9 dependent symbols, depth 2. flowchart TB
A0(("Graph Management<br/>5 symbols"))
A1(("Graph Engine<br/>2 symbols"))
A2(("Pull Request Review<br/>2 symbols"))
classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
class A0,A1,A2 reached;
Who knows this code — 5 people across 6 areas
Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no All 9 dependent symbols, grouped by areaGraph Management — 5 symbols in 5 files
Graph Engine — 2 symbols in 1 file
Pull Request Review — 2 symbols in 2 files
Test signal per changed area — 1 ✓ · 1 ⚠ · 1 –Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.
38 test suites also reference this code46 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.
Open the interactive graph → — click an area to see the code that changed, and the line that reaches it. |
…io (trailhq#211) Closes trailhq#211 trailhq#212 — same root cause on both spec revisions: the stdio server loaded native grammars at boot (or returned before stdin was armed), so official SDK clients saw Connection closed / timeout instead of an initialize result. Co-authored-by: Cursor <cursoragent@cursor.com>
cfaf5c5 to
2b2f590
Compare
The handshake rebase changed rpc() to return {responses, exitCode, stderr}; listTools still treated the return as an array.
|
Rebased onto current main. Handshake is completed before native grammars load; tools/list still uses advertised() (including the parent-worktree case). CI is green — please review when you can. |
actionlint checks context availability, expression syntax, action inputs, and shellchecks `run:` blocks -- none of which YAML validation catches. Soup's CI found this the hard way (a job-level `env:` referencing the `runner` context, valid YAML, invalid Actions, dead before any job logged) and pins the release binary by SHA-256 rather than `go install`-ing it. Ported verbatim, credited. Also lands test/ratchet-lazy-grammar-import.test.ts: an AST-based repo-wide ratchet banning a static top-level import of any native tree-sitter grammar package (trailhq#323) -- the exact idiom trailhq#337 and trailhq#217 both moved away from. It runs as an ordinary test under ci.yml's existing `npm test`, no new job needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… depth language (trailhq#337 trailhq#217 trailhq#214 trailhq#325) Four PRs were rewriting the same code path in extract.ts — one function now does what they each asked for: - Lazy loading (trailhq#217): a grammar (and the core tree-sitter binding itself) is required at most once, the first time entryFor()/depthExtensions() is asked about that language, not for all nine at module-import time. `graft mcp` never asks before the client's `initialize` reply, so a client can no longer see that handshake stall behind nine native loads. - Per-language isolation (trailhq#337, unchanged): a grammar that will not load costs its own language, not the CLI. - WASM fallback for every depth language, not just Kotlin/Java (trailhq#214): the breadth tier already had a "FALLBACK row" mechanism (a GENERIC_LANGS row reachable only when the matching depth grammar failed); tree-sitter-wasm ships a .wasm for all nine depth languages already, so the remaining seven now have one too. None has a queries/<name>.scm, so on the rare machine that actually reaches one it degrades to the node-kind walker (symbols only) instead of leaving the language unindexed. - optionalDependencies (trailhq#325's core ask): the eight native grammar packages moved out of dependencies, so a platform lacking a prebuild for one no longer fails `npm install` for the other eight (core tree-sitter stays required — a missing core is a bigger question than one language, left for a follow-up). New test: `graft mcp` answers `initialize` with tree-sitter-typescript broken and never touches grammar loading at all (no warning on stderr) — the concrete claim trailhq#217 exists for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for digging into this! On current main, |
2 participants
Summary
graft mcpeither exited before answering or never wrote aninitializeresult, so the official SDK sawConnection closed/ timeout.initializeeven whenprotocolVersionis missing (default2025-11-25) or unsupported (JSON-RPC-32022with{ supported, requested }— not an echo of1999-01-01).tools/call.initialize/tools/list/server/discoveronly need the tool roster.Closes #211
Closes #212
Follow-up (still not this PR)
Pagination, resources, prompts, result-envelope fields (
resultType, cache hints on list results), andsubscriptions/listenwere not verified in those issues because the handshake never completed. They are out of scope here.After this lands they may start running in
@hasmcp/mcp-spec-testand fail for real capability gaps rather than “server exited”. Local run of2026-07-28against this branch already shows that: handshake / discover / SDK interop pass; 4 remaining failures are envelope fields ontools/listandtools/call(resultType,cacheScope,ttlMs,_metaserverInfo).Test plan
npm run build && npm test— 923/923initialize, unsupported version refused,tools/list,server/discoverwithout a sessiongraft mcp <tmpdir>: initialize → initialized → tools/list → tools/call; process still running on a quiet stdin pipe@hasmcp/mcp-spec-test@0.1.1 -c "node dist/cli.js mcp <abs>" --spec-version 2025-11-25: 14 passed, 0 failed (handshake, version negotiation, official SDK initialize +tools/list)--spec-version 2026-07-28: handshake,server/discover, and official SDK interop pass; 4 envelope failures left as follow-upMade with Cursor