diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f5d9b4..bb158af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,15 @@ example a hook-less desktop chat sharing a connector. Its calls can then be credited to the session that did declare, so the report remains a proxy. +- **An `[lsp]` restart can no longer land on a superseded config.** A + reconfigure that resolved its config just before a newer one was published + re-resolves instead of restarting the server back onto the old one, the + config and the project file's validity now come from one resolution, and a + restart round that already has a newer change waiting no longer wakes its + server for the config about to be replaced (costly for a server such as + jdtls). A resync after lost file events is now logged as one, not as a config + change. + - **The file watcher leaves linked git worktrees out.** The OS watcher a workspace's topology index and language servers share now drops every event inside a linked worktree of the workspace's repository (or of one of its diff --git a/internal/cli/pool.go b/internal/cli/pool.go index dbaf0e94..aad22ea3 100644 --- a/internal/cli/pool.go +++ b/internal/cli/pool.go @@ -63,8 +63,11 @@ type workspacePool struct { langs []langConfig langsMu sync.RWMutex - // langsGen counts the times enableLanguage has widened the effective set. It - // is the signal a live connection watches to know its primary language may be + // langsGen is the pool's language-config generation, bumped AFTER every + // publish of a new effective set (enableLanguage, applyGlobalLSP, + // invalidateLanguageConfig). reconfigureEntry reads it before its unlocked + // resolve and re-checks it under p.mu, so a resolve that straddled a publish + // is repeated rather than acted on. It is also the signal a live connection watches to know its primary language may be // resolvable now when it was not at attach time: `enable-lsp` deliberately // leaves existing pool entries and pinned workspaces untouched, so without // this a session that attached as LanguageNone would report "none attached" @@ -89,6 +92,10 @@ type workspacePool struct { // the cost, since the walk and the stats run on a hit as well. The guard would // then pass while the regression it exists to catch was back. langResolves atomic.Uint64 + // reconfigureResolved, when set, runs in reconfigureEntry and wakeHeld + // between the unlocked resolve and the locked decision: a test seam for the + // window the config generation check closes. Nil in production. + reconfigureResolved func(*poolEntry) baseConfig config.Config // global base for per-workspace LSP overrides languageConfig languageConfigState diff --git a/internal/cli/pool_config.go b/internal/cli/pool_config.go index 5ce6a528..51253c33 100644 --- a/internal/cli/pool_config.go +++ b/internal/cli/pool_config.go @@ -14,3 +14,12 @@ func (p *workspacePool) cfgFor(language string) (config.LSPConfig, bool) { func (p *workspacePool) cfgForWorkspace(root, language string) (config.LSPConfig, bool) { return cfgAmong(p.effectiveLanguages(root), language) } + +// entryConfig is e's effective config, whether its language is enabled, and +// whether the project config governing it fails to parse — all from one +// resolution (resolveLanguages). +func (p *workspacePool) entryConfig(e *poolEntry) (cfg config.LSPConfig, enabled, invalid bool) { + langs, invalid := p.resolveLanguages(e.root) + cfg, enabled = cfgAmong(langs, e.language) + return cfg, enabled, invalid +} diff --git a/internal/cli/pool_languages.go b/internal/cli/pool_languages.go index c7efc3a6..64b0078e 100644 --- a/internal/cli/pool_languages.go +++ b/internal/cli/pool_languages.go @@ -62,7 +62,7 @@ type cachedWorkspaceLanguages struct { stamp languageConfigStamp langs []langConfig // invalid marks a project config that did not parse, so langs is the - // global fallback (see projectConfigInvalid). + // global fallback (see resolveLanguages). invalid bool } @@ -197,17 +197,26 @@ func languageBoundaryKnownHome(dir string, atHome bool) bool { // LoadProject would fail validation. LogLevel is the sentinel for that, matching // what cfgForWorkspace used before this became a shared resolver. func (p *workspacePool) effectiveLanguages(root string) []langConfig { + langs, _ := p.resolveLanguages(root) + return langs +} + +// resolveLanguages is effectiveLanguages plus whether the project config +// governing root fails to parse (the languages are then the global fallback), +// both from ONE resolution: asking twice could straddle an edit to the file and +// pair one state's languages with the other's verdict (PLAN-539 follow-up). +func (p *workspacePool) resolveLanguages(root string) ([]langConfig, bool) { p.langResolves.Add(1) // LogLevel is written once at construction and never mutated, so it needs no // lock; the maps and slices behind it do (see langsSnapshot). Safe only while // nothing assigns p.baseConfig WHOLE — .LSP is replaced field-wise by // enableLanguage for exactly this reason. if p.baseConfig.LogLevel == "" { - return p.langsSnapshot() + return p.langsSnapshot(), false } policyRoot := languagePolicyRoot(root) if policyRoot == "" { - return p.langsSnapshot() + return p.langsSnapshot(), false } stamp := projectLanguageStamp(policyRoot) @@ -215,13 +224,13 @@ func (p *workspacePool) effectiveLanguages(root string) []langConfig { cached, ok := p.languageConfig.cache[policyRoot] p.languageConfig.mu.RUnlock() if ok && cached.stamp == stamp { - return cached.langs + return cached.langs, cached.invalid } p.languageConfig.mu.Lock() defer p.languageConfig.mu.Unlock() if cached, ok = p.languageConfig.cache[policyRoot]; ok && cached.stamp == stamp { - return cached.langs + return cached.langs, cached.invalid } langs, err := p.resolveProjectLanguages(policyRoot) invalid := err != nil @@ -245,22 +254,7 @@ func (p *workspacePool) effectiveLanguages(root string) []langConfig { p.languageConfig.cache = make(map[string]cachedWorkspaceLanguages) } p.languageConfig.cache[policyRoot] = resolved - return resolved.langs -} - -// projectConfigInvalid reports whether the project config governing root's -// languages fails to parse — the pool is then running on the global fallback, -// which is a degradation rather than an edit, so a reconfigure must not act on -// it (PLAN-539). Resolving first keeps the answer current for the file's stamp. -func (p *workspacePool) projectConfigInvalid(root string) bool { - p.effectiveLanguages(root) - policyRoot := languagePolicyRoot(root) - if policyRoot == "" { - return false - } - p.languageConfig.mu.RLock() - defer p.languageConfig.mu.RUnlock() - return p.languageConfig.cache[policyRoot].invalid + return resolved.langs, resolved.invalid } // resolveProjectLanguages merges the project config at policyRoot onto the base diff --git a/internal/cli/pool_reconfigure.go b/internal/cli/pool_reconfigure.go index 91ed1ac8..befafe7b 100644 --- a/internal/cli/pool_reconfigure.go +++ b/internal/cli/pool_reconfigure.go @@ -116,13 +116,36 @@ func (p *workspacePool) reconfigureEntries(selected func(*poolEntry) bool) int { // instead of finishing on the one it started with. An entry governed by a // project config that does not parse is left as it is until the file is fixed // — the pool's fallback to the global config is a degradation, not an edit. +// +// The config is resolved outside p.mu, so it is checked against the pool's +// config generation under the lock: one that moved means a newer config was +// published while this one resolved, and the resolve is repeated rather than +// acting on a stale want — which could otherwise restart a server back onto +// an old config just after a newer restart finished. func (p *workspacePool) reconfigureEntry(e *poolEntry) bool { - if p.projectConfigInvalid(e.root) { - return false + for { + gen := p.langsGen.Load() + want, enabled, invalid := p.entryConfig(e) + if invalid { + return false + } + if p.reconfigureResolved != nil { + p.reconfigureResolved(e) + } + p.mu.Lock() + if p.langsGen.Load() != gen { + p.mu.Unlock() + continue + } + started := p.startReconfigureLocked(e, want, enabled) + p.mu.Unlock() + return started } - want, enabled := p.cfgForWorkspace(e.root, e.language) - p.mu.Lock() - defer p.mu.Unlock() +} + +// startReconfigureLocked starts e's restart onto want, or marks a running one +// to run again. Caller holds p.mu. +func (p *workspacePool) startReconfigureLocked(e *poolEntry, want config.LSPConfig, enabled bool) bool { if e.reconfiguring { e.reconfigureAgain = true return false @@ -131,7 +154,7 @@ func (p *workspacePool) reconfigureEntry(e *poolEntry) bool { return false } e.reconfiguring = true - go p.restartWithConfig(e, want, enabled) + go p.restartWithConfig(e, want, enabled, restartReasonConfig) return true } @@ -160,13 +183,29 @@ func serverChange(a, b config.LSPConfig) bool { return !reflect.DeepEqual(a, b) } +// Why an entry is restarted in place, for its log lines: a changed config +// (PLAN-539), or a resync after its file watcher lost events (PLAN-538), which +// restarts it on the config it already has. +const ( + restartReasonConfig = "its new config" + restartReasonResync = "a resync after lost file events" +) + // restartWithConfig is one entry's in-place restart: hibernate it once no call // holds it, swap in a supervisor for cfg, and wake it when a session holds it. -func (p *workspacePool) restartWithConfig(e *poolEntry, cfg config.LSPConfig, enabled bool) { +// A round that ends with another already requested does not wake the server it +// built — that would start it on a config about to be replaced (expensive for a +// server such as jdtls) — so the wake is left to the round that follows, or, if +// none needs a restart after all, done here at the end. +func (p *workspacePool) restartWithConfig(e *poolEntry, cfg config.LSPConfig, enabled bool, reason string) { for { - p.restartOnce(e, cfg, enabled) + deferred := p.restartOnce(e, cfg, enabled, reason) + reason = restartReasonConfig // a further round is always a config change's var again bool if cfg, enabled, again = p.nextRestart(e); !again { + if deferred { + p.wakeHeld(e) + } return } } @@ -187,10 +226,10 @@ func (p *workspacePool) nextRestart(e *poolEntry) (config.LSPConfig, bool, bool) } e.reconfigureAgain = false p.mu.Unlock() - if p.projectConfigInvalid(e.root) { + want, enabled, invalid := p.entryConfig(e) + if invalid { continue // re-checks the flag; ends the round unless a request came in } - want, enabled := p.cfgForWorkspace(e.root, e.language) p.mu.Lock() restart := needsRestartLocked(e, want, enabled) p.mu.Unlock() @@ -201,16 +240,17 @@ func (p *workspacePool) nextRestart(e *poolEntry) (config.LSPConfig, bool, bool) } // restartOnce is one restart round: hibernate e once no call holds it, swap in -// a supervisor for cfg, and wake it when a session holds it. -func (p *workspacePool) restartOnce(e *poolEntry, cfg config.LSPConfig, enabled bool) { +// a supervisor for cfg, and wake it when a session holds it — unless another +// round is already requested, when it reports the wake deferred instead. +func (p *workspacePool) restartOnce(e *poolEntry, cfg config.LSPConfig, enabled bool, reason string) (wakeDeferred bool) { var env []string var goWorkOff string if enabled { env, goWorkOff = goLSPEnv(e.language, e.root, cfg, envFor(cfg)) } if err := p.hibernateWhenIdle(e); err != nil { - slog.Warn("pool: could not restart a language server for its new config", "root", e.root, "language", e.language, "err", err) - return + slog.Warn("pool: could not restart a language server for "+reason, "root", e.root, "language", e.language, "err", err) + return false } // p.mu is held from here (hibernateWhenIdle returns with it). if p.closed || p.entries[poolKey{e.root, e.language}] != e || !enabled { @@ -218,7 +258,7 @@ func (p *workspacePool) restartOnce(e *poolEntry, cfg config.LSPConfig, enabled if !enabled { slog.Info("pool: language disabled by config — its server stays stopped", "root", e.root, "language", e.language) } - return + return false } e.lspCfg = cfg e.goWorkOff = goWorkOff @@ -228,20 +268,65 @@ func (p *workspacePool) restartOnce(e *poolEntry, cfg config.LSPConfig, enabled e.cache.Clear() } held := e.refs > 0 + if held && e.reconfigureAgain { + p.mu.Unlock() + slog.Info("pool: restarted a language server for "+reason+"; its wake waits for the newer config", "root", e.root, "language", e.language) + return true + } var readyCh <-chan error var err error if held { readyCh, err = p.wakeLocked(e) } p.mu.Unlock() - slog.Info("pool: restarted a language server for its new config", "root", e.root, "language", e.language, "woken", held && err == nil) + slog.Info("pool: restarted a language server for "+reason, "root", e.root, "language", e.language, "woken", held && err == nil) + p.afterWake(e, held, readyCh, err) + return false +} + +// wakeHeld wakes e when a session still holds it and it is hibernated, as the +// last round of a restart that deferred its wake leaves it. An entry torn down, +// woken already, or taken by a newer restart (which wakes it itself) is left. +// +// It resolves the config first: the request that deferred the wake may have +// DISABLED the language, and a disabled language's server stays stopped. The +// resolve is checked against the config generation under p.mu, as in +// reconfigureEntry, so a disable published in between is not missed. +func (p *workspacePool) wakeHeld(e *poolEntry) { + for { + gen := p.langsGen.Load() + if _, enabled, invalid := p.entryConfig(e); !enabled && !invalid { + return + } + if p.reconfigureResolved != nil { + p.reconfigureResolved(e) + } + p.mu.Lock() + if p.langsGen.Load() == gen { + break // p.mu stays held + } + p.mu.Unlock() + } + if p.closed || p.entries[poolKey{e.root, e.language}] != e || e.reconfiguring || e.refs == 0 || e.state != poolHibernated { + p.mu.Unlock() + return + } + readyCh, err := p.wakeLocked(e) + p.mu.Unlock() + slog.Info("pool: woke a restarted language server once its config settled", "root", e.root, "language", e.language, "woken", err == nil) + p.afterWake(e, true, readyCh, err) +} + +// afterWake finishes a restart's wake outside p.mu: it reports a failed wake +// (the next acquire retries), and otherwise reattaches the file watcher +// hibernation dropped — the wake does not rebuild it (attachWatcher, PLAN-536) +// — and watches for a late start failure. +func (p *workspacePool) afterWake(e *poolEntry, woke bool, readyCh <-chan error, err error) { if err != nil { slog.Warn("pool: waking a reconfigured language server failed; the next acquire retries", "root", e.root, "language", e.language, "err", err) return } - if held { - // The wake does not rebuild the file watcher hibernation dropped; it is - // attached here, after the unlock (attachWatcher, PLAN-536). + if woke { p.attachWatcher(e) } if readyCh != nil { diff --git a/internal/cli/pool_reconfigure_test.go b/internal/cli/pool_reconfigure_test.go index 5a50872c..03661c6c 100644 --- a/internal/cli/pool_reconfigure_test.go +++ b/internal/cli/pool_reconfigure_test.go @@ -311,7 +311,7 @@ func TestRestartWithConfig_DoesNotReviveAfterClose(t *testing.T) { p.close() g := cfg.LSP["go"] g.Args = []string{"35"} - p.restartWithConfig(e, g, true) + p.restartWithConfig(e, g, true, restartReasonConfig) if now, c, state := entryNow(p, e); now != sup || slices.Equal(c.Args, []string{"35"}) || state.running() { t.Fatalf("a closed pool's entry was restarted (new supervisor %v, args %v, state %s)", now != sup, c.Args, state) } diff --git a/internal/cli/pool_reconfigure_window_test.go b/internal/cli/pool_reconfigure_window_test.go new file mode 100644 index 00000000..3a7ded46 --- /dev/null +++ b/internal/cli/pool_reconfigure_window_test.go @@ -0,0 +1,295 @@ +package cli + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "github.com/plumbkit/plumb/internal/config" +) + +// pool_reconfigure_window_test.go — the PLAN-539 follow-ups: one resolution +// per reconfigure, a stale resolve never undoing a newer restart, and no wake +// for a config already superseded. + +// waitSettled waits until e has no restart running. +func waitSettled(t *testing.T, p *workspacePool, e *poolEntry) { + t.Helper() + eventually(t, 5*time.Second, "the restart never finished", func() bool { + p.mu.Lock() + defer p.mu.Unlock() + return !e.reconfiguring + }) +} + +// TestReconfigureEntry_ResolvesOnce: the effective config and the project +// config's validity come from one resolution. +func TestReconfigureEntry_ResolvesOnce(t *testing.T) { + p, _ := reconfigPool(t) + e, err := p.acquireLang(context.Background(), markedRoot(t, "go.mod"), "go", true) + if err != nil { + t.Fatal(err) + } + before := p.langResolves.Load() + if p.reconfigureEntry(e) { + t.Fatal("control: an unchanged config started a restart") + } + if n := p.langResolves.Load() - before; n != 1 { + t.Fatalf("one reconfigure resolved the config %d times, want 1", n) + } +} + +// TestReconfigureEntry_AStaleResolveDoesNotUndoANewerRestart: a reconfigure +// that resolved its config just before a newer one was published, and reaches +// its decision after that newer restart finished, re-resolves instead of +// restarting the server back onto the old config. +func TestReconfigureEntry_AStaleResolveDoesNotUndoANewerRestart(t *testing.T) { + p, cfg := reconfigPool(t) + e, err := p.acquireLang(context.Background(), markedRoot(t, "go.mod"), "go", true) + if err != nil { + t.Fatal(err) + } + fired := false // the publish below re-enters this hook; only the outer call acts + p.reconfigureResolved = func(*poolEntry) { + if fired { + return + } + fired = true + // Inside the stale reconfigure's window: publish a newer config and + // let its restart finish. + p.applyGlobalLSP(withGoArgs(cfg, "50")) + waitReconfigured(t, p, e, func(c config.LSPConfig) bool { return slices.Equal(c.Args, []string{"50"}) }) + waitSettled(t, p, e) + } + p.reconfigureEntry(e) // resolved the original config, args 30 + waitSettled(t, p, e) + if _, c, _ := entryNow(p, e); !slices.Equal(c.Args, []string{"50"}) { + t.Fatalf("the server ended on args %v; a stale resolve undid the newer restart", c.Args) + } +} + +// heldAndBusy acquires e with a session hold, and starts a call on it that +// keeps a restart waiting until done is called. +func heldAndBusy(t *testing.T, p *workspacePool) (e *poolEntry, root string, done func()) { + t.Helper() + root = markedRoot(t, "go.mod") + e, err := p.acquireLang(context.Background(), root, "go", true) + if err != nil { + t.Fatal(err) + } + _, done, err = p.acquireCall(context.Background(), root, "go") + if err != nil { + t.Fatal(err) + } + return e, root, done +} + +// TestRestartWithConfig_DoesNotWakeASupersededConfig: a round that ends with +// a newer config already requested leaves its server asleep; the round for the +// newer config wakes it. +func TestRestartWithConfig_DoesNotWakeASupersededConfig(t *testing.T) { + logs := captureLogs(t) + p, cfg := reconfigPool(t) + e, _, done := heldAndBusy(t, p) + p.applyGlobalLSP(withGoArgs(cfg, "60")) + p.applyGlobalLSP(withGoArgs(cfg, "61")) + done() + waitReconfigured(t, p, e, func(c config.LSPConfig) bool { return slices.Equal(c.Args, []string{"61"}) }) + waitSettled(t, p, e) + eventually(t, 5*time.Second, "the deferred wake was never logged", func() bool { + return strings.Contains(logs.String(), "once its config settled") || strings.Contains(logs.String(), "woken=true") + }) + if !strings.Contains(logs.String(), "its wake waits for the newer config") { + t.Errorf("the superseded round woke its server; log:\n%s", logs.String()) + } + if _, _, state := entryNow(p, e); !state.running() { + t.Fatalf("the held server ended %s, want running on the newest config", state) + } +} + +// TestRestartWithConfig_ADeferredWakeIsNotLost: when the request that deferred +// the wake turns out to need no restart, the server is still woken. +func TestRestartWithConfig_ADeferredWakeIsNotLost(t *testing.T) { + p, cfg := reconfigPool(t) + e, root, done := heldAndBusy(t, p) + p.applyGlobalLSP(withGoArgs(cfg, "70")) + p.reconfigureRoot(root) // the same config again: marks the round to re-check + done() + waitReconfigured(t, p, e, func(c config.LSPConfig) bool { return slices.Equal(c.Args, []string{"70"}) }) + waitSettled(t, p, e) + eventually(t, 5*time.Second, "a held server whose wake was deferred was left asleep", func() bool { + _, _, state := entryNow(p, e) + return state.running() + }) +} + +// TestWakeHeld_LeavesAnEntryANewerRestartOwns: the deferred wake does not wake +// an entry a newer restart has already taken — that restart wakes it itself. +func TestWakeHeld_LeavesAnEntryANewerRestartOwns(t *testing.T) { + p, _ := reconfigPool(t) + e, err := p.acquireLang(context.Background(), markedRoot(t, "go.mod"), "go", true) + if err != nil { + t.Fatal(err) + } + if err := p.hibernateWhenIdle(e); err != nil { + t.Fatal(err) + } + e.reconfiguring = true // hibernateWhenIdle returned holding p.mu + p.mu.Unlock() + p.wakeHeld(e) + if _, _, state := entryNow(p, e); state != poolHibernated { + t.Fatalf("an entry a newer restart owns was woken (%s)", state) + } + p.mu.Lock() + e.reconfiguring = false + p.mu.Unlock() + p.wakeHeld(e) + eventually(t, 5*time.Second, "control: a held, hibernated entry with no restart running was not woken", func() bool { + _, _, state := entryNow(p, e) + return state.running() + }) +} + +// TestRestartWithConfig_ADeferredWakeLeavesADisabledLanguageStopped: a change +// that disables the language while a restart waits does not have its server +// woken at the end (review of #701). +func TestRestartWithConfig_ADeferredWakeLeavesADisabledLanguageStopped(t *testing.T) { + p, cfg := reconfigPool(t) + e, _, done := heldAndBusy(t, p) + p.applyGlobalLSP(withGoArgs(cfg, "90")) + off := withGoArgs(cfg, "90") + g := off.LSP["go"] + g.Enabled = false + off.LSP["go"] = g + p.applyGlobalLSP(off) + done() + waitSettled(t, p, e) + time.Sleep(100 * time.Millisecond) + if _, _, state := entryNow(p, e); state != poolHibernated { + t.Fatalf("a disabled language's held server ended %s, want hibernated", state) + } +} + +// TestRestartWithConfig_ASingleRoundWakesAtOnce: with no newer change waiting, +// a held server is woken by its own round, and the restart says it was for its +// new config. +func TestRestartWithConfig_ASingleRoundWakesAtOnce(t *testing.T) { + logs := captureLogs(t) + p, cfg := reconfigPool(t) + e, _, done := heldAndBusy(t, p) + p.applyGlobalLSP(withGoArgs(cfg, "91")) + done() + waitReconfigured(t, p, e, func(c config.LSPConfig) bool { return slices.Equal(c.Args, []string{"91"}) }) + waitSettled(t, p, e) + eventually(t, 5*time.Second, "the restart was never logged", func() bool { + return strings.Contains(logs.String(), "restarted a language server for") + }) + out := logs.String() + if strings.Contains(out, "its wake waits for the newer config") || strings.Contains(out, "once its config settled") { + t.Errorf("a single round deferred its wake:\n%s", out) + } + if !strings.Contains(out, "restarted a language server for its new config") { + t.Errorf("a config restart is not logged as one:\n%s", out) + } +} + +// TestWakeHeld_WakesOnlyAHeldHibernatedLiveEntry: the deferred wake leaves an +// entry nobody holds, one already running, and one of a closed pool. +func TestWakeHeld_WakesOnlyAHeldHibernatedLiveEntry(t *testing.T) { + logs := captureLogs(t) + p, _ := reconfigPool(t) + root := markedRoot(t, "go.mod") + e, err := p.acquireLang(context.Background(), root, "go", true) + if err != nil { + t.Fatal(err) + } + p.wakeHeld(e) // running already + if strings.Contains(logs.String(), "waking a reconfigured language server failed") || strings.Contains(logs.String(), "once its config settled") { + t.Errorf("a running entry was woken again:\n%s", logs.String()) + } + p.hibernateEntry(e) + p.mu.Lock() + refs := e.refs + e.refs = 0 + p.mu.Unlock() + p.wakeHeld(e) + if _, _, state := entryNow(p, e); state != poolHibernated { + t.Fatalf("an entry nobody holds was woken (%s)", state) + } + p.mu.Lock() + e.refs = refs + p.mu.Unlock() + p.close() + p.wakeHeld(e) + if _, _, state := entryNow(p, e); state != poolHibernated { + t.Fatalf("a closed pool's entry was woken (%s)", state) + } +} + +// TestRestartWithConfig_AnInvalidProjectFileDoesNotMoveAWaitingRound: a +// project file that breaks while a restart waits leaves the server on the +// config that round installed, not the global fallback. +func TestRestartWithConfig_AnInvalidProjectFileDoesNotMoveAWaitingRound(t *testing.T) { + p, cfg := reconfigPool(t) + p.baseConfig.LogLevel = "info" // the project-config resolver is active + root := markedRoot(t, "go.mod") + if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil { + t.Fatal(err) + } + writePoolProjectConfig(t, root, "[lsp.go]\ndiagnostics = \"pull\"\n") + e, err := p.acquireLang(context.Background(), root, "go", true) + if err != nil { + t.Fatal(err) + } + _, done, err := p.acquireCall(context.Background(), root, "go") + if err != nil { + t.Fatal(err) + } + p.applyGlobalLSP(withGoArgs(cfg, "92")) + p.applyGlobalLSP(withGoArgs(cfg, "93")) // marks the waiting round to re-check + // The file breaks before that re-check: it must not move the server onto + // the global fallback (args 93, push diagnostics). + writePoolProjectConfig(t, root, "[lsp.go\ndiagnostics = \n") + p.invalidateLanguageConfig(root) + done() + waitReconfigured(t, p, e, func(c config.LSPConfig) bool { return slices.Equal(c.Args, []string{"92"}) }) + waitSettled(t, p, e) + if _, c, _ := entryNow(p, e); c.Diagnostics != "pull" { + t.Fatalf("the server moved onto the global fallback (diagnostics %q)", c.Diagnostics) + } +} + +// TestWakeHeld_ADisableDuringItsResolveIsSeen: a disable published between the +// deferred wake's resolve and its decision leaves the server stopped. +func TestWakeHeld_ADisableDuringItsResolveIsSeen(t *testing.T) { + p, cfg := reconfigPool(t) + e, err := p.acquireLang(context.Background(), markedRoot(t, "go.mod"), "go", true) + if err != nil { + t.Fatal(err) + } + p.hibernateEntry(e) + off := cfg + g := cfg.LSP["go"] + g.Enabled = false + off.LSP = map[string]config.LSPConfig{"go": g, "python": cfg.LSP["python"]} + fired := false // the publish re-enters this hook; only the outer call acts + p.reconfigureResolved = func(*poolEntry) { + if fired { + return + } + fired = true + p.applyGlobalLSP(off) + } + p.wakeHeld(e) + if !fired { + t.Fatal("control: the hook never ran") + } + time.Sleep(100 * time.Millisecond) + if _, _, state := entryNow(p, e); state.running() { + t.Fatalf("a disable published during the deferred wake's resolve was missed (%s)", state) + } +} diff --git a/internal/cli/pool_resync.go b/internal/cli/pool_resync.go index e55b33fe..665be6dc 100644 --- a/internal/cli/pool_resync.go +++ b/internal/cli/pool_resync.go @@ -76,7 +76,7 @@ func (p *workspacePool) resyncEntry(e *poolEntry) { return } e.reconfiguring = true - go p.restartWithConfig(e, e.lspCfg, true) + go p.restartWithConfig(e, e.lspCfg, true, restartReasonResync) } // resyncInterval is the minimum gap between an entry's resyncs: losses that diff --git a/internal/cli/pool_resync_test.go b/internal/cli/pool_resync_test.go index c2ce3c35..da851cb2 100644 --- a/internal/cli/pool_resync_test.go +++ b/internal/cli/pool_resync_test.go @@ -449,3 +449,26 @@ func TestWatcherLost_ASkippedHibernationKeepsThePendingResync(t *testing.T) { t.Fatal("a refused hibernation dropped the pending resync") } } + +// TestWatcherLost_TheResyncSaysItIsOne: a resync's restart is logged as one, +// not as a config change. +func TestWatcherLost_TheResyncSaysItIsOne(t *testing.T) { + logs := captureLogs(t) + pool, f := resyncPool(t) + root := goModRoot(t) + e, err := pool.acquireLang(t.Context(), root, "go", true) + if err != nil { + t.Fatal(err) + } + sup := supOf(pool, e) + f.latest(root).lost <- struct{}{} + eventually(t, 5*time.Second, "the server was not resynced", func() bool { return supOf(pool, e) != sup }) + // The line is written after the swap is visible. + eventually(t, 5*time.Second, "the resync's restart was never logged", func() bool { + return strings.Contains(logs.String(), "restarted a language server for") + }) + out := logs.String() + if !strings.Contains(out, "restarted a language server for a resync after lost file events") || strings.Contains(out, "for its new config") { + t.Fatalf("the resync's restart is not logged as a resync:\n%s", out) + } +}