fix(vscode): stop the LSP restarting on unrelated configuration changes

[tripleaceme]

Description

Fixes #5920. Fixes #5642.

Both issues are the same bug: the extension restarted the language server far more often than it needed to, and let those restarts overlap. That produced the errors reported in both — Client got disposed and can't be restarted, and a stream of command 'sqlmesh.external_model_update_columns' already exists.

1. The LSP restarted on every configuration change in the editor

extension.ts subscribed to workspace.onDidChangeConfiguration and restarted unconditionally:

onDidChangeConfiguration(() => restartLsp()),

That event fires for any setting in the editor, including settings written by other extensions. It now restarts only when a section the server actually reads is affected — sqlmesh (which covers projectPaths and lspEntrypoint, both of which decide how the server is launched) or python.defaultInterpreterPath (the server runs inside that interpreter).

This accounts for the observations in #5642 that were otherwise hard to explain:

• Running any python command in a VS Code terminal killed the extension — the Python extension touches its own settings in response, and that alone triggered a restart.
• Running a copy of the same interpreter from a different path did not reproduce it — an unrecognised path makes the Python extension do nothing.
• su, uvx, a subshell or a notebook did not reproduce it either — none of those go through VS Code's shell integration, so no setting is written.

2. Restarts could overlap

LSPClient.restart() is stop() then start(). With a burst of triggers, several of those interleave: the outgoing client's command registrations collide with the incoming client's, and in-flight requests are aimed at a client that has already been disposed — exactly the two error messages in the reports.

Restarts are now serialized. Triggers arriving mid-restart collapse into a single rerun rather than queueing one restart each, since all a trigger needs is for a restart to have happened after it. An explicit restart (sqlmesh.restart, sign-in, format) is never downgraded to an automatic one when coalesced with one.

A related finding, deliberately not changed here

initializePython() in utilities/common/python.ts has no call sites, so onDidChangePythonInterpreterEvent is never fired and the onDidChangePythonInterpreter(() => restartLsp()) subscription in extension.ts is currently inert. Two consequences worth flagging:

• Changing the Python interpreter does not currently restart the server, so it keeps running under the old one.
• If that function is ever wired up as-is, it re-fires on every onDidChangeActiveEnvironmentPath without comparing against the path it last saw, which would reintroduce a restart storm.

I left it alone rather than widen this PR — happy to follow up if you'd like it wired up with a path comparison, or removed.

Test Plan

New unit tests, both in modules kept free of vscode imports so vitest can cover them in the test-vscode stage:

• configurationChange.test.ts — restarts for sqlmesh.projectPaths, sqlmesh.lspEntrypoint and python.defaultInterpreterPath; does not restart for editor.fontSize, workbench.colorTheme, files.autoSave, python.terminal.activateEnvironment, python.analysis.typeCheckingMode or terminal.integrated.env.linux. The last three are the regression tests for the terminal scenario in SQLMesh extension clashes with running sqlmesh in terminal #5642.
• coalesceAsync.test.ts — runs immediately when idle; four calls during a run collapse into exactly one rerun; every coalesced caller resolves; a later call still runs after a failure.

Ran both CI job commands locally:

$ pnpm run lint                                    # ui-style
exit 0

$ PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 pnpm run ci   # test-vscode
exit 0     # 18 unit tests pass (7 before this PR)

I have not reproduced the crash end to end in a live VS Code session — test-vscode-e2e is still disabled, and the trigger needs a real Python extension writing settings. The causal chain is established from the code path plus the reporter's process-of-elimination in #5642, and the unit tests pin the specific settings that must and must not cause a restart. Worth a sanity check from someone who can reproduce the original crash.

Checklist

• I have run make style and fixed any issues
• I have added tests for my changes (if applicable)
• All existing tests pass — ran the JS/TS suites via pnpm run ci; no Python changed, so make fast-test is unaffected by this PR
• My commits are signed off (git commit -s) per the DCO

cc @cmgoffena13 — picking this up per your note on #6054. Checked both issues for assignees and linked PRs beforehand; both were unassigned with nothing linked.

[tripleaceme]

@cmgoffena13 — this is the #5920 / #5642 one you pointed me at, ready for a look whenever you have time.

Both issues turned out to share a root cause: extension.ts calls restartLsp() on every onDidChangePythonInterpreter without checking whether the interpreter path actually changed, so ordinary terminal use triggers a restart storm.

It's still showing zero checks, since the workflow run needs maintainer approval.

[cmgoffena13]

@tripleaceme -- I am not that familiar with JavaScript so kind of going on good faith here. Ran this through AI and here's some feedback on a bug:

The settings filter looks right. handleError has to run after the coalesced restart finishes, not inside runRestart. On not_signed_in it awaits restartLsp() again, and that now waits on the restart that is still inside handleError, so sign-in never completes and the server stays stopped. Please cover that path with a test.

Seems like a successful sign in for Tobiko Cloud stalls it

[tripleaceme]

Thanks @cmgoffena13 — the bug is real, and it's fixed in 9e787b5. Good catch; a successful Tobiko Cloud sign-in would indeed have stalled with the server stopped.

The chain, confirmed by reproducing it:

1. restartLsp() → coalesceAsync records the run as in flight
2. the run awaits lspClient.restart(), which fails with not_signed_in
3. the run awaits handleError, which offers Sign In
4. signIn does await onSignInSuccess() — and that is restartLsp
5. that call sees a run already in flight, so it waits on it
6. the in-flight run is still waiting on the handler. Neither settles.

The fix: the run now records the error and returns, and the caller handles it after the run has finished. The restart triggered by signing in therefore starts a fresh run. The error is claimed by whichever caller reads it first, so callers coalesced into the same run don't each report the same failure.

One thing worth knowing about the test, because my first attempt was wrong in a way that looked fine.

The deadlock only happens if the run awaits something before the re-entrant call. running = start() invokes start(), which runs synchronously up to its first await — and the task is called inside that synchronous prefix, before the run has been recorded as in flight. So a re-entrant call that arrives synchronously sees an idle coalescer and starts a fresh run, no deadlock.

The real code has await lspClient.restart(...) before handleError, which is exactly what makes it reachable. My first test's simulated run had no such await, so it passed against both the old and the new arrangement — it proved nothing. The committed version awaits before reporting failure, and I checked it times out against the previous arrangement and passes against this one.

The test lives in coalesceAsync.test.ts rather than beside extension.ts, since the restart wiring is inside activate() and there's no vscode mock to drive it. It models the same shape and names the sign-in path, but if you'd rather see it exercised through the extension itself I'm happy to add the mocking for it.

pnpm run lint            ✓
pnpm run ci (extension)  4 files, 19 tests passed

[cmgoffena13]

@tripleaceme thanks, glad it was useful. One last thing: for new files, you need to add an SPDX license header per CONTRIBUTING.md ex. # SPDX-License-Identifier: Apache-2.0

[tripleaceme]

Done in 067d511. The four new files (coalesceAsync.ts, configurationChange.ts and their tests) now start with // SPDX-License-Identifier: Apache-2.0, the TypeScript form of the header in CONTRIBUTING.md. Lint, prettier and the 19 extension tests still pass.