git.iko.soy/crush
on main help sign in
fix(lsp): mitigate stale diagnostics
d3f6d98Christian Rocha
internal/agent/tools/diagnostics.go+23−2
compare with main
unifiedsplit
@@ -87,25 +87,46 @@ func waitForLSPDiagnostics(
87
87
88// notifyLSPs notifies LSP servers that a file has changed and waits for
88// notifyLSPs notifies LSP servers that a file has changed and waits for
89// updated diagnostics. Use this after edit/multiedit operations.
89// updated diagnostics. Use this after edit/multiedit operations.
90+// When filepath is empty, refreshes all open files across all LSP clients
91+// and sends a workspace-level change notification for full re-analysis.
90func notifyLSPs(
92func notifyLSPs(
91 ctx context.Context,
93 ctx context.Context,
92 manager *lsp.Manager,
94 manager *lsp.Manager,
93 filepath string,
95 filepath string,
94) {
96) {
97+ if manager == nil {
98+ return
99+ }
95− if filepath == "" || manager == nil {
100+ if filepath == "" {
101+ // No specific file — refresh all open files for all clients.
102+ var wg sync.WaitGroup
103+ for client := range manager.Clients().Seq() {
104+ wg.Go(func() {
105+ client.RefreshOpenFiles(ctx)
106+ if err := client.NotifyWorkspaceChange(ctx); err != nil {
107+ slog.WarnContext(ctx, "Failed to notify workspace change", "error", err)
108+ }
109+ client.WaitForDiagnostics(ctx, 5*time.Second)
110+ })
111+ }
112+ wg.Wait()
96 return
113 return
97 }
114 }
98
115
99 manager.Start(ctx, filepath)
116 manager.Start(ctx, filepath)
100
117
118+ var wg sync.WaitGroup
101 for client := range manager.Clients().Seq() {
119 for client := range manager.Clients().Seq() {
102 if !client.HandlesFile(filepath) {
120 if !client.HandlesFile(filepath) {
103 continue
121 continue
104 }
122 }
105 _ = client.OpenFileOnDemand(ctx, filepath)
123 _ = client.OpenFileOnDemand(ctx, filepath)
106 _ = client.NotifyChange(ctx, filepath)
124 _ = client.NotifyChange(ctx, filepath)
125+ wg.Go(func() {
107− client.WaitForDiagnostics(ctx, 5*time.Second)
126+ client.WaitForDiagnostics(ctx, 5*time.Second)
127+ })
108 }
128 }
129+ wg.Wait()
109}
130}
110
131
111func getDiagnostics(filePath string, manager *lsp.Manager) string {
132func getDiagnostics(filePath string, manager *lsp.Manager) string {
internal/lsp/client.go+100−7
@@ -530,23 +530,116 @@ func (c *Client) openKeyConfigFiles(ctx context.Context) {
530 }
530 }
531}
531}
532
532
533−// WaitForDiagnostics waits until diagnostics change or the timeout is reached.
533+// NotifyWorkspaceChange sends a workspace-level file change notification to
534+// trigger re-analysis of all files. This is useful when the overall project
535+// state may have changed (e.g., after a project-wide refactoring) and
536+// diagnostics for files not currently being edited may be stale.
534−func (c *Client) WaitForDiagnostics(ctx context.Context, d time.Duration) {
537+func (c *Client) NotifyWorkspaceChange(ctx context.Context) error {
538+ if c == nil {
539+ return nil
540+ }
541+ return c.client.NotifyDidChangeWatchedFiles(ctx, []protocol.FileEvent{
542+ {URI: protocol.DocumentURI(protocol.URIFromPath(c.cwd)), Type: protocol.Changed},
543+ })
544+}
545+
546+// RefreshOpenFiles re-notifies the LSP server about all currently open files,
547+// which triggers re-analysis and fresh diagnostics for the entire project.
548+func (c *Client) RefreshOpenFiles(ctx context.Context) {
535 if c == nil {
549 if c == nil {
536 return
550 return
537 }
551 }
552+ for uri, info := range c.openFiles.Seq2() {
553+ path, err := protocol.DocumentURI(uri).Path()
554+ if err != nil {
555+ slog.Warn("Failed to convert URI to path", "uri", uri, "error", err)
556+ continue
557+ }
558+ content, err := os.ReadFile(path)
559+ if err != nil {
560+ slog.Warn("Failed to read file for refresh", "path", path, "error", err)
561+ continue
562+ }
563+ info.Version++
564+ changes := []protocol.TextDocumentContentChangeEvent{
565+ {
566+ Value: protocol.TextDocumentContentChangeWholeDocument{
567+ Text: string(content),
568+ },
569+ },
570+ }
571+ if err := c.client.NotifyDidChangeTextDocument(ctx, uri, int(info.Version), changes); err != nil {
572+ slog.Warn("Failed to notify file change", "uri", uri, "error", err)
573+ }
574+ }
575+}
576+
577+// WaitForDiagnostics waits until diagnostics stop changing for a settling
578+// period, indicating the LSP server has finished processing. If no
579+// diagnostics change within firstChangeDuration, it returns early since the
580+// server likely isn't going to republish.
581+func (c *Client) WaitForDiagnostics(ctx context.Context, timeout time.Duration) {
582+ if c == nil {
583+ return
584+ }
585+
586+ const (
587+ firstChangeDuration = 1 * time.Second
588+ settleDuration = 300 * time.Millisecond
589+ )
590+
591+ deadline := time.NewTimer(timeout)
592+ defer deadline.Stop()
593+ firstChangeTimer := time.NewTimer(min(timeout, firstChangeDuration))
594+ defer firstChangeTimer.Stop()
595+ previousVersion := c.diagnostics.Version()
538− ticker := time.NewTicker(200 * time.Millisecond)
596+ ticker := time.NewTicker(100 * time.Millisecond)
539 defer ticker.Stop()
597 defer ticker.Stop()
540− timeout := time.After(d)
598+
541− pv := c.diagnostics.Version()
542 for {
599 for {
543 select {
600 select {
544 case <-ctx.Done():
601 case <-ctx.Done():
545 return
602 return
546− case <-timeout:
603+ case <-deadline.C:
604+ return
605+ case <-firstChangeTimer.C:
606+ // No change arrived quickly — server isn't republishing.
547 return
607 return
548 case <-ticker.C:
608 case <-ticker.C:
549− if pv != c.diagnostics.Version() {
609+ currentVersion := c.diagnostics.Version()
610+ if currentVersion != previousVersion {
611+ // Diagnostics changed — now wait for them to settle.
612+ c.waitForDiagnosticsToSettle(ctx, deadline.C, settleDuration)
613+ return
614+ }
615+ }
616+ }
617+}
618+
619+// waitForDiagnosticsToSettle waits until diagnostics version stays the same
620+// for settleDuration, indicating the LSP server has finished publishing.
621+func (c *Client) waitForDiagnosticsToSettle(ctx context.Context, deadline <-chan time.Time, settleDuration time.Duration) {
622+ lastVersion := c.diagnostics.Version()
623+ settleTicker := time.NewTicker(50 * time.Millisecond)
624+ defer settleTicker.Stop()
625+
626+ // Track how long the version has been stable.
627+ stableStart := time.Now()
628+
629+ for {
630+ select {
631+ case <-ctx.Done():
632+ return
633+ case <-deadline:
634+ return
635+ case <-settleTicker.C:
636+ currentVersion := c.diagnostics.Version()
637+ if currentVersion != lastVersion {
638+ // New change detected — reset the stable timer.
639+ lastVersion = currentVersion
640+ stableStart = time.Now()
641+ } else if time.Since(stableStart) >= settleDuration {
642+ // Diagnostics have been stable for the settle duration.
550 return
643 return
551 }
644 }
552 }
645 }
internal/lsp/client_test.go+96
@@ -2,11 +2,14 @@ package lsp
2
2
3import (
3import (
4 "context"
4 "context"
5+ "fmt"
5 "testing"
6 "testing"
6 "time"
7 "time"
7
8
8 "github.com/charmbracelet/crush/internal/config"
9 "github.com/charmbracelet/crush/internal/config"
10+ "github.com/charmbracelet/crush/internal/csync"
9 "github.com/charmbracelet/crush/internal/env"
11 "github.com/charmbracelet/crush/internal/env"
12+ "github.com/charmbracelet/x/powernap/pkg/lsp/protocol"
10 "github.com/stretchr/testify/require"
13 "github.com/stretchr/testify/require"
11)
14)
12
15
@@ -70,3 +73,96 @@ func TestNilClient(t *testing.T) {
70 require.Nil(t, c.NotifyChange(context.Background(), "/some/file.go"))
73 require.Nil(t, c.NotifyChange(context.Background(), "/some/file.go"))
71 c.WaitForDiagnostics(context.Background(), time.Second)
74 c.WaitForDiagnostics(context.Background(), time.Second)
72}
75}
76+
77+func newTestClient() *Client {
78+ c := &Client{
79+ name: "test",
80+ diagnostics: csync.NewVersionedMap[protocol.DocumentURI, []protocol.Diagnostic](),
81+ openFiles: csync.NewMap[string, *OpenFileInfo](),
82+ }
83+ c.serverState.Store(StateStopped)
84+ return c
85+}
86+
87+func TestWaitForDiagnostics_NoChange(t *testing.T) {
88+ t.Parallel()
89+
90+ c := newTestClient()
91+ start := time.Now()
92+ c.WaitForDiagnostics(t.Context(), 5*time.Second)
93+ elapsed := time.Since(start)
94+
95+ // Should return early via firstChangeDeadline (~1s), not the full timeout.
96+ require.Less(t, elapsed, 2*time.Second, "should return early when no diagnostics change")
97+}
98+
99+func TestWaitForDiagnostics_ImmediateChange(t *testing.T) {
100+ t.Parallel()
101+
102+ c := newTestClient()
103+
104+ go func() {
105+ time.Sleep(100 * time.Millisecond)
106+ c.diagnostics.Set(protocol.DocumentURI("file:///test.go"), nil)
107+ }()
108+
109+ start := time.Now()
110+ c.WaitForDiagnostics(t.Context(), 5*time.Second)
111+ elapsed := time.Since(start)
112+
113+ // Should detect the change and then settle (~300ms settle + overhead).
114+ require.Less(t, elapsed, 2*time.Second, "should return after settling, not full timeout")
115+ require.Greater(t, elapsed, 200*time.Millisecond, "should wait for settle duration")
116+}
117+
118+func TestWaitForDiagnostics_RepeatedChanges(t *testing.T) {
119+ t.Parallel()
120+
121+ c := newTestClient()
122+
123+ // Simulate an LSP server that publishes diagnostics in bursts.
124+ go func() {
125+ for i := range 5 {
126+ time.Sleep(50 * time.Millisecond)
127+ c.diagnostics.Set(protocol.DocumentURI("file:///test.go"), []protocol.Diagnostic{
128+ {Message: fmt.Sprintf("diag-%d", i)},
129+ })
130+ }
131+ }()
132+
133+ start := time.Now()
134+ c.WaitForDiagnostics(t.Context(), 5*time.Second)
135+ elapsed := time.Since(start)
136+
137+ // Should wait for diagnostics to settle after the burst finishes.
138+ // Burst lasts ~250ms, then 300ms settle window, so total ~550ms+.
139+ require.Less(t, elapsed, 2*time.Second, "should return after settling, not full timeout")
140+ require.Greater(t, elapsed, 400*time.Millisecond, "should wait for all changes to settle")
141+}
142+
143+func TestWaitForDiagnostics_ContextCancellation(t *testing.T) {
144+ t.Parallel()
145+
146+ c := newTestClient()
147+ ctx, cancel := context.WithCancel(t.Context())
148+ defer cancel()
149+
150+ go func() {
151+ time.Sleep(200 * time.Millisecond)
152+ cancel()
153+ }()
154+
155+ start := time.Now()
156+ c.WaitForDiagnostics(ctx, 5*time.Second)
157+ elapsed := time.Since(start)
158+
159+ require.Less(t, elapsed, 1*time.Second, "should return shortly after context cancellation")
160+}
161+
162+func TestWaitForDiagnostics_NilClient(t *testing.T) {
163+ t.Parallel()
164+
165+ var c *Client
166+ // Should not panic.
167+ c.WaitForDiagnostics(context.Background(), time.Second)
168+}