fix(mcp): survive non-standard client probes before initialize (#109) - #110
Merged
Conversation
Copilot CLI >= 1.0.79 sends a vendor-specific `server/discover` request before the standard `initialize` handshake. rmcp's server handshake accepts only `ping` before `initialize` and aborts on anything else with `ExpectedInitializeRequest`, so the process exited 1 and the client never got to send `initialize` at all. Answer such probes with JSON-RPC `-32601 Method not found` before rmcp is handed the stream. Per github/copilot-cli#4370 Copilot tolerates -32601 and then proceeds with the normal lifecycle (it is the `-32602` that FastMCP returns which it treats as fatal). The filter runs once, only until the first non-probe message, and hands that message back to rmcp byte-for-byte. `initialize`, `ping` and `notifications/*` are never intercepted, so rmcp keeps full ownership of the lifecycle and clients that do not probe see an unchanged byte stream. Also fix the updater picking the wrong release asset (same issue report): `target_arch == "aarch64"` was tested first, so Windows and Linux arm64 hosts downloaded the macOS binary ("%1 is not a valid Win32 application"). Match on target_os first, use the existing windows-x64 asset, and error clearly on platforms with no prebuilt binary. While there, three latent Windows breakages in the same path: the temp download needs a `.exe` extension to be executable for the verification step, `with_extension("old")` turned `graphmind.exe` into `graphmind.old` and lost the extension on rollback, and a stale backup blocked the rename.
CI runs `dtolnay/rust-toolchain@stable` unpinned, and stable has since moved to rustc 1.98 / clippy 0.1.90, which added `clippy::chunks_exact_to_as_chunks`. `bytes_to_float32` tripped it, so `rust-check` failed with -D warnings and gated every downstream job. This is independent of the #109 fix — the line is unchanged since before that branch and main is red for the same reason. Use `as_chunks::<4>()` (stable since 1.88) as clippy suggests: it yields fixed-size arrays, so `from_le_bytes` needs no per-element indexing. Behaviour is unchanged, trailing-byte truncation included — verified identical to the old implementation for all buffer lengths 0..=32, for MIN/MAX/EPSILON/±0.0/±INFINITY, and for NaN bit patterns.
This was referenced Sep 2, 2026
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
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.
Fixes #109.
Two separate bugs were reported in that issue. Both are fixed here.
1. MCP server dies on Copilot CLI's pre-initialize probe
Copilot CLI >= 1.0.79 sends a vendor-specific
server/discoverrequest before the standardinitializehandshake.rmcp's server handshake (
service/server.rs) permits onlypingbeforeinitialize; anything else falls through to theClientRequest::InitializeRequestguard and aborts withExpectedInitializeRequest. The process exits 1, so the client never gets to sendinitializeat all.Reproduced against the released
v0.2.214binary:rmcp exposes no option to relax this, so
handshake.rsanswers such probes with JSON-RPC-32601 Method not foundbefore rmcp is handed the stream. Per github/copilot-cli#4370 Copilot tolerates-32601and proceeds with the normal lifecycle — it is the-32602that FastMCP returns which it treats as fatal.Why this has no side effects
initialize,pingandnotifications/*are never intercepted, so rmcp keeps full ownership of the lifecycle (including its own pre-initpinghandling).prepend), with no reparse or reserialization, so a client that does not probe sees exactly the stream it saw before.id) are dropped without a reply, per JSON-RPC.Verified end-to-end, probe vs no-probe, identical results apart from the added
-32601:initializetools/listtools/call gm_statusisError=falseisError=false2. Updater downloaded the macOS binary on Windows
Also reported in #109 (
%1 is geen geldige Win32-toepassing).download_and_replacetestedtarget_arch == "aarch64"first, so Windows arm64 gotmacos-arm64, and Windows x64 fell through to themacos-x64fallback. Thegraphmind-cli-windows-x64.exeasset already exists in releases — the updater just never selected it.Now matches on
target_osfirst, and errors clearly (pointing atcargo install) where no prebuilt binary exists instead of silently downloading the wrong one.While in that function, three latent Windows breakages on the same path:
.exeextension, or the--versionverification step cannot execute it at all;with_extension("old")turnedgraphmind.exeintographmind.old, losing the extension on rollback;.oldfrom a previous update blocked the rename (Windows refuses to rename onto an existing path).Also added an
EXDEVcopy fallback, sincetemp_dirand the install dir can be on different volumes.Tests
9 new regression tests in
crates/graphmind-mcp/tests/preinit_handshake.rs, including the exact Copilot 1.0.81 sequence, the ping-forwarding contract, and byte-exactness of the replay.cargo clippy --workspace --all-targets -- -D warnings— cleancargo test -p graphmind-mcp -p graphmind-cli— 20 suites, 0 failures, 4 consecutive runsNote
cargo test --workspacehas one pre-existing intermittent failure,graphmind-desktopsettings::startup_settings_tests::set_build_all_on_startup_persists(aHOMEenv-var race between tests). It reproduces at the same rate with this branch's changes reverted (1/3 baseline vs 1/3 with changes) and is unrelated to this fix — worth a separate issue.Addendum: unblocking CI
rust-checkwas failing on this PR — and onmain— for a reason unrelated to #109.CI runs
dtolnay/rust-toolchain@stableunpinned, and stable has since moved to rustc 1.98 / clippy 0.1.90, which addedclippy::chunks_exact_to_as_chunks. That lint fires ongraphmind-embeddings/src/store.rs:150(bytes_to_float32) — a line untouched by this branch — sorust-checkfailed under-D warningsand gated every downstream job (rust-build,e2e-cli,e2e-setup,desktop-buildall reported SKIPPED).mainhas been red for this since 2026-07-30, so any PR opened today hits it. Fixed here in a separate commit so it can be reviewed or split off independently.Worth noting for reproduction: local clippy passed because the
rustcon my PATH was older (1.90) and simply did not have the lint.rustup update stable→ rustc 1.98 reproduced the CI failure exactly, and the fix was verified against CI's literal command.as_chunks::<4>()is stable since 1.88. Behaviour is unchanged including trailing-byte truncation — verified identical to the old implementation across all buffer lengths 0..=32,MIN/MAX/EPSILON/±0.0/±INFINITY, and NaN bit patterns.Given the repo declares no MSRV and CI floats on
@stable, pinning the toolchain (arust-toolchain.toml, or a pinned version in the workflow) would stop this class of breakage recurring. Out of scope here.