Skip to content

LSP server capabilities are negotiated but never checked before dispatching tool requests #240

Description

@bug-ops

Description

LspServer::initialize() (crates/mcpls-core/src/lsp/lifecycle.rs) performs full LSP capability negotiation and stores the resulting ServerCapabilities on the LspServer (exposed via LspServer::capabilities()). However, this value is never read anywhere in the request-dispatch path.

Translator holds both lsp_clients: HashMap<ServerId, LspClient> (used to send requests) and lsp_servers: HashMap<ServerId, LspServer> (which owns the capabilities, held "for lifetime management" per its doc comment). Every one of the 20 handle_* tool methods in bridge/translator.rs (handle_rename, handle_code_actions, handle_call_hierarchy_prepare, handle_format_document, handle_signature_help, handle_inlay_hints, etc.) reaches its LSP client exclusively through get_client_for_file -> prepare_document, which resolves only by language routing (ToolRouter::resolve) and never consults lsp_servers/ServerCapabilities for the corresponding provider field (e.g. rename_provider, code_action_provider, document_formatting_provider, call_hierarchy_provider).

The only production call site of LspServer::capabilities() in the whole workspace is a single unit-test assertion (lifecycle.rs test), confirmed via rg -n "\.capabilities\(\)".

Practical effect: if a configured LSP server does not implement a given LSP method (common for lightweight/partial servers, or misconfigured mcpls.toml server routing), mcpls sends the request anyway. Whatever the server does with an unsupported method (JSON-RPC -32601 Method not found, a silent null, or a server-specific quirk) is translated as-is; in mcp/server.rs the error path for all tool handlers is a generic McpError::internal_error(e.to_string(), None). The AI-agent caller sees an opaque internal error rather than a clear, typed "this server does not support X" diagnostic mcpls could produce proactively from data it already collected during initialize.

Reproduction Steps

  1. Configure mcpls.toml to route a language to an LSP server binary that does not implement textDocument/rename (e.g. a minimal/diagnostics-only server, or serde_json-based test double as used in the existing test harness).
  2. Call the MCP rename_symbol tool (or code_actions, call_hierarchy_prepare, format_document, etc.) against a file routed to that server.
  3. Observe: the request is sent over the wire regardless of ServerCapabilities.rename_provider being None/absent from that server's initialize response.

Expected Behavior

Before dispatching a capability-gated LSP request, Translator should check the relevant ServerCapabilities field on the routed LspServer and, if the capability is absent, return a clear, typed error (e.g. a new Error::CapabilityNotSupported { server_id, capability } variant) instead of sending the request and opaquely relaying whatever the server returns.

Actual Behavior

Capability negotiation happens once at initialize and the result is stored but never consulted again; every capability-specific tool handler blindly sends its request regardless of what the server advertised.

Environment

  • Version: main @ 2c8ad7f
  • Features: default

Logs / Evidence

  • crates/mcpls-core/src/bridge/translator.rs:747 (prepare_document) and :675 (get_client_for_file) — no capability check.
  • crates/mcpls-core/src/lsp/lifecycle.rs:420-423 (LspServer::capabilities()) — only call site outside its own module is a test assertion.
  • crates/mcpls-core/src/mcp/server.rs:173-177 (rename_symbol) — representative example of the generic internal_error fallback that a capability check would let mcpls avoid.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Medium: suboptimal behavior, minor inconsistencybugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions