Add watcher preopen controls and harden server shutdown

README.md

@@ -239,6 +239,37 @@ Rebuild after making changes.
 
 Setting the `LOG_LEVEL` environment variable to DEBUG enables verbose logging to stderr for all components including messages to and from the language server and the language server's logs.
 
+### Runtime Controls
+
+The server keeps existing behavior by default. You can opt in to additional runtime controls with CLI flags or env vars:
+
+- `--watcher-preopen-on-register` (default: `true`)
+  - Env: `WATCHER_PREOPEN_ON_REGISTER` or `MCP_WATCHER_PREOPEN_ON_REGISTER`
+- `--watcher-preopen-max-files` (default: `0`, unlimited)
+  - Env: `WATCHER_PREOPEN_MAX_FILES` or `MCP_WATCHER_PREOPEN_MAX_FILES`
+- `--idle-timeout` (default: `0s`, disabled)
+  - Env: `IDLE_TIMEOUT` or `MCP_IDLE_TIMEOUT`
+
+Codex-specific example (disable registration preopen and auto-clean idle servers):
+
+```json
+{
+  "mcpServers": {
+    "swift-lsp": {
+      "command": "/Users/you/.local/bin/mcp-language-server-codex",
+      "args": [
+        "--workspace",
+        "/path/to/workspace",
+        "--lsp",
+        "sourcekit-lsp",
+        "--watcher-preopen-on-register=false",
+        "--idle-timeout=15m"
+      ]
+    }
+  }
+}
+```
+
 ### LSP interaction
 
 - `internal/lsp/methods.go` contains generated code to make calls to the connected language server.

internal/lsp/client.go

@@ -44,6 +44,12 @@ type Client struct {
 	// Files are currently opened by the LSP
 	openFiles   map[string]*OpenFileInfo
 	openFilesMu sync.RWMutex
+
+	// Idempotent shutdown state
+	closeOnce sync.Once
+	closeErr  error
+	waitOnce  sync.Once
+	waitErr   error
 }
 
 func NewClient(command string, args ...string) (*Client, error) {
@@ -228,6 +234,14 @@ func (c *Client) InitializeLSPClient(ctx context.Context, workspaceDir string) (
 }
 
 func (c *Client) Close() error {
+	c.closeOnce.Do(func() {
+		c.closeErr = c.closeInternal()
+	})
+
+	return c.closeErr
+}
+
+func (c *Client) closeInternal() error {
 	// Try to close all open files first
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
@@ -236,35 +250,42 @@ func (c *Client) Close() error {
 	c.CloseAllFiles(ctx)
 
 	// Force kill the LSP process if it doesn't exit within timeout
-	forcedKill := make(chan struct{})
+	forcedKillDone := make(chan struct{})
 	go func() {
 		select {
 		case <-time.After(2 * time.Second):
 			lspLogger.Warn("LSP process did not exit within timeout, forcing kill")
-			if c.Cmd.Process != nil {
+			if c.Cmd != nil && c.Cmd.Process != nil {
 				if err := c.Cmd.Process.Kill(); err != nil {
 					lspLogger.Error("Failed to kill process: %v", err)
 				} else {
 					lspLogger.Info("Process killed successfully")
 				}
 			}
-			close(forcedKill)
-		case <-forcedKill:
+		case <-forcedKillDone:
 			// Channel closed from completion path
 			return
 		}
 	}()
 
 	// Close stdin to signal the server
-	if err := c.stdin.Close(); err != nil {
-		lspLogger.Error("Failed to close stdin: %v", err)
+	if c.stdin != nil {
+		if err := c.stdin.Close(); err != nil {
+			lspLogger.Error("Failed to close stdin: %v", err)
+		}
 	}
 
-	// Wait for process to exit
-	err := c.Cmd.Wait()
-	close(forcedKill) // Stop the force kill goroutine
+	// Wait for process to exit once.
+	c.waitOnce.Do(func() {
+		if c.Cmd == nil {
+			return
+		}
+		c.waitErr = c.Cmd.Wait()
+	})
+
+	close(forcedKillDone) // Stop the force kill goroutine
 
-	return err
+	return c.waitErr
 }
 
 type ServerState int

internal/lsp/client_close_test.go

@@ -0,0 +1,15 @@
+package lsp
+
+import "testing"
+
+func TestClientCloseIsIdempotent(t *testing.T) {
+	client := &Client{}
+
+	if err := client.Close(); err != nil {
+		t.Fatalf("first close returned error: %v", err)
+	}
+
+	if err := client.Close(); err != nil {
+		t.Fatalf("second close returned error: %v", err)
+	}
+}

internal/watcher/interfaces.go

@@ -27,6 +27,12 @@ type WatcherConfig struct {
 	// DebounceTime is the duration to wait before sending file change events
 	DebounceTime time.Duration
 
+	// PreopenOnRegistration controls whether AddRegistrations should pre-open matching files.
+	PreopenOnRegistration bool
+
+	// PreopenMaxFiles limits file pre-opening per registration (0 = unlimited).
+	PreopenMaxFiles int
+
 	// ExcludedDirs are directory names that should be excluded from watching
 	ExcludedDirs map[string]bool
 
@@ -43,7 +49,9 @@ type WatcherConfig struct {
 // DefaultWatcherConfig returns a configuration with sensible defaults
 func DefaultWatcherConfig() *WatcherConfig {
 	return &WatcherConfig{
-		DebounceTime: 300 * time.Millisecond,
+		DebounceTime:          300 * time.Millisecond,
+		PreopenOnRegistration: true,
+		PreopenMaxFiles:       0,
 		ExcludedDirs: map[string]bool{
 			".git":         true,
 			"node_modules": true,

internal/watcher/testing/mock_client.go

@@ -19,10 +19,12 @@ type MockLSPClient struct {
 	mu             sync.Mutex
 	events         []FileEvent
 	openedFiles    map[string]bool
+	openCalls      int
 	openErrors     map[string]error
 	notifyErrors   map[string]error
 	changeErrors   map[string]error
 	eventsReceived chan struct{}
+	opensReceived  chan struct{}
 }
 
 // NewMockLSPClient creates a new mock LSP client for testing
@@ -34,6 +36,7 @@ func NewMockLSPClient() *MockLSPClient {
 		notifyErrors:   make(map[string]error),
 		changeErrors:   make(map[string]error),
 		eventsReceived: make(chan struct{}, 100), // Buffer to avoid blocking
+		opensReceived:  make(chan struct{}, 100), // Buffer to avoid blocking
 	}
 }
 
@@ -53,7 +56,16 @@ func (m *MockLSPClient) OpenFile(ctx context.Context, path string) error {
 		return err
 	}
 
+	m.openCalls++
 	m.openedFiles[path] = true
+
+	// Signal that an open happened
+	select {
+	case m.opensReceived <- struct{}{}:
+	default:
+		// Channel is full, but we don't want to block
+	}
+
 	return nil
 }
 
@@ -153,5 +165,31 @@ func (m *MockLSPClient) WaitForEvent(ctx context.Context) bool {
 	}
 }
 
+// OpenCallCount returns how many times OpenFile has been called.
+func (m *MockLSPClient) OpenCallCount() int {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.openCalls
+}
+
+// WaitForOpenCalls waits until at least min OpenFile calls have been observed.
+func (m *MockLSPClient) WaitForOpenCalls(ctx context.Context, min int) bool {
+	for {
+		m.mu.Lock()
+		current := m.openCalls
+		m.mu.Unlock()
+
+		if current >= min {
+			return true
+		}
+
+		select {
+		case <-m.opensReceived:
+		case <-ctx.Done():
+			return false
+		}
+	}
+}
+
 // Verify the MockLSPClient implements the watcher.LSPClient interface
 var _ watcher.LSPClient = (*MockLSPClient)(nil)

internal/watcher/testing/watcher_test.go

@@ -433,3 +433,122 @@ func TestRapidChangesDebouncing(t *testing.T) {
 		}
 	})
 }
+
+func TestRegistrationPreopenControls(t *testing.T) {
+	if os.Getenv("GITHUB_ACTIONS") == "true" {
+		t.Skip("Skipping filesystem watcher tests in GitHub Actions environment")
+	}
+
+	t.Run("PreopenDisabledSkipsWorkspaceScan", func(t *testing.T) {
+		openCalls := runPreopenRegistrationScenario(t, func(config *watcher.WatcherConfig) {
+			config.PreopenOnRegistration = false
+			config.PreopenMaxFiles = 0
+		})
+
+		if openCalls != 0 {
+			t.Fatalf("expected 0 opened files with preopen disabled, got %d", openCalls)
+		}
+	})
+
+	t.Run("PreopenEnabledOpensMatchingFiles", func(t *testing.T) {
+		openCalls := runPreopenRegistrationScenario(t, func(config *watcher.WatcherConfig) {
+			config.PreopenOnRegistration = true
+			config.PreopenMaxFiles = 0
+		})
+
+		// Three .swift files should match the watcher pattern.
+		if openCalls != 3 {
+			t.Fatalf("expected 3 opened files with preopen enabled, got %d", openCalls)
+		}
+	})
+
+	t.Run("PreopenMaxFilesCapsOpenedFiles", func(t *testing.T) {
+		openCalls := runPreopenRegistrationScenario(t, func(config *watcher.WatcherConfig) {
+			config.PreopenOnRegistration = true
+			config.PreopenMaxFiles = 2
+		})
+
+		if openCalls != 2 {
+			t.Fatalf("expected opened files to be capped at 2, got %d", openCalls)
+		}
+	})
+}
+
+func runPreopenRegistrationScenario(t *testing.T, configure func(*watcher.WatcherConfig)) int {
+	t.Helper()
+
+	testDir, err := os.MkdirTemp("", "watcher-preopen-test-*")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer func() {
+		if err := os.RemoveAll(testDir); err != nil {
+			t.Logf("failed to remove test directory: %v", err)
+		}
+	}()
+
+	srcDir := filepath.Join(testDir, "src")
+	if err := os.MkdirAll(srcDir, 0755); err != nil {
+		t.Fatalf("failed to create src directory: %v", err)
+	}
+
+	matchingFiles := []string{
+		filepath.Join(srcDir, "Alpha.swift"),
+		filepath.Join(srcDir, "Beta.swift"),
+		filepath.Join(srcDir, "Gamma.swift"),
+	}
+	nonMatchingFiles := []string{
+		filepath.Join(srcDir, "Notes.txt"),
+	}
+
+	for _, path := range append(matchingFiles, nonMatchingFiles...) {
+		if err := os.WriteFile(path, []byte("test content"), 0644); err != nil {
+			t.Fatalf("failed to write test file %s: %v", path, err)
+		}
+	}
+
+	mockClient := NewMockLSPClient()
+	config := watcher.DefaultWatcherConfig()
+	config.DebounceTime = 50 * time.Millisecond
+	configure(config)
+
+	testWatcher := watcher.NewWorkspaceWatcherWithConfig(mockClient, config)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second)
+	defer cancel()
+
+	go testWatcher.WatchWorkspace(ctx, testDir)
+	time.Sleep(400 * time.Millisecond)
+
+	watchers := []protocol.FileSystemWatcher{
+		{
+			GlobPattern: protocol.GlobPattern{Value: "**/*.swift"},
+			Kind: func() *protocol.WatchKind {
+				kind := protocol.WatchKind(protocol.WatchCreate | protocol.WatchChange | protocol.WatchDelete)
+				return &kind
+			}(),
+		},
+	}
+
+	testWatcher.AddRegistrations(ctx, "preopen-test", watchers)
+
+	expectedOpenCalls := 0
+	if config.PreopenOnRegistration {
+		expectedOpenCalls = len(matchingFiles)
+		if config.PreopenMaxFiles > 0 && config.PreopenMaxFiles < expectedOpenCalls {
+			expectedOpenCalls = config.PreopenMaxFiles
+		}
+	}
+
+	if expectedOpenCalls > 0 {
+		waitCtx, waitCancel := context.WithTimeout(ctx, 3*time.Second)
+		defer waitCancel()
+
+		if !mockClient.WaitForOpenCalls(waitCtx, expectedOpenCalls) {
+			t.Fatalf("timed out waiting for %d opened files, got %d", expectedOpenCalls, mockClient.OpenCallCount())
+		}
+	}
+
+	time.Sleep(200 * time.Millisecond)
+	return mockClient.OpenCallCount()
+}