Repository navigation
fix(pool): an [lsp] restart can no longer land on a superseded config - #701
Merged
Merged
Conversation
atlas-from-plumb
added a commit
that referenced
this pull request
Oct 16, 2026
Review of #701: when the change that deferred a round's wake disabled the language, nextRestart found nothing to restart (the entry was hibernated) and wakeHeld woke the disabled server anyway. wakeHeld now resolves the config and refuses a disabled language. Also: only a restart's first round carries a resync's reason (later rounds are config changes), wakeHeld logs its wake, and the langsGen comment documents its role as the reconfigure staleness token. Tests pin every wakeHeld guard, a single round waking at once, the config reason, and an invalid project file during a waiting round. Co-Authored-By: Claude Opus 5.5 <[email protected]>
atlas-from-plumb
added a commit
that referenced
this pull request
Oct 16, 2026
…gure does Re-review of #701: wakeHeld resolved the config outside p.mu, so a disable published in between could still be woken. It now re-resolves when the generation moved, exactly as reconfigureEntry does. Co-Authored-By: Claude Opus 5.5 <[email protected]>
PLAN-539's three MINOR windows, and PLAN-538's log wording: - reconfigureEntry checks the pool's config generation under p.mu after its unlocked resolve and re-resolves when it moved, so a stale want can no longer restart a server back onto an old config after a newer restart finished; - the effective config and the project file's validity come from one resolution (resolveLanguages / entryConfig) instead of two that could straddle an edit; - a restart round that ends with another already requested defers its wake to that round (wakeHeld wakes it at the end if none needs a restart), so a superseded config's server is never started; - a resync's restart is logged as a resync, not as a config change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Review of #701: when the change that deferred a round's wake disabled the language, nextRestart found nothing to restart (the entry was hibernated) and wakeHeld woke the disabled server anyway. wakeHeld now resolves the config and refuses a disabled language. Also: only a restart's first round carries a resync's reason (later rounds are config changes), wakeHeld logs its wake, and the langsGen comment documents its role as the reconfigure staleness token. Tests pin every wakeHeld guard, a single round waking at once, the config reason, and an invalid project file during a waiting round. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ile still parses Co-Authored-By: Claude Opus 5.5 <[email protected]>
…gure does Re-review of #701: wakeHeld resolved the config outside p.mu, so a disable published in between could still be woken. It now re-resolves when the generation moved, exactly as reconfigureEntry does. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
atlas-from-plumb
force-pushed
the
atlas/pool-reconfig-windows
branch
from
October 16, 2026 03:06
b8cfe6e to
16c50a5
Compare
golimpio
approved these changes
Oct 16, 2026
atlas-from-plumb
added a commit
that referenced
this pull request
Oct 16, 2026
Hooks stay inside their session's namespace (#698); context-hint uptake credits a hint's use on a connection its session alone declared (#699); the shared file watcher leaves linked git worktrees out (#700); an [lsp] restart can no longer land on a superseded config (#701). Plumb-Session: forest-ridge Co-authored-by: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The three MINOR windows PLAN-539's review left open, plus PLAN-538's log wording:
reconfigureEntryresolves its config outsidep.mu, so a reconfigure delayed past a newer config's whole restart could restart the server back onto the old config. It now reads the pool's config generation (langsGen, bumped after every publish) before resolving and re-checks it underp.mu; one that moved means a newer config was published, and it re-resolves. A publish that lands between the check and the restart is still caught by the again-flag, because every publish is followed by a reconfigure pass.entryConfig/resolveLanguagesreturn the effective config and the project file's validity from one resolution, replacingprojectConfigInvalid+cfgForWorkspace, which could straddle an edit.wakeHelddoes at the end when no further restart is needed.wakeHeldleaves an entry a newer restart owns.Verification
pool_reconfigure_window_test.go:wakeHeldleaves an entry a newer restart owns, with its control.TestWatcherLost_TheResyncSaysItIsOne.wakeHeldguard, two resolves, invalid ignored, resync wording), all KILLED. Existing reconfigure and resync suites pass under-race.🤖 Generated with Claude Code