feat(lsp): idle-timeout shutdown for LSP servers by Tsonov · Pull Request #809 · getkimchi/kimchi

Tsonov · 2026-07-07T11:37:23Z

Linked issue

Closes #808

What does this PR do?

Introduces a background sweeper for idle servers. If a server is idle for too long (15 minute hard threshold now), it is stopped and will have to be started again. This is a tradeoff between latency to restart a server sometimes (and let it do its processing) vs idle resource usage.

How it works

A 60-second setInterval sweep checks each live LSP client against three conditions:

No pending requests in flight
No active progress tokens (project loading/indexing)
lastActivity older than 15 minutes

Idle clients get a best-effort LSP shutdown request followed by proc.kill(). Map entries are deleted synchronously before kill so getOrCreateClient never returns a dying client — JS single-threading guarantees no interleaving with the
synchronous cache-hit path.

The next LSP tool call transparently spawns a fresh server.

Changes

src/extensions/lsp/client.ts — shutdownIdleClientsIn() (testable, injectable maps + timestamp) and shutdownIdleClients() (thin wrapper over module-level state). Per-client shutdown body is guarded with try/catch so an already-dead
process can't crash the interval callback.
src/extensions/lsp.ts — Starts the sweep interval on session_start (when servers are detected), clears it on session_shutdown. Timer clear runs before the no-servers early return to prevent leaking a prior session's interval.
src/extensions/lsp/client.test.ts — 10 tests covering: idle vs active threshold, pending requests, progress tokens, lock map cleanup, graceful shutdown ordering, multi-client selective sweep, empty map, mid-initialize preservation,
and proc.kill throwing.
src/extensions/lsp.test.ts — 6 wiring tests: interval start, threshold value, no-servers skip, shutdown cleanup, duplicate-start prevention, and stale-sweep-after-no-servers regression.

Checklist

[ x] I have read CONTRIBUTING.md and agree to the CLA
[ x] This PR links to an open issue above
[ x] Tests pass locally (pnpm run test) (some tests seem to fail but they are in files unrelated to change)
[ x] Lint passes (pnpm run check)
[ x] Documentation updated if behavior changed -> not needed?

Shut down gopls/tsserver subprocesses after 15 minutes of inactivity, transparently restart on next use via getOrCreateClient cache-miss path. Implementation: - shutdownIdleClientsIn(): pure function that sweeps a client map, skipping clients with pending requests or active progress tokens. Fully synchronous check-delete-kill — no event-loop interleaving with getOrCreateClient's synchronous cache-hit path. - shutdownIdleClients(): thin wrapper passing module-level maps. - 60s setInterval sweep wired in session_start, cleared on shutdown. Duplicate-safe (clears previous timer before starting new one). Tests: 8 unit tests for shutdownIdleClientsIn (threshold, guards, ordering, multi-client, empty map) + 5 extension tests for timer lifecycle (start, threshold value, no-servers, clear, no-duplicate).

readme-ai-writer · 2026-07-07T11:37:27Z

📚 No documentation changes were needed for this PR.

kimchi-review · 2026-07-07T11:37:31Z

Kimchi Code Review

Property	Value
Commit	`89468e4`
Author	@Tsonov
Files changed	0
Review status	Completed
Comments	1 (1 info)
Duration	121s

Summary

📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 3/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — Comprehensive tests cover the extension-level timer lifecycle (start, stop, duplicate prevention, no-servers cleanup, threshold passing) and the client sweep logic (idle detection, pending request/progress token preservation, lock map cleanup, shutdown-before-kill ordering, error resilience, and initialize-request protection).

📝 Found 1 issue(s). See inline comments for details.

What to expect

Kimchi will analyze the changes in this pull request and post:

A summary of the overall changes
Inline comments on specific lines with findings categorized by issue type

The review typically completes within a few minutes. This comment will be updated once the review is ready.

Interact with Kimchi

@getkimchi review — re-trigger a full review on the latest commit
@getkimchi summary — regenerate the PR summary
@getkimchi ignore — skip this PR (no review will be posted)
Reply to any inline comment to ask follow-up questions or request clarification

Configuration

Reviews are configured by your organization admin.
Review instructions, excluded directories, and severity thresholds can be adjusted per repository in the Kimchi dashboard.

Powered by Kimchi — AI-powered code review by CAST AI

kimchi-review

📊 Review Score: 86/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — The PR adds targeted tests for both the idle sweep timer lifecycle in src/extensions/lsp.test.ts and the idle-shutdown logic in the new src/extensions/lsp/client.test.ts. Coverage includes threshold handling, pending/progress-token protection, shutdown request ordering, duplicate timer prevention, and cleanup on shutdown.

📝 Found 4 issue(s). See inline comments for details.

Tsonov · 2026-07-07T12:09:45Z

@getkimchi review

kimchi-review · 2026-07-07T12:09:50Z

🔄 Starting review on 56b0fe4…
Triggered by @Tsonov via the command.

kimchi-review

📊 Review Score: 88/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — Good coverage for the new behavior. src/extensions/lsp.test.ts verifies timer lifecycle (start, threshold passing, no-servers skip, shutdown cleanup, duplicate prevention, and sweep clearing). src/extensions/lsp/client.test.ts adds thorough unit tests for shutdownIdleClientsIn including idle/active clients, pending requests, progress tokens, lock-map cleanup, mid-initialize protection, ordering of shutdown write vs kill, and kill errors.

