Skip to content

fix: LSP handshake race condition, duplicate notification, and slow-start timeout#127

Open
marcelmaatkamp wants to merge 5 commits into
isaacphi:mainfrom
marcelmaatkamp:main
Open

fix: LSP handshake race condition, duplicate notification, and slow-start timeout#127
marcelmaatkamp wants to merge 5 commits into
isaacphi:mainfrom
marcelmaatkamp:main

Conversation

@marcelmaatkamp
Copy link
Copy Markdown

Summary

Three related fixes found while integrating the Kotlin LSP (fwcd/kotlin-language-server). All three affect any LSP that sends server-initiated requests during the handshake or takes a long time to start.

Fix 1: Register handlers before initialize (race condition)

The LSP spec allows servers to send requests like workspace/configuration at any point after receiving initialize — including before the client sends initialized. The previous code registered handlers after the initialize call returned, so these requests arrived with no handler and got a method not found response. This put the server in a bad state for all subsequent requests.

Fix: Move handler registration to before the initialize call.

Fix 2: Remove duplicate initialized notification

initialized was being sent twice: once via an explicit Notify call and once via c.Initialized(). Strict LSP servers (like Kotlin) treat this as a protocol error, causing all subsequent requests to fail with nonsensical errors (kotlin.Nothing does not have instances).

Fix: Remove the duplicate Notify call, keep only c.Initialized().

Fix 3: Lazy LSP initialization to avoid MCP timeout

ServeStdio() was only called after InitializeLSPClient() completed. For LSPs with slow startup (Kotlin triggers a Gradle sync, ~95s on first run), this exceeded the MCP client's 120s timeout before the server was even reachable.

Fix: Initialize the LSP in a background goroutine so ServeStdio() starts immediately. Tool handlers block via waitForLSP() until the LSP is ready, making the timeout apply per tool call rather than at startup.

Test plan

  • Tested with fwcd/kotlin-language-server v1.3.13 against a real Gradle project
  • hover, definition, references, and diagnostics tools all return correct results
  • MCP server responds to initialize and tools/list immediately, before LSP is ready
  • Tool calls block and succeed once LSP finishes initializing

🤖 Generated with Claude Code

Marcel Maatkamp and others added 5 commits April 6, 2026 10:55
Initialize the LSP process in a background goroutine so ServeStdio
starts immediately. Tool handlers block via waitForLSP() until the
LSP is ready. Fixes timeout with slow-starting LSPs like Kotlin
(~95s Gradle sync on first run).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
initialized was sent twice: once via Notify before handler registration
and once via c.Initialized() after. The duplicate confused Kotlin LSP,
causing 'kotlin.Nothing' does not have instances errors on all requests.
Handler registration is now done before the single initialized call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
workspace/configuration and other server requests can arrive at any time
during the initialize handshake. Registering handlers after initialize
caused 'method not found' responses, putting Kotlin LSP in a bad state
which produced 'kotlin.Nothing' does not have instances errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
STRd6 pushed a commit to STRd6/mcp-language-server that referenced this pull request May 14, 2026
Three race fixes from upstream PR isaacphi#127 (marcelmaatkamp), found while
integrating the Kotlin LSP. Applied as one commit because they all
have to land together for slow servers to work.

1. Register server-request handlers before sending initialize.
   workspace/configuration, client/registerCapability, etc. can
   arrive at any point after initialize -- including before our
   initialized notification. Registering after the call left a race
   window where strict servers (Kotlin, async-lsp-based Rust LSPs)
   saw method-not-found and refused all subsequent requests.

2. Drop the duplicate initialized notification. The handshake was
   firing it twice (raw Notify plus c.Initialized), which most LSPs
   tolerate but Kotlin treats as a protocol error. Supersedes
   upstream PR isaacphi#49.

3. Initialize LSP in a background goroutine so ServeStdio starts
   immediately. Tool handlers gate via mcpServer.waitForLSP, which
   blocks per-call rather than stalling the whole MCP handshake.
   Fixes 120s MCP-client timeout against LSPs with slow startup
   (Kotlin/Gradle ~95s on cold runs, large rust-analyzer indexes).

The lazy-init adaptation diverges slightly from upstream because
our fork registers tools based on advertised LSP capabilities.
Always-on tools (edit_file, diagnostics) register before
ServeStdio so tools/list responds immediately; capability-gated
tools register from the background goroutine after the handshake
completes, and mcp-go emits tools/list_changed so connected
clients pick them up live. Every tool handler is wrapped with
addTool, which routes through waitForLSP, so even calls to
always-on tools that arrive during startup block instead of
running against a nil LSP client.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6 pushed a commit to STRd6/mcp-language-server that referenced this pull request May 14, 2026
Two CI failures on 54ec662, both reproducible only against gopls
(the local test pass before push missed them because gopls was not
installed):

- tools.go:19 was an infinite recursive call in mcpServer.addTool.
  The bulk rename of s.mcpServer.AddTool -> s.addTool in 54ec662
  rewrote the call inside the helper itself, so every tool invocation
  recursed into the gate. staticcheck SA5007 caught it; reverted that
  one call back to s.mcpServer.AddTool.

- edit_file.go was constructing a WorkspaceEdit key via
  protocol.DocumentUri(filePath) -- raw cast, no scheme. The PR isaacphi#82
  edit_file.go ApplyTextEdits adaptation in utilities/edit.go is now
  uri.Path() instead of TrimPrefix("file://"), and Path() panics on a
  non-file URI. Fix the construction site (use URIFromPath) and make
  utilities/edit.go defensive via a uriToPath helper that falls back
  to the raw string when the prefix is missing, so an analogous bug
  surfaces as a file-not-found error rather than a runtime panic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant