Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: minor (1-2), nit (3-4) — VS Code LSP client lifecycle robustness (editors/vscode/src/lsp/client.ts).
- Config-change decision samples
running outside the serialized queue (client.ts:105-121): handleLspConfigChange computes the action at event time using isLspRunning(), which only reflects completed queue operations (client is assigned only after candidate.start() resolves). Scenario: a path change enqueues restart; while doStart is awaiting start(), the user disables inference.lsp.enabled — the disable event sees running == false, decides no stop is needed, and the server ends up running against enabled: false. Decide the action inside the queued operation (or re-check state when the queued op runs).
- A spawned-but-unresponsive server wedges the queue forever (
client.ts:173): doStart awaits candidate.start() with no timeout; vscode-languageclient v9's start() awaits the initialize response with no timeout either. A binary that spawns, holds stdin open, and never answers initialize permanently blocks every subsequent lifecycle operation (restart command included) with no user-visible signal. Add a timeout + error surface.
inference-lsp.trace.server is not contributed in package.json (package.json:151), so the standard vscode-languageclient protocol-trace knob is undiscoverable and flags as an unknown setting.documentSelector is [{ scheme: 'file', language: 'inference' }] (client.ts:162): untitled .inf buffers get no language features, which contradicts the bundled walkthrough's "Create New File" step. Consider adding scheme: 'untitled' (the server's URI layer currently ignores non-file URIs gracefully — see the e2e "non_file_uri" tests — so this also needs the server to accept untitled docs or the selector decision documented).
Related: the promise-queue race-freedom invariant has zero tests — tracked in the test-coverage follow-up issue.
Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: minor (1-2), nit (3-4) — VS Code LSP client lifecycle robustness (
editors/vscode/src/lsp/client.ts).runningoutside the serialized queue (client.ts:105-121):handleLspConfigChangecomputes the action at event time usingisLspRunning(), which only reflects completed queue operations (clientis assigned only aftercandidate.start()resolves). Scenario: a path change enqueues restart; while doStart is awaitingstart(), the user disablesinference.lsp.enabled— the disable event seesrunning == false, decides no stop is needed, and the server ends up running againstenabled: false. Decide the action inside the queued operation (or re-check state when the queued op runs).client.ts:173):doStartawaitscandidate.start()with no timeout; vscode-languageclient v9'sstart()awaits the initialize response with no timeout either. A binary that spawns, holds stdin open, and never answers initialize permanently blocks every subsequent lifecycle operation (restart command included) with no user-visible signal. Add a timeout + error surface.inference-lsp.trace.serveris not contributed in package.json (package.json:151), so the standard vscode-languageclient protocol-trace knob is undiscoverable and flags as an unknown setting.documentSelectoris[{ scheme: 'file', language: 'inference' }](client.ts:162): untitled.infbuffers get no language features, which contradicts the bundled walkthrough's "Create New File" step. Consider addingscheme: 'untitled'(the server's URI layer currently ignores non-file URIs gracefully — see the e2e "non_file_uri" tests — so this also needs the server to accept untitled docs or the selector decision documented).Related: the promise-queue race-freedom invariant has zero tests — tracked in the test-coverage follow-up issue.