📝 Found 2 issue(s). See inline comments for details.

Tsonov · 2026-07-07T12:26:45Z

@getkimchi review

kimchi-review · 2026-07-07T12:26:47Z

🔄 Starting review on 89468e4…
Triggered by @Tsonov via the command.

kimchi-review

📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 3/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — Comprehensive tests cover the extension-level timer lifecycle (start, stop, duplicate prevention, no-servers cleanup, threshold passing) and the client sweep logic (idle detection, pending request/progress token preservation, lock map cleanup, shutdown-before-kill ordering, error resilience, and initialize-request protection).

📝 Found 1 issue(s). See inline comments for details.

- Move timer clear above the no-servers early return in session_start so a prior session's interval doesn't leak when the next session detects no LSP servers. - Wrap sendRequest/proc.kill in try/catch inside shutdownIdleClientsIn to prevent an uncaughtException in the setInterval callback if the process is already dead. - Add regression tests: stale sweep on no-servers restart, mid-initialize client preserved, proc.kill throwing on dead process. Co-Authored-By: Kimchi <noreply@kimchi.dev>

vmizg · 2026-07-10T06:32:42Z

 	// controller is combined with ctx.signal so user/session aborts also unwind
 	// the wait, but we never abort ctx.signal ourselves.
 	let pendingRefresh: { abort: AbortController } | undefined
+	let idleSweepTimer: ReturnType<typeof setInterval> | null = null


There is a nodejs.Timeout type that can be used for this

vmizg · 2026-07-10T06:33:32Z

+		if (typeof idleSweepTimer === "object" && "unref" in idleSweepTimer) {
+			idleSweepTimer.unref()


No need for these additional checks, this is synchronous execution, timeout object will always be available at this point

Suggested change

if (typeof idleSweepTimer === "object" && "unref" in idleSweepTimer) {

idleSweepTimer.unref()

idleSweepTimer.unref()

vmizg · 2026-07-10T06:35:15Z

 }

+/**
+ * Shut down LSP clients that have been idle for longer than `thresholdMs`.


I wonder if the same could be done for regular commands? I've had vitest freezing up the client sometimes

If it works well, it is possible

@vmizg

Address PR feedback from @vmizg — use NodeJS.Timeout instead of ReturnType<typeof setInterval>. This also lets us drop the runtime guard around .unref() since the method is declared on the type.

Merge origin/master into feat/lsp-idle-shutdown. Conflict was in the session_start handler of src/extensions/lsp.ts: - Master added degraded-state detection (detectMissingCandidates, warned flag, before_agent_start one-time warning) and restructured the status-bar logic. - Our branch added the idle sweep timer (start on session_start, clear on session_shutdown). Resolution: kept both features. The idle sweep timer start was moved after the activeServers.length === 0 early return so it only starts when servers are actually active. Timer clearing stays before the early return to prevent leaks from prior sessions.

Tsonov · 2026-07-13T07:09:14Z

@vmizg updated from code review + resolved merge conflicts, PTAL

kimchi-review Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/extensions/lsp/client.ts Outdated

Comment thread src/extensions/lsp/client.test.ts

Comment thread src/extensions/lsp.ts

Comment thread src/extensions/lsp/client.ts

Tsonov force-pushed the feat/lsp-idle-shutdown branch from b7f071d to 56b0fe4 Compare July 7, 2026 12:03

kimchi-review Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/extensions/lsp/client.ts

Comment thread src/extensions/lsp.ts

Tsonov force-pushed the feat/lsp-idle-shutdown branch from 56b0fe4 to 89468e4 Compare July 7, 2026 12:20

kimchi-review Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/extensions/lsp/client.ts

Tsonov force-pushed the feat/lsp-idle-shutdown branch from 89468e4 to afcbc6f Compare July 7, 2026 12:38

vmizg reviewed Jul 10, 2026

View reviewed changes

vmizg changed the title ~~Feat: Stop gopls if idle for too long~~ feat(lsp): idle-timeout shutdown for LSP servers Jul 10, 2026

Tsonov added 2 commits July 10, 2026 15:54

refactor(lsp): use NodeJS.Timeout type for idle sweep timer

e84ca6d

Address PR feedback from @vmizg — use NodeJS.Timeout instead of ReturnType<typeof setInterval>. This also lets us drop the runtime guard around .unref() since the method is declared on the type.

Ivaka approved these changes Jul 30, 2026

View reviewed changes

		if (typeof idleSweepTimer === "object" && "unref" in idleSweepTimer) {
		idleSweepTimer.unref()

Uh oh!

Conversation

Tsonov commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked issue

What does this PR do?

How it works

Changes

Checklist

Uh oh!

readme-ai-writer Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kimchi-review Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Kimchi Code Review

Summary

Uh oh!

kimchi-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tsonov commented Jul 7, 2026

Uh oh!

kimchi-review Bot commented Jul 7, 2026

Uh oh!

kimchi-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Tsonov commented Jul 7, 2026

Uh oh!

kimchi-review Bot commented Jul 7, 2026

Uh oh!

kimchi-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vmizg Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Tsonov Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

vmizg Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Tsonov Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

vmizg Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Tsonov Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Tsonov commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Tsonov commented Jul 7, 2026 •

edited

Loading

readme-ai-writer Bot commented Jul 7, 2026 •

edited

Loading

kimchi-review Bot commented Jul 7, 2026 •

edited

Loading