Skip to content

per-request LSP timeouts are not configurable, despite timeout_seconds docs previously implying they were #267

Description

@bug-ops

Description

LspServerConfig::timeout_seconds (crates/mcpls-core/src/config/server.rs:173-175, default 30) is a documented, user-facing config knob. Its only consumer is the initialize handshake (lsp/lifecycle.rs:460) — it does not affect any of the per-request LSP round-trip timeouts in bridge::translator (DEFAULT_LSP_TIMEOUT = 30s, COMPLETIONS_LSP_TIMEOUT = 10s, both fixed constants as of #231/PR #264).

Prior to PR #264, docs/user-guide/configuration.md and docs/user-guide/troubleshooting.md incorrectly implied timeout_seconds also bounds individual tool-call requests (hover, completion, etc.), and told users experiencing per-request timeouts to raise timeout_seconds — which would not actually help. PR #264 corrected the documentation to accurately scope timeout_seconds to the initialize handshake only, but did not add configurability for per-request timeouts, since that was outside the scope of #231 (a pure refactor extracting a shared constant, not adding new config surface).

Issue #231's own "Why" section anticipated this gap: "makes the timeout policy discoverable and changeable in one place instead of requiring a repo-wide find-and-replace" — the natural next step is making that one place configurable.

Reproduction Steps

  1. Configure timeout_seconds = 120 for a slow LSP server.
  2. Trigger a slow textDocument/hover (or any other per-request tool) that takes longer than 30s (or 10s for completions) to respond, but less than 120s.
  3. Observe the request still times out at the fixed internal constant, ignoring the configured timeout_seconds.

Expected Behavior

Users should be able to configure per-request LSP timeouts (or timeout_seconds should transparently bound both the initialize handshake and per-request round trips) so that slow servers/large projects don't need a source change to avoid spurious tool-call timeouts.

Actual Behavior

Per-request timeouts are hardcoded to DEFAULT_LSP_TIMEOUT/COMPLETIONS_LSP_TIMEOUT in bridge::translator, independent of any user configuration.

Environment

Logs / Evidence

Found by the rust-critic agent during adversarial review of PR #264 (issue #231's fix). Two implementation directions to weigh: (a) thread timeout_seconds (or a new dedicated config field) through Translator's handler methods as a fallback over the constants, or (b) introduce a separate request_timeout_seconds config field distinct from the handshake timeout_seconds, to avoid conflating two semantically different timeouts under one knob.

Metadata

Metadata

Assignees

Labels

P3Low: cosmetic, edge case unlikely in practiceenhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions