From abb7971dd79e208f77544dd3c905e664b59000d5 Mon Sep 17 00:00:00 2001 From: Vladislav Chugunov Date: Sat, 16 May 2026 22:41:54 +0300 Subject: [PATCH] feat: fix file change handling and auto-detect open-files-on-registration - For open files, send didChange + didSave instead of didChange alone. didSave triggers expensive server-side work (type checking, diagnostics). Atomic writes appear as Created events, now treated same as Changed. - Closed files and deletes use workspace/didChangeWatchedFiles (as per LSP spec). - OpenFilesOnRegistration now defaults to false and is auto-detected from ServerInfo.Name in the initialize response instead of always true hardcode. - Add NotifySave to LSPClient interface and Client. --- internal/lsp/client.go | 12 ++++++++ internal/watcher/interfaces.go | 8 +++++ internal/watcher/testing/mock_client.go | 5 ++++ internal/watcher/watcher.go | 39 ++++++++++++++++++++----- main.go | 18 +++++++++++- 5 files changed, 73 insertions(+), 9 deletions(-) diff --git a/internal/lsp/client.go b/internal/lsp/client.go index fc07059d..bd81e37b 100644 --- a/internal/lsp/client.go +++ b/internal/lsp/client.go @@ -366,6 +366,18 @@ func (c *Client) NotifyChange(ctx context.Context, filepath string) error { return c.Notify(ctx, "textDocument/didChange", params) } +func (c *Client) NotifySave(ctx context.Context, filepath string) error { + uri := fmt.Sprintf("file://%s", filepath) + + params := protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: protocol.DocumentUri(uri), + }, + } + + return c.Notify(ctx, "textDocument/didSave", params) +} + func (c *Client) CloseFile(ctx context.Context, filepath string) error { uri := fmt.Sprintf("file://%s", filepath) diff --git a/internal/watcher/interfaces.go b/internal/watcher/interfaces.go index db05631a..7617c49e 100644 --- a/internal/watcher/interfaces.go +++ b/internal/watcher/interfaces.go @@ -18,6 +18,9 @@ type LSPClient interface { // NotifyChange notifies the server of a file change NotifyChange(ctx context.Context, path string) error + // NotifySave notifies the server that a file was saved (triggers diagnostics re-run) + NotifySave(ctx context.Context, path string) error + // DidChangeWatchedFiles sends watched file events to the server DidChangeWatchedFiles(ctx context.Context, params protocol.DidChangeWatchedFilesParams) error } @@ -27,6 +30,11 @@ type WatcherConfig struct { // DebounceTime is the duration to wait before sending file change events DebounceTime time.Duration + // OpenFilesOnRegistration controls whether all matching workspace files are opened + // via textDocument/didOpen when file watchers are registered. Required for some + // language servers (e.g. typescript-language-server). + OpenFilesOnRegistration bool + // ExcludedDirs are directory names that should be excluded from watching ExcludedDirs map[string]bool diff --git a/internal/watcher/testing/mock_client.go b/internal/watcher/testing/mock_client.go index abfd003d..83eb8c02 100644 --- a/internal/watcher/testing/mock_client.go +++ b/internal/watcher/testing/mock_client.go @@ -82,6 +82,11 @@ func (m *MockLSPClient) NotifyChange(ctx context.Context, path string) error { return nil } +// NotifySave mocks notifying the server that a file was saved +func (m *MockLSPClient) NotifySave(ctx context.Context, path string) error { + return nil +} + // DidChangeWatchedFiles mocks sending watched file events to the server func (m *MockLSPClient) DidChangeWatchedFiles(ctx context.Context, params protocol.DidChangeWatchedFilesParams) error { m.mu.Lock() diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 6e7a0b8a..72bf05d0 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -111,8 +111,11 @@ func (w *WorkspaceWatcher) AddRegistrations(ctx context.Context, id string, watc } } - // Find and open all existing files that match the newly registered patterns - // TODO: not all language servers require this, but typescript does. Make this configurable + // Find and open all existing files that match the newly registered patterns. + // Only enabled for language servers that require it (e.g. typescript-language-server). + if !w.config.OpenFilesOnRegistration { + return + } go func() { startTime := time.Now() filesOpened := 0 @@ -531,17 +534,37 @@ func (w *WorkspaceWatcher) debounceHandleFileEvent(ctx context.Context, uri stri // handleFileEvent sends file change notifications func (w *WorkspaceWatcher) handleFileEvent(ctx context.Context, uri string, changeType protocol.FileChangeType) { - // If the file is open and it's a change event, use didChange notification filePath := uri[7:] // Remove "file://" prefix - if changeType == protocol.FileChangeType(protocol.Changed) && w.client.IsFileOpen(filePath) { - err := w.client.NotifyChange(ctx, filePath) - if err != nil { - watcherLogger.Error("Error notifying change: %v", err) + + // Atomic writes (write-to-temp, rename) appear as Created events, so Changed and + // Created need the same treatment. + if changeType == protocol.FileChangeType(protocol.Changed) || + changeType == protocol.FileChangeType(protocol.Created) { + if w.client.IsFileOpen(filePath) { + // didChange: delivers the new file content to the server so it can update + // its in-memory document and run incremental analysis (syntax, basic linting). + if err := w.client.NotifyChange(ctx, filePath); err != nil { + watcherLogger.Error("Error notifying change: %v", err) + return + } + // didSave: signals that the content was persisted to disk, triggering + // expensive server-side work such as type checking and full diagnostics. + // An external tool writing a file is act like a save, + // so this should be semantically correct. + if err := w.client.NotifySave(ctx, filePath); err != nil { + watcherLogger.Error("Error notifying save: %v", err) + } + return + } + // File is not open: use workspace/didChangeWatchedFiles so the server + // reloads from disk. + if err := w.notifyFileEvent(ctx, uri, changeType); err != nil { + watcherLogger.Error("Error notifying LSP server about file event: %v", err) } return } - // Notify LSP server about the file event using didChangeWatchedFiles + // For delete events, use workspace/didChangeWatchedFiles if err := w.notifyFileEvent(ctx, uri, changeType); err != nil { watcherLogger.Error("Error notifying LSP server about file event: %v", err) } diff --git a/main.go b/main.go index f6f3ed5c..13964f46 100644 --- a/main.go +++ b/main.go @@ -80,6 +80,17 @@ func newServer(config *config) (*mcpServer, error) { }, nil } +// requiresOpenFilesOnRegistration returns true for language servers that need all +// workspace files opened via didOpen before they can provide correct results. +func requiresOpenFilesOnRegistration(serverName string) bool { + switch serverName { + case "typescript-language-server": + return true + default: + return false + } +} + func (s *mcpServer) initializeLSP() error { if err := os.Chdir(s.config.workspaceDir); err != nil { return fmt.Errorf("failed to change to workspace directory: %v", err) @@ -90,7 +101,6 @@ func (s *mcpServer) initializeLSP() error { return fmt.Errorf("failed to create LSP client: %v", err) } s.lspClient = client - s.workspaceWatcher = watcher.NewWorkspaceWatcher(client) initResult, err := client.InitializeLSPClient(s.ctx, s.config.workspaceDir) if err != nil { @@ -99,6 +109,12 @@ func (s *mcpServer) initializeLSP() error { coreLogger.Debug("Server capabilities: %+v", initResult.Capabilities) + watcherConfig := watcher.DefaultWatcherConfig() + if initResult.ServerInfo != nil { + watcherConfig.OpenFilesOnRegistration = requiresOpenFilesOnRegistration(initResult.ServerInfo.Name) + } + s.workspaceWatcher = watcher.NewWorkspaceWatcherWithConfig(client, watcherConfig) + go s.workspaceWatcher.WatchWorkspace(s.ctx, s.config.workspaceDir) return client.WaitForServerReady(s.ctx) }