Skip to content

land: lsp-exit-sweep - #614

Merged
ualtinok merged 2 commits into
mainfrom
land/lsp-exit-sweep
Oct 19, 2026
Merged

ualtinok merged 2 commits into
mainfrom
land/lsp-exit-sweep

Conversation

@aft-alfonso

@aft-alfonso aft-alfonso Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Accepted agent delivery; branch-keyed search descriptor included.


Summary by cubic

Fixes a race where the daemon's exit phase could report zero or too few language servers stopped because per-root background reap threads shut them down first.

  • Connection teardown now closes start admission but leaves running servers in their managers, so the exit phase stops them all under its single shared shutdown budget and reports the true count.
  • Route goodbyes that a process outlives (no exit phase behind them) keep the background reap behavior.
  • The drain test now requires the exit summary to count every server exactly, instead of at least three.

Written for commit 844e15e. Summary will update on new commits.

View guided diff Turn on auto-fix

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

The PR keeps running language servers in their managers when the SUBC connection ends. The exit phase then collects them under one shared shutdown budget.

  • The daemon’s exit sweep now owns every running language server.

Reviewed the code without running tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Route closes] --> B[Close start admission]
  B --> C[Take clients and stop them in background]
  D[SUBC connection ends] --> E[Close start admission and retain clients]
  E --> F[Module loop returns]
  F --> G[Collect clients from all actors]
  G --> H[Stop clients under one shared budget]
Loading

Reviews (1) · Last reviewed commit: "release notes and search-quality descrip..." · Reviewed by Greptile

ualtinok and others added 2 commits October 19, 2026 21:58
The drain tests read `lsp shutdown_all: servers=N` from the module's exit
phase. When the connection ends, the teardown used to quiesce every root
by detaching its clients to a per-root background reap thread. Each thread
waits 25 ms for Shutdown, then terminates its server. The exit phase then
found every manager empty (`phase=lsp_shutdown servers=0`) and counted
only the pids those threads had not reaped yet. If the main thread stalled
for about 100 ms after quiesce on a loaded runner, all servers were gone,
and the summary read `servers=0 ... elapsed_ms=0` (macOS integration
shard, main 396d0bf). The race dates from 83f7adb, the commit that
added the quiesce of root servers. #588 and #590 do not touch LSP
lifecycle.

Connection teardown now closes start admission but leaves the clients in
their managers (QuiescedLsp::LeaveForExitSweep). The exit phase in
run_subc_mode_inner takes them all under its one shared budget, as before.
A route goodbye during a session, and loop callers with no exit phase
behind them, keep the background reap (QuiescedLsp::ReapInBackground).

The drain test assertion goes from `servers >= 3` to exact equality: the
exit phase must take every root's server, and the summary must count all
of them. With ownership causal, the count no longer depends on timing.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
@ualtinok
ualtinok added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 5433db4 Oct 19, 2026
49 checks passed
jacobocasado pushed a commit to starredjaco/aft that referenced this pull request Oct 20, 2026
Rebase the release prep onto origin/main 3faaded and extend
.alfonso/release-notes/v0.61.0.md to cover the merges after cortexkit#590:
cortexkit#578, cortexkit#596, cortexkit#600, cortexkit#601, cortexkit#612, cortexkit#613, cortexkit#614, cortexkit#616, cortexkit#617, cortexkit#618, cortexkit#619, cortexkit#620,
cortexkit#622 and cortexkit#623. Fold and delete the 27 next/ fragments those merges added.
Credit cortexkit#465 (@null-axiom) and cortexkit#564 (@iceteaSA), and extend the cortexkit#577 line
with the shell timeout task-id fix.

Refresh ANNOUNCEMENT_FEATURES in both plugins. Pi leaves out the
OpenCode 2 permission line, which does not apply to Pi users.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant