From 84af16f8de38d435610e25bd4a33e8a67c5d5b89 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Mon, 15 Sep 2025 16:33:18 -0300 Subject: [PATCH 01/12] adding plan for file monitoring --- docs/filemon.checklist.md | 154 ++++++++++++++++++++++ docs/filemon.design.md | 149 ++++++++++++++++++++++ docs/filemon.plan.md | 261 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 564 insertions(+) create mode 100644 docs/filemon.checklist.md create mode 100644 docs/filemon.design.md create mode 100644 docs/filemon.plan.md diff --git a/docs/filemon.checklist.md b/docs/filemon.checklist.md new file mode 100644 index 0000000..de1f0cd --- /dev/null +++ b/docs/filemon.checklist.md @@ -0,0 +1,154 @@ +# File Monitoring Implementation Checklist + +## Phase 1: Core File Monitoring Service + +### 1.1 Interface Creation +- [ ] Create `IFileMonitoringService` interface +- [ ] Create `IFileChangeVisitor` interface +- [ ] Add interfaces to appropriate namespace +- [ ] Add XML documentation for public APIs + +### 1.2 FileMonitoringService Implementation +- [ ] Create `FileMonitoringService` class implementing `IFileMonitoringService` +- [ ] Implement `FileSystemWatcher` setup and disposal +- [ ] Add support for monitored file extensions (`.sln`, `.csproj`, `.vbproj`, `.fsproj`, `.cs`, `.vb`, `.fs`, `.editorconfig`) +- [ ] Implement operation tracking with concurrent operation support +- [ ] Create `FileChangeVisitor` class implementing `IFileChangeVisitor` +- [ ] Implement expected change tracking and validation logic +- [ ] Add external change detection logic (3 scenarios) +- [ ] Implement change accounting and statistics +- [ ] Add error recovery for `FileSystemWatcher` failures +- [ ] Add logging for all file change events +- [ ] Handle edge cases (temporary files, permission issues) + +### 1.3 Dependency Injection Setup +- [ ] Register `IFileMonitoringService` as singleton in `ServiceCollectionExtensions.cs` +- [ ] Ensure proper disposal through DI container +- [ ] Add configuration binding for `FileMonitoringOptions` + +## Phase 2: Integration with SolutionManager + +### 2.1 SolutionManager Modifications +- [ ] Add `IFileMonitoringService` dependency injection to constructor +- [ ] Modify `ToolHelpers.EnsureSolutionLoaded()` to check `IsReloadNeeded` +- [ ] Add reload logic calling `ReloadSolutionFromDiskAsync()` when needed +- [ ] Call `MarkReloadComplete()` after successful reload +- [ ] Add file monitoring setup in `LoadSolutionAsync()` +- [ ] Add logging for reload triggers + +### 2.2 CodeModificationService Modifications +- [ ] Add `IFileMonitoringService` dependency injection to constructor +- [ ] Modify `ApplyChangesAsync()` signature to use visitor pattern +- [ ] Add operation begin/end logic with `fileChangeVisitor` +- [ ] Implement dynamic change registration for changed documents +- [ ] Implement dynamic change registration for added documents +- [ ] Implement dynamic change registration for removed documents +- [ ] Add support for additional file paths (non-code files) +- [ ] Add git file registration for git operations +- [ ] Add operation completion logging with statistics +- [ ] Update all callers to handle new signature + +### 2.3 File Discovery Implementation +- [ ] Implement `SetupFileMonitoring()` method in SolutionManager +- [ ] Add solution file monitoring +- [ ] Add project file monitoring for all projects +- [ ] Add source document monitoring for all documents +- [ ] Add `.editorconfig` file monitoring in directory tree +- [ ] Handle null/empty file paths gracefully + +### 2.4 Lazy Reload Integration +- [ ] Modify `ToolHelpers.EnsureSolutionLoaded()` to check reload flag +- [ ] Update all MCP tools to use the helper (verify existing usage) +- [ ] Add appropriate logging when reload is triggered +- [ ] Handle edge cases (deleted files, temporary files) +- [ ] Add timeout/retry logic for reload operations + +### 2.5 Document Operations Integration +- [ ] Modify `DocumentOperationsService.WriteFileAsync()` to accept `IFileChangeVisitor?` +- [ ] Add expected change registration before file write +- [ ] Add git metadata change registration when git enabled +- [ ] Update all callers to pass visitor when available +- [ ] Maintain backward compatibility for existing callers + +### 2.6 High-Level Tool Integration +- [ ] Identify MCP tools that need operation tracking +- [ ] Update `ModificationTools` methods to use visitor pattern +- [ ] Update `DocumentTools` methods to pass visitor to sub-operations +- [ ] Update `AnalysisTools` methods that modify files +- [ ] Add operation tracking to complex tools (FindAndReplace, MoveMember, etc.) +- [ ] Ensure all file modification paths are covered + +## Phase 3: Configuration and Control + +### 3.1 Configuration Options +- [ ] Create `FileMonitoringOptions` class +- [ ] Add configuration properties (enable, extensions, logging) +- [ ] Add options validation +- [ ] Register options in DI container +- [ ] Add appsettings.json configuration section +- [ ] Add XML documentation for configuration + +### 3.2 MCP Tool for Control +- [ ] Create `sharp_configure_file_monitoring` tool +- [ ] Add enable/disable functionality +- [ ] Add status reporting functionality +- [ ] Add statistics reporting (expected vs actual changes) +- [ ] Add tool description and parameter documentation +- [ ] Test tool functionality + +## Phase 4: Testing and Validation + +### 4.1 Unit Tests +- [ ] Test `FileMonitoringService` file detection +- [ ] Test operation tracking and visitor pattern +- [ ] Test expected change validation logic +- [ ] Test external change detection scenarios +- [ ] Test lazy reload logic in SolutionManager +- [ ] Test integration with `ToolHelpers.EnsureSolutionLoaded()` +- [ ] Mock file system changes for reliable testing +- [ ] Test concurrent operation handling +- [ ] Test error recovery scenarios + +### 4.2 Integration Tests +- [ ] Create test solutions with various file types +- [ ] Test lazy reload with real file modifications +- [ ] Test visitor pattern with real MCP operations +- [ ] Test performance impact measurement +- [ ] Test edge cases (network drives, permission issues) +- [ ] Test cross-platform compatibility (Windows, macOS, Linux) +- [ ] Test with large solutions (100+ projects) +- [ ] Test concurrent MCP operations + +### 4.3 Performance Testing +- [ ] Measure solution loading time impact +- [ ] Measure memory usage during long-running monitoring +- [ ] Test with large solutions performance +- [ ] Measure file change detection latency +- [ ] Test FileSystemWatcher resource usage +- [ ] Profile visitor pattern overhead +- [ ] Test lazy reload performance vs immediate reload + +## Documentation and Finalization + +### Documentation Updates +- [ ] Update README.md with file monitoring information +- [ ] Document configuration options +- [ ] Add troubleshooting guide for file monitoring issues +- [ ] Document known limitations (network drives, etc.) +- [ ] Add examples of expected vs external changes + +### Final Validation +- [ ] End-to-end testing with real development workflow +- [ ] Test with VS Code + MCP server combination +- [ ] Test with Visual Studio + MCP server combination +- [ ] Validate all success criteria are met +- [ ] Performance regression testing +- [ ] User acceptance testing + +### Code Review and Cleanup +- [ ] Code review for all modified files +- [ ] Remove any debug logging or test code +- [ ] Ensure consistent error handling patterns +- [ ] Verify thread safety for concurrent operations +- [ ] Check for resource leaks (FileSystemWatcher disposal) +- [ ] Validate XML documentation completeness \ No newline at end of file diff --git a/docs/filemon.design.md b/docs/filemon.design.md new file mode 100644 index 0000000..f2e7c04 --- /dev/null +++ b/docs/filemon.design.md @@ -0,0 +1,149 @@ +# File Monitoring Design for SharpTools MCP Server + +## Overview + +The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It uses a **visitor pattern** to distinguish between self-generated changes (from MCP operations) and external changes (from IDE editing), triggering lazy reloads only when necessary. + +## Core Concepts + +### File Change Visitor Pattern + +Operations use an `IFileChangeVisitor` to register files they expect to modify as they discover them during execution. This solves the problem of predicting file changes upfront, which is difficult with Roslyn's dynamic solution modification process. + +```csharp +// Operation gets visitor and registers expected changes as discovered +var fileChangeVisitor = _fileMonitoring.BeginOperation(operationId); +fileChangeVisitor.ExpectChange("C:\Solution\MyClass.cs"); // As we discover changes +fileChangeVisitor.ExpectChanges(additionalFiles); // Batch registration +``` + +### Lazy Reload Strategy + +Instead of immediately reloading when files change, the system sets an `IsReloadNeeded` flag and reloads only when the next MCP request is made. This provides: + +- **Zero overhead** when no files have changed +- **Natural batching** of multiple file changes +- **User sees delay only when they make a request** after changes + +### External Change Detection + +The system triggers reload only in these scenarios: + +1. **File changes while no operations are active** - Clearly external +2. **Unexpected file changes during operation** - File wasn't in expected list +3. **Multiple changes to same file during operation** - Indicates external interference + +## Architecture Components + +### IFileMonitoringService + +Central service that coordinates file watching and operation tracking: + +```csharp +public interface IFileMonitoringService : IDisposable +{ + // Basic monitoring + void StartMonitoring(string solutionPath); + bool IsReloadNeeded { get; } + void MarkReloadComplete(); + + // Visitor pattern for operations + IFileChangeVisitor BeginOperation(string operationId); + void EndOperation(string operationId); +} +``` + +### IFileChangeVisitor + +Visitor that operations use to register expected file changes: + +```csharp +public interface IFileChangeVisitor +{ + string OperationId { get; } + void ExpectChange(string filePath); + void ExpectChanges(IEnumerable filePaths); + int ExpectedChangeCount { get; } + int ActualChangeCount { get; } +} +``` + +### Integration Points + +**SolutionManager**: Checks `IsReloadNeeded` in `ToolHelpers.EnsureSolutionLoaded()` which all MCP tools use + +**CodeModificationService**: Uses visitor to register Roslyn solution changes as they're discovered: +- Changed documents +- Added documents +- Removed documents +- Git metadata files + +**DocumentOperationsService**: Registers non-Roslyn file operations (like `.editorconfig` edits) + +**MCP Tools**: Can register additional expected changes for complex operations + +## File Discovery + +The system monitors files discovered through Roslyn APIs: + +- **Solution file** (`.sln`) - Main solution file +- **Project files** (`.csproj`, `.vbproj`, `.fsproj`) - All project files in solution +- **Source files** (`.cs`, `.vb`, `.fs`) - All documents in all projects +- **Configuration files** (`.editorconfig`) - Used by EditorConfigProvider + +## Change Detection Logic + +``` +For each file change detected by FileSystemWatcher: +1. Is any operation currently active? + - No → Set IsReloadNeeded = true (external change) + - Yes → Continue to step 2 + +2. Is this file expected by any active operation? + - No → Set IsReloadNeeded = true (unexpected change) + - Yes → Continue to step 3 + +3. Has this file already changed during this operation? + - No → Mark as occurred, continue monitoring + - Yes → Set IsReloadNeeded = true (external interference) +``` + +## Example Scenarios + +### Pure MCP Operation (No Reload) +``` +1. sharp_add_member → BeginOperation with visitor +2. CodeModificationService registers expected file: MyClass.cs +3. File watcher sees MyClass.cs change → Expected, mark as occurred +4. EndOperation → No reload needed ✓ +``` + +### External Change During Operation (Triggers Reload) +``` +1. sharp_add_member → BeginOperation, expects MyClass.cs +2. MyClass.cs changes (expected) → Mark as occurred +3. User edits MyClass.cs again → Same file, 2nd change → IsReloadNeeded = true +4. Next MCP request → Reload triggered ✓ +``` + +### External Change While Idle (Triggers Reload) +``` +1. No operations active +2. User edits MyClass.cs → No operation to check against → IsReloadNeeded = true +3. Next MCP request → Reload triggered ✓ +``` + +## Performance Characteristics + +- **Zero impact** when no files change +- **Minimal overhead** during operations (just registering expected files) +- **FileSystemWatcher efficiency** for change detection +- **Lazy loading** avoids unnecessary work +- **Request-time cost** only when reload actually needed + +## Error Handling Strategy + +- **FileSystemWatcher failures** → Restart monitoring, log errors +- **Operation tracking failures** → Fail safe by assuming external changes +- **Reload failures** → Log error, maintain current state, notify user +- **Unknown file changes** → Treat as potentially external (better safe than sorry) \ No newline at end of file diff --git a/docs/filemon.plan.md b/docs/filemon.plan.md new file mode 100644 index 0000000..80734ed --- /dev/null +++ b/docs/filemon.plan.md @@ -0,0 +1,261 @@ +# File Monitoring Implementation Plan + +## Phase 1: Core File Monitoring Service + +### 1.1 Create IFileMonitoringService Interface +```csharp +public interface IFileMonitoringService : IDisposable +{ + void StartMonitoring(string solutionPath); + void StopMonitoring(); + void AddWatchPath(string filePath); + void RemoveWatchPath(string filePath); + bool IsMonitoring { get; } + bool IsReloadNeeded { get; } + void MarkReloadComplete(); + + // Operation tracking with file change visitor pattern + IFileChangeVisitor BeginOperation(string operationId); + void EndOperation(string operationId); + bool IsOperationInProgress(string operationId); + + // For backward compatibility and simple operations + void RegisterExpectedChange(string operationId, string filePath); + bool IsChangeExpected(string filePath); + void MarkExpectedChangeOccurred(string filePath); +} + +public interface IFileChangeVisitor +{ + string OperationId { get; } + void ExpectChange(string filePath); + void ExpectChanges(IEnumerable filePaths); + bool IsChangeExpected(string filePath); + int ExpectedChangeCount { get; } + int ActualChangeCount { get; } +} +``` + +### 1.2 Implement FileMonitoringService +- Use `FileSystemWatcher` for efficient file system monitoring +- Monitor relevant file extensions: `.sln`, `.csproj`, `.vbproj`, `.fsproj`, `.cs`, `.vb`, `.fs`, `.editorconfig` +- **Expected Change Tracking**: For each operation, track exactly which files are expected to change +- **Precise External Change Detection**: Trigger reload only when: + - File changes and no operation is active, OR + - File changes but it wasn't in the expected files list for current operations, OR + - Same file changes multiple times during a single operation (indicates external interference) +- **Change Accounting**: Track which expected changes have occurred vs. which are still pending +- Handle watcher disposal and error recovery +- Support both directory-based and individual file monitoring + +### 1.3 Add to Dependency Injection +- Register `IFileMonitoringService` as singleton in `ServiceCollectionExtensions.cs` +- Ensure proper disposal through DI container + +## Phase 2: Integration with SolutionManager + +### 2.1 Modify SolutionManager +- Add `IFileMonitoringService` dependency injection +- Add lazy reload check to key operations: + - `EnsureSolutionLoaded()` method checks `fileMonitoring.IsReloadNeeded` + - If reload needed, calls `ReloadSolutionFromDiskAsync()` and `fileMonitoring.MarkReloadComplete()` +- Setup monitoring in `LoadSolutionAsync()` after successful load + +### 2.2 Modify CodeModificationService +- Add `IFileMonitoringService` dependency injection +- Use visitor pattern to dynamically register expected file changes as they're discovered: + ```csharp + public async Task ApplyChangesAsync(Solution newSolution, CancellationToken cancellationToken, string commitMessage, IEnumerable? additionalFilePaths = null) + { + var operationId = Guid.NewGuid().ToString(); + var fileChangeVisitor = _fileMonitoring.BeginOperation(operationId); + + try + { + var originalSolution = _solutionManager.CurrentSolution!; + var solutionChanges = newSolution.GetChanges(originalSolution); + + // Register expected changes as we discover them during processing + foreach (var projectChange in solutionChanges.GetProjectChanges()) + { + // Register changed documents + foreach (var changedDocumentId in projectChange.GetChangedDocuments()) + { + var document = newSolution.GetDocument(changedDocumentId); + if (document?.FilePath != null) + { + fileChangeVisitor.ExpectChange(document.FilePath); + _logger.LogTrace("Expecting change to: {FilePath}", document.FilePath); + } + } + + // Register added documents + foreach (var addedDocumentId in projectChange.GetAddedDocuments()) + { + var document = newSolution.GetDocument(addedDocumentId); + if (document?.FilePath != null) + { + fileChangeVisitor.ExpectChange(document.FilePath); + _logger.LogTrace("Expecting new file: {FilePath}", document.FilePath); + } + } + + // Register removed documents + foreach (var removedDocumentId in projectChange.GetRemovedDocuments()) + { + var document = originalSolution.GetDocument(removedDocumentId); + if (document?.FilePath != null) + { + fileChangeVisitor.ExpectChange(document.FilePath); + _logger.LogTrace("Expecting removal of: {FilePath}", document.FilePath); + } + } + } + + // Register additional files (non-code files from tools like FindAndReplace) + if (additionalFilePaths != null) + { + fileChangeVisitor.ExpectChanges(additionalFilePaths); + } + + // Apply changes to workspace - this triggers the actual file writes + if (workspace.TryApplyChanges(finalSolutionToApply)) + { + _solutionManager.RefreshCurrentSolution(); + } + + // Register git files if git operations enabled + if (_gitEnabled && !string.IsNullOrEmpty(commitMessage)) + { + fileChangeVisitor.ExpectChange(".git/index"); + fileChangeVisitor.ExpectChange(".git/logs/HEAD"); + fileChangeVisitor.ExpectChange(".git/COMMIT_EDITMSG"); + await CommitChangesIfEnabled(commitMessage, cancellationToken); + } + } + finally + { + _fileMonitoring.EndOperation(operationId); + _logger.LogDebug("Operation {OperationId} completed. Expected: {Expected}, Actual: {Actual}", + operationId, fileChangeVisitor.ExpectedChangeCount, fileChangeVisitor.ActualChangeCount); + } + } + ``` + +### 2.3 File Discovery Integration +```csharp +private void SetupFileMonitoring() +{ + // Monitor solution file + _fileMonitoring.AddWatchPath(_currentSolution.FilePath); + + // Monitor all project files + foreach (var project in _currentSolution.Projects) + { + if (!string.IsNullOrEmpty(project.FilePath)) + _fileMonitoring.AddWatchPath(project.FilePath); + + // Monitor all source documents + foreach (var document in project.Documents) + { + if (!string.IsNullOrEmpty(document.FilePath)) + _fileMonitoring.AddWatchPath(document.FilePath); + } + } + + // Monitor .editorconfig files in solution directory tree + MonitorEditorConfigFiles(Path.GetDirectoryName(_currentSolution.FilePath)); +} +``` + +### 2.4 Lazy Reload Integration +- Add reload check to `ToolHelpers.EnsureSolutionLoaded()` method +- All MCP tools use this helper, ensuring automatic reload when needed +- Add logging when reload is triggered +- Handle edge cases (file deletion, temporary files, etc.) + +### 2.5 Integration with Document Operations +- Modify `DocumentOperationsService.WriteFileAsync()` to accept file change visitor for tracking: + ```csharp + public async Task WriteFileAsync(string filePath, string content, bool overwrite, + CancellationToken cancellationToken, string commitMessage, IFileChangeVisitor? fileChangeVisitor = null) + { + // Register expected change just before performing the write + fileChangeVisitor?.ExpectChange(filePath); + + // If git is enabled, also expect git metadata changes + if (_gitEnabled && !string.IsNullOrEmpty(commitMessage)) + { + fileChangeVisitor?.ExpectChange(".git/index"); + fileChangeVisitor?.ExpectChange(".git/logs/HEAD"); + } + + // ... perform file write + } + ``` + +### 2.6 High-Level Tool Integration +- Modify MCP tools to pass file change visitor to sub-operations: + ```csharp + [McpServerTool(Name = "sharp_find_and_replace")] + public static async Task FindAndReplace(/*...*/) + { + var operationId = Guid.NewGuid().ToString(); + var fileChangeVisitor = fileMonitoring.BeginOperation(operationId); + + try + { + // Tools can register expected changes as they discover them + if (isNonCodeFileOperation) + { + fileChangeVisitor.ExpectChange(targetFilePath); + } + + await modificationService.ApplyChangesAsync(newSolution, cancellationToken, commitMessage); + // ApplyChangesAsync will register its own expected changes via the fileChangeVisitor + } + finally + { + fileMonitoring.EndOperation(operationId); + } + } + ``` + +## Phase 3: Configuration and Control + +### 3.1 Add Configuration Options +```csharp +public class FileMonitoringOptions +{ + public bool EnableFileMonitoring { get; set; } = true; + public string[] MonitoredExtensions { get; set; } = { ".cs", ".vb", ".fs", ".sln", ".csproj", ".vbproj", ".fsproj", ".editorconfig" }; + public bool LogFileChanges { get; set; } = true; +} +``` + +### 3.2 Add MCP Tool for Control +```csharp +[McpServerTool(Name = "sharp_configure_file_monitoring")] +public static object ConfigureFileMonitoring( + ISolutionManager solutionManager, + [Description("Enable or disable automatic file monitoring")] bool enabled) +``` + +## Phase 4: Testing and Validation + +### 4.1 Unit Tests +- Test FileMonitoringService file detection and flag setting +- Test lazy reload logic in SolutionManager +- Test integration with ToolHelpers.EnsureSolutionLoaded() +- Mock file system changes for reliable testing + +### 4.2 Integration Tests +- Test with real solution files +- Verify lazy reload triggered on MCP requests after file changes +- Test performance impact of monitoring large solutions +- Test edge cases (network drives, permission issues) + +### 4.3 Performance Testing +- Measure impact on solution loading time +- Test with large solutions (100+ projects) +- Memory usage analysis for long-running monitoring \ No newline at end of file From ccf860a6e8e4bd3d8e99c562eb2168a0d35b5420 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Mon, 15 Sep 2025 20:50:08 -0300 Subject: [PATCH 02/12] add a test project --- .../SharpTools.Tools.Tests.csproj | 21 +++++++++++++++++++ SharpTools.Tools.Tests/UnitTest1.cs | 10 +++++++++ 2 files changed, 31 insertions(+) create mode 100644 SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj create mode 100644 SharpTools.Tools.Tests/UnitTest1.cs diff --git a/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj new file mode 100644 index 0000000..e1b74a9 --- /dev/null +++ b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj @@ -0,0 +1,21 @@ + + + + net8.0 + enable + enable + false + + + + + + + + + + + + + + diff --git a/SharpTools.Tools.Tests/UnitTest1.cs b/SharpTools.Tools.Tests/UnitTest1.cs new file mode 100644 index 0000000..c86fa18 --- /dev/null +++ b/SharpTools.Tools.Tests/UnitTest1.cs @@ -0,0 +1,10 @@ +namespace SharpTools.Tools.Tests; + +public class UnitTest1 +{ + [Fact] + public void Test1() + { + + } +} From 72f0ae7e4014fb34804af4198cc29f2059ff9c16 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Mon, 15 Sep 2025 21:41:39 -0300 Subject: [PATCH 03/12] update filemon plans (was using Gemini 2.5 pro, hit free limit for the day) --- docs/filemon.checklist.md | 154 -------------------------------------- docs/filemon.design.md | 54 +++++++++---- docs/filemon.plan.md | 95 +++++++++++++++-------- 3 files changed, 106 insertions(+), 197 deletions(-) delete mode 100644 docs/filemon.checklist.md diff --git a/docs/filemon.checklist.md b/docs/filemon.checklist.md deleted file mode 100644 index de1f0cd..0000000 --- a/docs/filemon.checklist.md +++ /dev/null @@ -1,154 +0,0 @@ -# File Monitoring Implementation Checklist - -## Phase 1: Core File Monitoring Service - -### 1.1 Interface Creation -- [ ] Create `IFileMonitoringService` interface -- [ ] Create `IFileChangeVisitor` interface -- [ ] Add interfaces to appropriate namespace -- [ ] Add XML documentation for public APIs - -### 1.2 FileMonitoringService Implementation -- [ ] Create `FileMonitoringService` class implementing `IFileMonitoringService` -- [ ] Implement `FileSystemWatcher` setup and disposal -- [ ] Add support for monitored file extensions (`.sln`, `.csproj`, `.vbproj`, `.fsproj`, `.cs`, `.vb`, `.fs`, `.editorconfig`) -- [ ] Implement operation tracking with concurrent operation support -- [ ] Create `FileChangeVisitor` class implementing `IFileChangeVisitor` -- [ ] Implement expected change tracking and validation logic -- [ ] Add external change detection logic (3 scenarios) -- [ ] Implement change accounting and statistics -- [ ] Add error recovery for `FileSystemWatcher` failures -- [ ] Add logging for all file change events -- [ ] Handle edge cases (temporary files, permission issues) - -### 1.3 Dependency Injection Setup -- [ ] Register `IFileMonitoringService` as singleton in `ServiceCollectionExtensions.cs` -- [ ] Ensure proper disposal through DI container -- [ ] Add configuration binding for `FileMonitoringOptions` - -## Phase 2: Integration with SolutionManager - -### 2.1 SolutionManager Modifications -- [ ] Add `IFileMonitoringService` dependency injection to constructor -- [ ] Modify `ToolHelpers.EnsureSolutionLoaded()` to check `IsReloadNeeded` -- [ ] Add reload logic calling `ReloadSolutionFromDiskAsync()` when needed -- [ ] Call `MarkReloadComplete()` after successful reload -- [ ] Add file monitoring setup in `LoadSolutionAsync()` -- [ ] Add logging for reload triggers - -### 2.2 CodeModificationService Modifications -- [ ] Add `IFileMonitoringService` dependency injection to constructor -- [ ] Modify `ApplyChangesAsync()` signature to use visitor pattern -- [ ] Add operation begin/end logic with `fileChangeVisitor` -- [ ] Implement dynamic change registration for changed documents -- [ ] Implement dynamic change registration for added documents -- [ ] Implement dynamic change registration for removed documents -- [ ] Add support for additional file paths (non-code files) -- [ ] Add git file registration for git operations -- [ ] Add operation completion logging with statistics -- [ ] Update all callers to handle new signature - -### 2.3 File Discovery Implementation -- [ ] Implement `SetupFileMonitoring()` method in SolutionManager -- [ ] Add solution file monitoring -- [ ] Add project file monitoring for all projects -- [ ] Add source document monitoring for all documents -- [ ] Add `.editorconfig` file monitoring in directory tree -- [ ] Handle null/empty file paths gracefully - -### 2.4 Lazy Reload Integration -- [ ] Modify `ToolHelpers.EnsureSolutionLoaded()` to check reload flag -- [ ] Update all MCP tools to use the helper (verify existing usage) -- [ ] Add appropriate logging when reload is triggered -- [ ] Handle edge cases (deleted files, temporary files) -- [ ] Add timeout/retry logic for reload operations - -### 2.5 Document Operations Integration -- [ ] Modify `DocumentOperationsService.WriteFileAsync()` to accept `IFileChangeVisitor?` -- [ ] Add expected change registration before file write -- [ ] Add git metadata change registration when git enabled -- [ ] Update all callers to pass visitor when available -- [ ] Maintain backward compatibility for existing callers - -### 2.6 High-Level Tool Integration -- [ ] Identify MCP tools that need operation tracking -- [ ] Update `ModificationTools` methods to use visitor pattern -- [ ] Update `DocumentTools` methods to pass visitor to sub-operations -- [ ] Update `AnalysisTools` methods that modify files -- [ ] Add operation tracking to complex tools (FindAndReplace, MoveMember, etc.) -- [ ] Ensure all file modification paths are covered - -## Phase 3: Configuration and Control - -### 3.1 Configuration Options -- [ ] Create `FileMonitoringOptions` class -- [ ] Add configuration properties (enable, extensions, logging) -- [ ] Add options validation -- [ ] Register options in DI container -- [ ] Add appsettings.json configuration section -- [ ] Add XML documentation for configuration - -### 3.2 MCP Tool for Control -- [ ] Create `sharp_configure_file_monitoring` tool -- [ ] Add enable/disable functionality -- [ ] Add status reporting functionality -- [ ] Add statistics reporting (expected vs actual changes) -- [ ] Add tool description and parameter documentation -- [ ] Test tool functionality - -## Phase 4: Testing and Validation - -### 4.1 Unit Tests -- [ ] Test `FileMonitoringService` file detection -- [ ] Test operation tracking and visitor pattern -- [ ] Test expected change validation logic -- [ ] Test external change detection scenarios -- [ ] Test lazy reload logic in SolutionManager -- [ ] Test integration with `ToolHelpers.EnsureSolutionLoaded()` -- [ ] Mock file system changes for reliable testing -- [ ] Test concurrent operation handling -- [ ] Test error recovery scenarios - -### 4.2 Integration Tests -- [ ] Create test solutions with various file types -- [ ] Test lazy reload with real file modifications -- [ ] Test visitor pattern with real MCP operations -- [ ] Test performance impact measurement -- [ ] Test edge cases (network drives, permission issues) -- [ ] Test cross-platform compatibility (Windows, macOS, Linux) -- [ ] Test with large solutions (100+ projects) -- [ ] Test concurrent MCP operations - -### 4.3 Performance Testing -- [ ] Measure solution loading time impact -- [ ] Measure memory usage during long-running monitoring -- [ ] Test with large solutions performance -- [ ] Measure file change detection latency -- [ ] Test FileSystemWatcher resource usage -- [ ] Profile visitor pattern overhead -- [ ] Test lazy reload performance vs immediate reload - -## Documentation and Finalization - -### Documentation Updates -- [ ] Update README.md with file monitoring information -- [ ] Document configuration options -- [ ] Add troubleshooting guide for file monitoring issues -- [ ] Document known limitations (network drives, etc.) -- [ ] Add examples of expected vs external changes - -### Final Validation -- [ ] End-to-end testing with real development workflow -- [ ] Test with VS Code + MCP server combination -- [ ] Test with Visual Studio + MCP server combination -- [ ] Validate all success criteria are met -- [ ] Performance regression testing -- [ ] User acceptance testing - -### Code Review and Cleanup -- [ ] Code review for all modified files -- [ ] Remove any debug logging or test code -- [ ] Ensure consistent error handling patterns -- [ ] Verify thread safety for concurrent operations -- [ ] Check for resource leaks (FileSystemWatcher disposal) -- [ ] Validate XML documentation completeness \ No newline at end of file diff --git a/docs/filemon.design.md b/docs/filemon.design.md index f2e7c04..6b4482a 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -2,19 +2,26 @@ ## Overview -The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It uses a **visitor pattern** to distinguish between self-generated changes (from MCP operations) and external changes (from IDE editing), triggering lazy reloads only when necessary. +The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It monitors all solution files for changes while ignoring expected changes. ## Core Concepts -### File Change Visitor Pattern +### Comprehensive file monitoring -Operations use an `IFileChangeVisitor` to register files they expect to modify as they discover them during execution. This solves the problem of predicting file changes upfront, which is difficult with Roslyn's dynamic solution modification process. +The file monitoring service is starts watching the solution folder recursively before the solution build starts. Once the +build is complete information from Roslyn is used to control what files can actually trigger a reload. All MCP operations +will check if a reload is needed before continuing their operation. + +### Tracking Expected Changes To Avoid Reloading + +Operations use an `IFileChangeVisitor` to register files they expect to modify as they discover them during execution. This allows file monitoring to ignore expected changes and prevent unnecessary reloads. ```csharp // Operation gets visitor and registers expected changes as discovered var fileChangeVisitor = _fileMonitoring.BeginOperation(operationId); fileChangeVisitor.ExpectChange("C:\Solution\MyClass.cs"); // As we discover changes fileChangeVisitor.ExpectChanges(additionalFiles); // Batch registration +fileChangeVisitor.EndOperation(); ``` ### Lazy Reload Strategy @@ -49,7 +56,6 @@ public interface IFileMonitoringService : IDisposable // Visitor pattern for operations IFileChangeVisitor BeginOperation(string operationId); - void EndOperation(string operationId); } ``` @@ -63,6 +69,7 @@ public interface IFileChangeVisitor string OperationId { get; } void ExpectChange(string filePath); void ExpectChanges(IEnumerable filePaths); + void EndOperation(); int ExpectedChangeCount { get; } int ActualChangeCount { get; } } @@ -70,7 +77,7 @@ public interface IFileChangeVisitor ### Integration Points -**SolutionManager**: Checks `IsReloadNeeded` in `ToolHelpers.EnsureSolutionLoaded()` which all MCP tools use +**SolutionManager**: Checks `IsReloadNeeded` in `ToolHelpers.EnsureSolutionLoaded()` and `ToolHelpers.EnsureSolutionLoadedWithDetails()` which all MCP tools use **CodeModificationService**: Uses visitor to register Roslyn solution changes as they're discovered: - Changed documents @@ -82,18 +89,39 @@ public interface IFileChangeVisitor **MCP Tools**: Can register additional expected changes for complex operations -## File Discovery +## File Discovery and Monitoring Strategy + +To simplify monitoring, the system watches the entire solution's root directory recursively. This automatically handles file additions and deletions without complex tracking. + +The monitoring is made efficient and relevant by filtering events against the actual files that make up the solution: + +1. **Directory Watching**: A single `FileSystemWatcher` is configured to monitor the solution's root directory and all its subdirectories for any file changes. +2. **Known File Set**: On solution load, the `SolutionManager` inspects the Roslyn workspace to get a complete list of all file paths that are part of the solution. This includes project files, source documents, additional files, and analyzer configuration files (like `.editorconfig`). This list is stored in a fast-lookup data structure like a `HashSet`. +3. **Event Filtering**: When the `FileSystemWatcher` raises a file change event, the `FileMonitoringService` checks if the full path of the changed file exists in the set of known solution files. Changes to files not in this set are ignored. + +This approach ensures that the system only reacts to changes in files it genuinely cares about, providing a balance of simplicity (one watcher) and precision (filtering by exact file path). -The system monitors files discovered through Roslyn APIs: +## Handling Race Conditions on Startup -- **Solution file** (`.sln`) - Main solution file -- **Project files** (`.csproj`, `.vbproj`, `.fsproj`) - All project files in solution -- **Source files** (`.cs`, `.vb`, `.fs`) - All documents in all projects -- **Configuration files** (`.editorconfig`) - Used by EditorConfigProvider +A potential race condition exists where a file could be modified by an external process after the application starts but before the Roslyn solution has been fully loaded and analyzed. To solve this, the monitoring service uses a two-phase initialization: + +1. **Early Monitoring**: The `FileMonitoringService` starts watching the solution directory for all changes *immediately* on application startup, even before it knows which files are part of the solution. It records any file change events, including the file path and the time of the change, into a temporary backlog. + +2. **Late Binding of Known Files**: After the `SolutionManager` has finished loading the solution, it provides the `FileMonitoringService` with the definitive set of "known" solution file paths. + +3. **Backlog Reconciliation**: Upon receiving the set of known files, the service reconciles its backlog of recorded changes. If any of the newly provided "known" files appear in the backlog, it means they were changed during the startup process. In this case, the `IsReloadNeeded` flag is immediately set to `true` to force a workspace reload on the next request, ensuring synchronization. + +This approach closes the gap and guarantees that any changes occurring during the solution load process are correctly detected. ## Change Detection Logic -``` +### Initial Reconciliation (when known files are set) +For each file in the provided set of known solution files: +1. Does this file exist in the backlog of changes recorded since monitoring began? + - Yes -> Set IsReloadNeeded = true, and the backlog can be cleared. + - No -> Continue. + +### Real-time Change Detection (after initialization) For each file change detected by FileSystemWatcher: 1. Is any operation currently active? - No → Set IsReloadNeeded = true (external change) @@ -115,7 +143,7 @@ For each file change detected by FileSystemWatcher: 1. sharp_add_member → BeginOperation with visitor 2. CodeModificationService registers expected file: MyClass.cs 3. File watcher sees MyClass.cs change → Expected, mark as occurred -4. EndOperation → No reload needed ✓ +4. visitor.EndOperation() → No reload needed ✓ ``` ### External Change During Operation (Triggers Reload) diff --git a/docs/filemon.plan.md b/docs/filemon.plan.md index 80734ed..4c86abd 100644 --- a/docs/filemon.plan.md +++ b/docs/filemon.plan.md @@ -6,17 +6,20 @@ ```csharp public interface IFileMonitoringService : IDisposable { - void StartMonitoring(string solutionPath); + // Phase 1: Start monitoring immediately on startup. + void StartMonitoring(string directory); void StopMonitoring(); - void AddWatchPath(string filePath); - void RemoveWatchPath(string filePath); + + // Phase 2: Provide the set of files to watch after solution load. + // This will reconcile any changes that happened in the interim. + void SetKnownFilePaths(ISet filePathsToWatch); + bool IsMonitoring { get; } bool IsReloadNeeded { get; } void MarkReloadComplete(); // Operation tracking with file change visitor pattern IFileChangeVisitor BeginOperation(string operationId); - void EndOperation(string operationId); bool IsOperationInProgress(string operationId); // For backward compatibility and simple operations @@ -30,6 +33,7 @@ public interface IFileChangeVisitor string OperationId { get; } void ExpectChange(string filePath); void ExpectChanges(IEnumerable filePaths); + void EndOperation(); bool IsChangeExpected(string filePath); int ExpectedChangeCount { get; } int ActualChangeCount { get; } @@ -37,16 +41,15 @@ public interface IFileChangeVisitor ``` ### 1.2 Implement FileMonitoringService -- Use `FileSystemWatcher` for efficient file system monitoring -- Monitor relevant file extensions: `.sln`, `.csproj`, `.vbproj`, `.fsproj`, `.cs`, `.vb`, `.fs`, `.editorconfig` -- **Expected Change Tracking**: For each operation, track exactly which files are expected to change -- **Precise External Change Detection**: Trigger reload only when: - - File changes and no operation is active, OR - - File changes but it wasn't in the expected files list for current operations, OR - - Same file changes multiple times during a single operation (indicates external interference) -- **Change Accounting**: Track which expected changes have occurred vs. which are still pending -- Handle watcher disposal and error recovery -- Support both directory-based and individual file monitoring +- On `StartMonitoring`, use a `FileSystemWatcher` to watch the root directory recursively. Record all file change events (path and timestamp) into a temporary, thread-safe backlog. +- On `SetKnownFilePaths`: + - Atomically replace the internal set of known file paths. + - Process the backlog of changes. For each change, if the file path is in the new set of known files, set `IsReloadNeeded = true` and stop processing the backlog. + - Clear the backlog. +- For new events arriving after `SetKnownFilePaths` has been called: + - Check if the file path is in the known set. If not, ignore it. + - If it is a known file, proceed with the existing logic (check for active operations, etc.). +- Maintain thread-safe access to the backlog, the known file set, and the `IsReloadNeeded` flag. ### 1.3 Add to Dependency Injection - Register `IFileMonitoringService` as singleton in `ServiceCollectionExtensions.cs` @@ -55,11 +58,12 @@ public interface IFileChangeVisitor ## Phase 2: Integration with SolutionManager ### 2.1 Modify SolutionManager -- Add `IFileMonitoringService` dependency injection +- In `LoadSolutionAsync`, call `_fileMonitoring.StartMonitoring(solutionDirectory)` *before* beginning the Roslyn solution load. +- After the solution is loaded successfully, gather the list of file paths. +- Call `_fileMonitoring.SetKnownFilePaths(solutionFilePaths)` to complete the setup. - Add lazy reload check to key operations: - `EnsureSolutionLoaded()` method checks `fileMonitoring.IsReloadNeeded` - If reload needed, calls `ReloadSolutionFromDiskAsync()` and `fileMonitoring.MarkReloadComplete()` -- Setup monitoring in `LoadSolutionAsync()` after successful load ### 2.2 Modify CodeModificationService - Add `IFileMonitoringService` dependency injection @@ -135,36 +139,68 @@ public interface IFileChangeVisitor } finally { - _fileMonitoring.EndOperation(operationId); + fileChangeVisitor.EndOperation(); _logger.LogDebug("Operation {OperationId} completed. Expected: {Expected}, Actual: {Actual}", operationId, fileChangeVisitor.ExpectedChangeCount, fileChangeVisitor.ActualChangeCount); } } ``` -### 2.3 File Discovery Integration +### 2.3 Finalizing Monitor Configuration + +After the solution is loaded, the `SolutionManager` provides the file monitoring service with the definitive list of files to watch. The service can then reconcile any changes that occurred during the load process. + +A conceptual example of the `SolutionManager` logic: ```csharp -private void SetupFileMonitoring() +public async Task LoadSolutionAsync(string solutionPath) +{ + var solutionDirectory = Path.GetDirectoryName(solutionPath); + _fileMonitoring.StartMonitoring(solutionDirectory); // Start watching immediately + + // ... proceed to load the solution with Roslyn ... + _currentSolution = await workspace.OpenSolutionAsync(solutionPath); + + FinalizeFileMonitoring(); // Now provide the list of files +} + +private void FinalizeFileMonitoring() { - // Monitor solution file - _fileMonitoring.AddWatchPath(_currentSolution.FilePath); + var solution = _solutionManager.CurrentSolution; - // Monitor all project files - foreach (var project in _currentSolution.Projects) + // 1. Discover all file paths from the Roslyn solution + var solutionFilePaths = new HashSet(StringComparer.OrdinalIgnoreCase); + if (!string.IsNullOrEmpty(solution.FilePath)) + { + solutionFilePaths.Add(solution.FilePath); + } + + foreach (var project in solution.Projects) { if (!string.IsNullOrEmpty(project.FilePath)) - _fileMonitoring.AddWatchPath(project.FilePath); + solutionFilePaths.Add(project.FilePath); - // Monitor all source documents foreach (var document in project.Documents) { if (!string.IsNullOrEmpty(document.FilePath)) - _fileMonitoring.AddWatchPath(document.FilePath); + solutionFilePaths.Add(document.FilePath); + } + + // Also include AdditionalDocuments, AnalyzerConfigDocuments, etc. + foreach (var additionalDoc in project.AdditionalDocuments) + { + if (!string.IsNullOrEmpty(additionalDoc.FilePath)) + solutionFilePaths.Add(additionalDoc.FilePath); + } + foreach (var configDoc in project.AnalyzerConfigDocuments) + { + if (!string.IsNullOrEmpty(configDoc.FilePath)) + solutionFilePaths.Add(configDoc.FilePath); } } - // Monitor .editorconfig files in solution directory tree - MonitorEditorConfigFiles(Path.GetDirectoryName(_currentSolution.FilePath)); + // 2. Provide the definitive set of files to the monitoring service. + _fileMonitoring.SetKnownFilePaths(solutionFilePaths); + _logger.LogInformation("File monitoring is now active for {FileCount} files in the solution.", solutionFilePaths.Count); } ``` @@ -216,7 +252,7 @@ private void SetupFileMonitoring() } finally { - fileMonitoring.EndOperation(operationId); + fileChangeVisitor.EndOperation(operationId); } } ``` @@ -228,7 +264,6 @@ private void SetupFileMonitoring() public class FileMonitoringOptions { public bool EnableFileMonitoring { get; set; } = true; - public string[] MonitoredExtensions { get; set; } = { ".cs", ".vb", ".fs", ".sln", ".csproj", ".vbproj", ".fsproj", ".editorconfig" }; public bool LogFileChanges { get; set; } = true; } ``` From f22f09f764b91e1522d163ff030b1a3c1ea19139 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Mon, 15 Sep 2025 21:50:30 -0300 Subject: [PATCH 04/12] cleanup design and plan --- docs/filemon.design.md | 18 ------------------ docs/filemon.plan.md | 6 +----- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/docs/filemon.design.md b/docs/filemon.design.md index 6b4482a..3992044 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -24,14 +24,6 @@ fileChangeVisitor.ExpectChanges(additionalFiles); // Batch registration fileChangeVisitor.EndOperation(); ``` -### Lazy Reload Strategy - -Instead of immediately reloading when files change, the system sets an `IsReloadNeeded` flag and reloads only when the next MCP request is made. This provides: - -- **Zero overhead** when no files have changed -- **Natural batching** of multiple file changes -- **User sees delay only when they make a request** after changes - ### External Change Detection The system triggers reload only in these scenarios: @@ -70,8 +62,6 @@ public interface IFileChangeVisitor void ExpectChange(string filePath); void ExpectChanges(IEnumerable filePaths); void EndOperation(); - int ExpectedChangeCount { get; } - int ActualChangeCount { get; } } ``` @@ -161,14 +151,6 @@ For each file change detected by FileSystemWatcher: 3. Next MCP request → Reload triggered ✓ ``` -## Performance Characteristics - -- **Zero impact** when no files change -- **Minimal overhead** during operations (just registering expected files) -- **FileSystemWatcher efficiency** for change detection -- **Lazy loading** avoids unnecessary work -- **Request-time cost** only when reload actually needed - ## Error Handling Strategy - **FileSystemWatcher failures** → Restart monitoring, log errors diff --git a/docs/filemon.plan.md b/docs/filemon.plan.md index 4c86abd..068b3ec 100644 --- a/docs/filemon.plan.md +++ b/docs/filemon.plan.md @@ -35,8 +35,6 @@ public interface IFileChangeVisitor void ExpectChanges(IEnumerable filePaths); void EndOperation(); bool IsChangeExpected(string filePath); - int ExpectedChangeCount { get; } - int ActualChangeCount { get; } } ``` @@ -116,8 +114,8 @@ public interface IFileChangeVisitor } } - // Register additional files (non-code files from tools like FindAndReplace) if (additionalFilePaths != null) + // Register additional files (non-code files from tools like FindAndReplace) { fileChangeVisitor.ExpectChanges(additionalFilePaths); } @@ -140,8 +138,6 @@ public interface IFileChangeVisitor finally { fileChangeVisitor.EndOperation(); - _logger.LogDebug("Operation {OperationId} completed. Expected: {Expected}, Actual: {Actual}", - operationId, fileChangeVisitor.ExpectedChangeCount, fileChangeVisitor.ActualChangeCount); } } ``` From dae135f3d534f73de835523d890e40f578158ba5 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Mon, 15 Sep 2025 22:28:49 -0300 Subject: [PATCH 05/12] cleanup design for error handling --- docs/filemon.design.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/docs/filemon.design.md b/docs/filemon.design.md index 3992044..a398f1d 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -153,7 +153,4 @@ For each file change detected by FileSystemWatcher: ## Error Handling Strategy -- **FileSystemWatcher failures** → Restart monitoring, log errors -- **Operation tracking failures** → Fail safe by assuming external changes -- **Reload failures** → Log error, maintain current state, notify user -- **Unknown file changes** → Treat as potentially external (better safe than sorry) \ No newline at end of file +- **FileSystemWatcher failures** → Stop monitoring and set IsReloadNeeded = true \ No newline at end of file From bc9ff6ece331360714965808105f1851ea534e6f Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 13:30:14 -0300 Subject: [PATCH 06/12] trying to hook up a no-op IFileMonitoringService to clarify how it would connect to the system --- .../Extensions/ServiceCollectionExtensions.cs | 6 +- .../Interfaces/IFileMonitoringService.cs | 36 +++ .../Interfaces/ISolutionManager.cs | 1 + SharpTools.Tools/Mcp/ToolHelpers.cs | 9 +- SharpTools.Tools/Mcp/Tools/AnalysisTools.cs | 24 +- SharpTools.Tools/Mcp/Tools/DocumentTools.cs | 8 +- .../Mcp/Tools/ModificationTools.cs | 14 +- SharpTools.Tools/Mcp/Tools/SolutionTools.cs | 4 +- .../Services/DocumentOperationsService.cs | 11 +- .../Services/FileMonitoringService.cs | 48 +++ .../Services/SemanticSimilarityService.cs | 10 +- SharpTools.Tools/Services/SolutionManager.cs | 64 +++- docs/filemon.design.md | 118 ++----- docs/filemon.plan.md | 292 ------------------ 14 files changed, 218 insertions(+), 427 deletions(-) create mode 100644 SharpTools.Tools/Interfaces/IFileMonitoringService.cs create mode 100644 SharpTools.Tools/Services/FileMonitoringService.cs delete mode 100644 docs/filemon.plan.md diff --git a/SharpTools.Tools/Extensions/ServiceCollectionExtensions.cs b/SharpTools.Tools/Extensions/ServiceCollectionExtensions.cs index 4a7a509..c3fb510 100644 --- a/SharpTools.Tools/Extensions/ServiceCollectionExtensions.cs +++ b/SharpTools.Tools/Extensions/ServiceCollectionExtensions.cs @@ -17,10 +17,11 @@ public static class ServiceCollectionExtensions { /// The service collection for chaining. public static IServiceCollection WithSharpToolsServices(this IServiceCollection services, bool enableGit = true, string? buildConfiguration = null) { services.AddSingleton(); - services.AddSingleton(sp => + services.AddSingleton(sp => new SolutionManager( - sp.GetRequiredService>(), + sp.GetRequiredService>(), sp.GetRequiredService(), + sp.GetRequiredService(), buildConfiguration ) ); @@ -36,6 +37,7 @@ public static IServiceCollection WithSharpToolsServices(this IServiceCollection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); return services; } diff --git a/SharpTools.Tools/Interfaces/IFileMonitoringService.cs b/SharpTools.Tools/Interfaces/IFileMonitoringService.cs new file mode 100644 index 0000000..08ddbbe --- /dev/null +++ b/SharpTools.Tools/Interfaces/IFileMonitoringService.cs @@ -0,0 +1,36 @@ +namespace SharpTools.Tools.Interfaces; + +/// +/// Service for monitoring file changes and coordinating with MCP operations to detect external modifications. +/// +public interface IFileMonitoringService : IDisposable +{ + /// + /// Start monitoring a directory for file changes. + /// + /// Directory to monitor recursively + void StartMonitoring(string directory); + + /// + /// Stop monitoring. + /// + void StopMonitoring(); + + /// + /// Provide the set of files to watch after solution load. + /// This will reconcile any changes that happened during startup. + /// + /// Set of file paths that are part of the solution + void SetKnownFilePaths(ISet filePathsToWatch); + + /// + /// Whether a solution reload is needed due to external file changes. + /// + bool IsReloadNeeded { get; } + + /// + /// Register an expected file change for backward compatibility. + /// + /// File path that will be modified + void RegisterExpectedChange(string filePath); +} \ No newline at end of file diff --git a/SharpTools.Tools/Interfaces/ISolutionManager.cs b/SharpTools.Tools/Interfaces/ISolutionManager.cs index 0ac3e50..9b5102e 100644 --- a/SharpTools.Tools/Interfaces/ISolutionManager.cs +++ b/SharpTools.Tools/Interfaces/ISolutionManager.cs @@ -20,4 +20,5 @@ public interface ISolutionManager : IDisposable { Task GetCompilationAsync(ProjectId projectId, CancellationToken cancellationToken); Task ReloadSolutionFromDiskAsync(CancellationToken cancellationToken); void RefreshCurrentSolution(); + Task ReloadSolutionIfChangedExternallyAsync(CancellationToken cancellationToken); } \ No newline at end of file diff --git a/SharpTools.Tools/Mcp/ToolHelpers.cs b/SharpTools.Tools/Mcp/ToolHelpers.cs index b90707f..324baf7 100644 --- a/SharpTools.Tools/Mcp/ToolHelpers.cs +++ b/SharpTools.Tools/Mcp/ToolHelpers.cs @@ -1,6 +1,7 @@ using System.Text.Encodings.Web; using System.Text.Json.Serialization; using ModelContextProtocol; +using SharpTools.Tools.Interfaces; using SharpTools.Tools.Services; namespace SharpTools.Tools.Mcp; @@ -8,20 +9,24 @@ namespace SharpTools.Tools.Mcp; internal static class ToolHelpers { public const string SharpToolPrefix = "SharpTool_"; - public static void EnsureSolutionLoaded(ISolutionManager solutionManager) { + public static async Task EnsureSolutionLoaded(ISolutionManager solutionManager, CancellationToken cancellationToken) { if (!solutionManager.IsSolutionLoaded) { throw new McpException($"No solution is currently loaded. Please use '{SharpToolPrefix}{nameof(Tools.SolutionTools.LoadSolution)}' first."); } + + await solutionManager.ReloadSolutionIfChangedExternallyAsync(cancellationToken); } /// /// Safely ensures that a solution is loaded, with detailed error information. /// - public static void EnsureSolutionLoadedWithDetails(ISolutionManager solutionManager, ILogger logger, string operationName) { + public static async Task EnsureSolutionLoadedWithDetails(ISolutionManager solutionManager, ILogger logger, string operationName, CancellationToken cancellationToken) { if (!solutionManager.IsSolutionLoaded) { logger.LogError("Attempted to execute {Operation} without a loaded solution", operationName); throw new McpException($"No solution is currently loaded. Please use '{SharpToolPrefix}{nameof(Tools.SolutionTools.LoadSolution)}' before calling '{operationName}'."); } + + await solutionManager.ReloadSolutionIfChangedExternallyAsync(cancellationToken); } private const string FqnHelpMessage = $" Try `{ToolHelpers.SharpToolPrefix}{nameof(Tools.AnalysisTools.SearchDefinitions)}`, `{ToolHelpers.SharpToolPrefix}{nameof(Tools.AnalysisTools.GetMembers)}`, or `{ToolHelpers.SharpToolPrefix}{nameof(Tools.DocumentTools.ReadTypesFromRoslynDocument)}` to find what you need."; public static async Task GetRoslynSymbolOrThrowAsync( diff --git a/SharpTools.Tools/Mcp/Tools/AnalysisTools.cs b/SharpTools.Tools/Mcp/Tools/AnalysisTools.cs index b73a245..e9fb2eb 100644 --- a/SharpTools.Tools/Mcp/Tools/AnalysisTools.cs +++ b/SharpTools.Tools/Mcp/Tools/AnalysisTools.cs @@ -22,7 +22,7 @@ public static async Task GetAllSubtypes( CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedParentTypeName, "fullyQualifiedParentTypeName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(GetAllSubtypes)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(GetAllSubtypes), cancellationToken); logger.LogInformation("Executing {GetAllSubtypes} for: {TypeName}", nameof(GetAllSubtypes), fullyQualifiedParentTypeName); @@ -161,7 +161,7 @@ public static async Task GetMembers( CancellationToken cancellationToken = default) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedTypeName, nameof(fullyQualifiedTypeName), logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(GetMembers)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(GetMembers), cancellationToken); logger.LogInformation("Executing '{GetMembers}' for: {TypeName} (IncludePrivate: {IncludePrivate})", nameof(GetMembers), fullyQualifiedTypeName, includePrivateMembers); @@ -357,7 +357,7 @@ public static async Task ViewDefinition( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedSymbolName, "fullyQualifiedSymbolName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewDefinition)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewDefinition), cancellationToken); logger.LogInformation("Executing '{ViewDefinition}' for: {SymbolName}", nameof(ViewDefinition), fullyQualifiedSymbolName); @@ -423,7 +423,7 @@ public static async Task ListImplementations( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedSymbolName, "fullyQualifiedSymbolName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ListImplementations)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ListImplementations), cancellationToken); logger.LogInformation("Executing '{ViewImplementations}' for: {SymbolName}", nameof(ListImplementations), fullyQualifiedSymbolName); @@ -698,7 +698,7 @@ public static async Task FindReferences( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedSymbolName, "fullyQualifiedSymbolName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindReferences)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindReferences), cancellationToken); logger.LogInformation("Executing '{FindReferences}' for: {SymbolName}", nameof(FindReferences), fullyQualifiedSymbolName); @@ -830,7 +830,7 @@ public static async Task ViewInheritanceChain( CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedTypeName, "fullyQualifiedTypeName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewInheritanceChain)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewInheritanceChain), cancellationToken); logger.LogInformation("Executing '{ViewInheritanceChain}' for: {TypeName}", nameof(ViewInheritanceChain), fullyQualifiedTypeName); @@ -1005,7 +1005,7 @@ public static async Task ViewCallGraph( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedMethodName, "fullyQualifiedMethodName", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewCallGraph)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ViewCallGraph), cancellationToken); logger.LogInformation("Executing '{ViewCallGraph}' for: {MethodName}", nameof(ViewCallGraph), fullyQualifiedMethodName); @@ -1121,7 +1121,7 @@ static bool IsGeneratedCode(string signature) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(regexPattern, "regexPattern", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(SearchDefinitions)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(SearchDefinitions), cancellationToken); logger.LogInformation("Executing '{SearchDefinitions}' with pattern: {RegexPattern}", nameof(SearchDefinitions), regexPattern); @@ -1518,7 +1518,7 @@ public static async Task ManageUsings( } // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ManageUsings)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ManageUsings), cancellationToken); var solution = solutionManager.CurrentSolution ?? throw new McpException("Current solution is null."); var document = solution.Projects @@ -1632,7 +1632,7 @@ public static async Task ManageAttributes( } // Ensure solution is loaded and get target symbol - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ManageAttributes)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ManageAttributes), cancellationToken); var symbol = await ToolHelpers.GetRoslynSymbolOrThrowAsync(solutionManager, targetDeclaration, cancellationToken); if (!symbol.DeclaringSyntaxReferences.Any()) { @@ -1727,7 +1727,7 @@ public static async Task AnalyzeComplexity( } // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(AnalyzeComplexity)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(AnalyzeComplexity), cancellationToken); // Track metrics for the final report var metrics = new Dictionary(); @@ -1779,7 +1779,7 @@ public static async Task FindPotentialDuplicates( [Description("The minimum similarity score (0.0 to 1.0) for methods to be considered similar. (start with 0.75)")] double similarityThreshold, CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindPotentialDuplicates)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindPotentialDuplicates), cancellationToken); logger.LogInformation("Executing '{ToolName}' with threshold {Threshold}", nameof(FindPotentialDuplicates), similarityThreshold); if (similarityThreshold < 0.0 || similarityThreshold > 1.0) { diff --git a/SharpTools.Tools/Mcp/Tools/DocumentTools.cs b/SharpTools.Tools/Mcp/Tools/DocumentTools.cs index b8a359e..aa6a756 100644 --- a/SharpTools.Tools/Mcp/Tools/DocumentTools.cs +++ b/SharpTools.Tools/Mcp/Tools/DocumentTools.cs @@ -31,7 +31,7 @@ public static async Task ReadRawFromRoslynDocument( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(filePath, "filePath", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReadRawFromRoslynDocument)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReadRawFromRoslynDocument), cancellationToken); logger.LogInformation("Reading document at {FilePath}", filePath); @@ -86,7 +86,7 @@ public static async Task CreateRoslynDocument( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(filePath, "filePath", logger); ErrorHandlingHelpers.ValidateStringParameter(content, "content", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(CreateRoslynDocument)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(CreateRoslynDocument), cancellationToken); content = content.TrimBackslash(); logger.LogInformation("Creating new document at {FilePath}", filePath); @@ -168,7 +168,7 @@ public static async Task OverwriteRoslynDocument( return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateStringParameter(filePath, "filePath", logger); ErrorHandlingHelpers.ValidateStringParameter(content, "content", logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(OverwriteRoslynDocument)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(OverwriteRoslynDocument), cancellationToken); content = content.TrimBackslash(); logger.LogInformation("Overwriting document at {FilePath}", filePath); @@ -298,7 +298,7 @@ public static async Task ReadTypesFromRoslynDocument( CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { ErrorHandlingHelpers.ValidateFilePath(filePath, logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReadTypesFromRoslynDocument)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReadTypesFromRoslynDocument), cancellationToken); var pathInfo = documentOperations.GetPathInfo(filePath); if (!pathInfo.Exists) { diff --git a/SharpTools.Tools/Mcp/Tools/ModificationTools.cs b/SharpTools.Tools/Mcp/Tools/ModificationTools.cs index c3d27f9..3ac3350 100644 --- a/SharpTools.Tools/Mcp/Tools/ModificationTools.cs +++ b/SharpTools.Tools/Mcp/Tools/ModificationTools.cs @@ -49,7 +49,7 @@ public static async Task AddMember( codeSnippet = codeSnippet.TrimBackslash(); // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(AddMember)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(AddMember), cancellationToken); logger.LogInformation("Executing '{AddMember}' for target: {TargetName}", nameof(AddMember), fullyQualifiedTargetName); // Get the target symbol @@ -237,7 +237,7 @@ public static async Task OverwriteMember( ErrorHandlingHelpers.ValidateStringParameter(newMemberCode, nameof(newMemberCode), logger); newMemberCode = newMemberCode.TrimBackslash(); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(OverwriteMember)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(OverwriteMember), cancellationToken); logger.LogInformation("Executing '{OverwriteMember}' for: {SymbolName}", nameof(OverwriteMember), fullyQualifiedMemberName); var symbol = await ToolHelpers.GetRoslynSymbolOrThrowAsync(solutionManager, fullyQualifiedMemberName, cancellationToken); @@ -370,7 +370,7 @@ public static async Task RenameSymbol( } // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(RenameSymbol)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(RenameSymbol), cancellationToken); logger.LogInformation("Executing '{RenameSymbol}' for {SymbolName} to {NewName}", nameof(RenameSymbol), fullyQualifiedSymbolName, newName); // Get the symbol to rename @@ -466,7 +466,7 @@ public static async Task ReplaceAllReferences( // Note: filenameFilter can be empty or null, as this indicates "replace in all files" // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReplaceAllReferences)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(ReplaceAllReferences), cancellationToken); logger.LogInformation("Executing '{ReplaceAllReferences}' for {SymbolName} with text '{ReplacementCode}', filter: {Filter}", nameof(ReplaceAllReferences), fullyQualifiedSymbolName, replacementCode, filenameFilter ?? "none"); @@ -602,7 +602,7 @@ public static async Task Undo( CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(Undo)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(Undo), cancellationToken); logger.LogInformation("Executing '{UndoLastChange}'", nameof(Undo)); var (success, message) = await modificationService.UndoLastChangeAsync(cancellationToken); @@ -642,7 +642,7 @@ public static async Task FindAndReplace( .Replace(@"\r", @"\n"); // Ensure solution is loaded - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindAndReplace)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(FindAndReplace), cancellationToken); logger.LogInformation("Executing '{FindAndReplace}' with pattern: '{Pattern}', replacement: {Replacement}, target: {Target}", nameof(FindAndReplace), regexPattern, replacementText, target); @@ -848,7 +848,7 @@ public static async Task MoveMember( ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedMemberName, nameof(fullyQualifiedMemberName), logger); ErrorHandlingHelpers.ValidateStringParameter(fullyQualifiedDestinationTypeOrNamespaceName, nameof(fullyQualifiedDestinationTypeOrNamespaceName), logger); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(MoveMember)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(MoveMember), cancellationToken); logger.LogInformation("Executing '{MoveMember}' moving {MemberName} to {DestinationName}", nameof(MoveMember), fullyQualifiedMemberName, fullyQualifiedDestinationTypeOrNamespaceName); diff --git a/SharpTools.Tools/Mcp/Tools/SolutionTools.cs b/SharpTools.Tools/Mcp/Tools/SolutionTools.cs index a295c40..57d6106 100644 --- a/SharpTools.Tools/Mcp/Tools/SolutionTools.cs +++ b/SharpTools.Tools/Mcp/Tools/SolutionTools.cs @@ -103,7 +103,7 @@ private static async Task GetProjectStructure( CancellationToken cancellationToken) { return await ErrorHandlingHelpers.ExecuteWithErrorHandlingAsync(async () => { - ToolHelpers.EnsureSolutionLoaded(solutionManager); + await ToolHelpers.EnsureSolutionLoaded(solutionManager, cancellationToken); var projectsData = new List(); @@ -370,7 +370,7 @@ public static async Task LoadProject( ErrorHandlingHelpers.ValidateStringParameter(projectName, "projectName", logger); logger.LogInformation("Executing '{LoadProjectToolName}' tool for project: {ProjectName}", nameof(LoadProject), projectName); - ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(LoadProject)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(solutionManager, logger, nameof(LoadProject), cancellationToken); int indexOfParen = projectName.IndexOf('('); string projectNameNormalized = indexOfParen == -1 ? projectName.Trim() diff --git a/SharpTools.Tools/Services/DocumentOperationsService.cs b/SharpTools.Tools/Services/DocumentOperationsService.cs index c05a448..cd3c12a 100644 --- a/SharpTools.Tools/Services/DocumentOperationsService.cs +++ b/SharpTools.Tools/Services/DocumentOperationsService.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using System.Xml; using Microsoft.CodeAnalysis.Text; +using SharpTools.Tools.Interfaces; namespace SharpTools.Tools.Services; @@ -13,6 +14,7 @@ public class DocumentOperationsService : IDocumentOperationsService { private readonly ISolutionManager _solutionManager; private readonly ICodeModificationService _modificationService; private readonly IGitService _gitService; + private readonly IFileMonitoringService _fileMonitoring; private readonly ILogger _logger; // Extensions for common code file types that can be formatted @@ -29,10 +31,12 @@ public DocumentOperationsService( ISolutionManager solutionManager, ICodeModificationService modificationService, IGitService gitService, + IFileMonitoringService fileMonitoring, ILogger logger) { _solutionManager = solutionManager; _modificationService = modificationService; _gitService = gitService; + _fileMonitoring = fileMonitoring; _logger = logger; } @@ -79,8 +83,11 @@ public async Task WriteFileAsync(string filePath, string content, bool ove Directory.CreateDirectory(directory); } + _fileMonitoring.RegisterExpectedChange(filePath); + // Write the content to the file await File.WriteAllTextAsync(filePath, content, cancellationToken); + _logger.LogInformation("File {Operation} at {FilePath}", File.Exists(filePath) ? "overwritten" : "created", filePath); @@ -123,8 +130,8 @@ public async Task WriteFileAsync(string filePath, string content, bool ove } else { _logger.LogWarning("Failed to format file: {FilePath}", filePath); } - return true; - } + return true; + } private async Task TryAddFileToLegacyProjectAsync(string filePath, Project project, CancellationToken cancellationToken) { if (!_solutionManager.IsSolutionLoaded || !File.Exists(filePath)) { diff --git a/SharpTools.Tools/Services/FileMonitoringService.cs b/SharpTools.Tools/Services/FileMonitoringService.cs new file mode 100644 index 0000000..d40ca51 --- /dev/null +++ b/SharpTools.Tools/Services/FileMonitoringService.cs @@ -0,0 +1,48 @@ +using SharpTools.Tools.Interfaces; + +namespace SharpTools.Tools.Services; + +/// +/// Stub implementation of file monitoring service. +/// This implementation has no side effects and does not trigger reloads. +/// +public sealed class FileMonitoringService : IFileMonitoringService +{ + private readonly ILogger _logger; + private bool _disposed; + + public FileMonitoringService(ILogger logger) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + public bool IsReloadNeeded { get; private set; } + + public void StartMonitoring(string directory) + { + _logger.LogInformation("Starting file monitoring for directory: {Directory}", directory); + } + + public void StopMonitoring() + { + _logger.LogInformation("Stopping file monitoring"); + } + + public void SetKnownFilePaths(ISet filePathsToWatch) + { + _logger.LogInformation("Setting known file paths: {FileCount} files", filePathsToWatch.Count); + } + + public void RegisterExpectedChange(string filePath) + { + _logger.LogTrace("Registering expected change to {FilePath}", filePath); + } + + public void Dispose() + { + if (_disposed) return; + + StopMonitoring(); + _disposed = true; + } +} \ No newline at end of file diff --git a/SharpTools.Tools/Services/SemanticSimilarityService.cs b/SharpTools.Tools/Services/SemanticSimilarityService.cs index 1b842cd..f623a08 100644 --- a/SharpTools.Tools/Services/SemanticSimilarityService.cs +++ b/SharpTools.Tools/Services/SemanticSimilarityService.cs @@ -16,7 +16,7 @@ namespace SharpTools.Tools.Services { public class SemanticSimilarityService : ISemanticSimilarityService { private static class Tuning { - public static readonly int MaxDegreesOfParallelism = Math.Max(1, Environment.ProcessorCount / 2); + public static readonly int MaxDegreesOfParallelism = Math.Max(1, Environment.ProcessorCount / 2); public const int MethodLineCountFilter = 10; public const double DefaultSimilarityThreshold = 0.7; @@ -186,7 +186,7 @@ public SemanticSimilarityService( public async Task> FindSimilarMethodsAsync( double similarityThreshold, CancellationToken cancellationToken) { - ToolHelpers.EnsureSolutionLoadedWithDetails(_solutionManager, _logger, nameof(FindSimilarMethodsAsync)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(_solutionManager, _logger, nameof(FindSimilarMethodsAsync), cancellationToken); _logger.LogInformation("Starting semantic similarity analysis with threshold {Threshold}, MaxDOP: {MaxDop}", similarityThreshold, Tuning.MaxDegreesOfParallelism); var allMethodFeatures = new System.Collections.Concurrent.ConcurrentBag(); @@ -504,7 +504,7 @@ private double CalculateCosineSimilarity(Dictionary vec1, Dictionar public async Task> FindSimilarClassesAsync( double similarityThreshold, CancellationToken cancellationToken) { - ToolHelpers.EnsureSolutionLoadedWithDetails(_solutionManager, _logger, nameof(FindSimilarClassesAsync)); + await ToolHelpers.EnsureSolutionLoadedWithDetails(_solutionManager, _logger, nameof(FindSimilarClassesAsync), cancellationToken); _logger.LogInformation("Starting class semantic similarity analysis with threshold {Threshold}, MaxDOP: {MaxDop}", similarityThreshold, Tuning.MaxDegreesOfParallelism); var allClassFeatures = new System.Collections.Concurrent.ConcurrentBag(); @@ -762,7 +762,7 @@ await Parallel.ForEachAsync(documents, parallelOptions, async (document, docCt) totalLinesOfCode, classMethodFeatures ); - } + } private List CompareClassFeatures( List allClassFeatures, @@ -933,6 +933,6 @@ private void AddTypeAndNamespaceIfExternal( if (typeSymbol is IPointerTypeSymbol pointerTypeSymbol) { AddTypeAndNamespaceIfExternal(pointerTypeSymbol.PointedAtType, containingClassSymbol, externalTypeFqns, usedNamespaceFqns); } - } + } } } diff --git a/SharpTools.Tools/Services/SolutionManager.cs b/SharpTools.Tools/Services/SolutionManager.cs index e8651dd..f951563 100644 --- a/SharpTools.Tools/Services/SolutionManager.cs +++ b/SharpTools.Tools/Services/SolutionManager.cs @@ -1,12 +1,14 @@ using System.Runtime.InteropServices; using System.Xml.Linq; using ModelContextProtocol; +using SharpTools.Tools.Interfaces; using SharpTools.Tools.Mcp.Tools; namespace SharpTools.Tools.Services; public sealed class SolutionManager : ISolutionManager { private readonly ILogger _logger; private readonly IFuzzyFqnLookupService _fuzzyFqnLookupService; + private readonly IFileMonitoringService _fileMonitoring; private MSBuildWorkspace? _workspace; private Solution? _currentSolution; private MetadataLoadContext? _metadataLoadContext; @@ -21,9 +23,10 @@ public sealed class SolutionManager : ISolutionManager { public Solution? CurrentSolution => _currentSolution; private readonly string? _buildConfiguration; - public SolutionManager(ILogger logger, IFuzzyFqnLookupService fuzzyFqnLookupService, string? buildConfiguration = null) { + public SolutionManager(ILogger logger, IFuzzyFqnLookupService fuzzyFqnLookupService, IFileMonitoringService fileMonitoring, string? buildConfiguration = null) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _fuzzyFqnLookupService = fuzzyFqnLookupService ?? throw new ArgumentNullException(nameof(fuzzyFqnLookupService)); + _fileMonitoring = fileMonitoring ?? throw new ArgumentNullException(nameof(fileMonitoring)); _buildConfiguration = buildConfiguration; } public async Task LoadSolutionAsync(string solutionPath, CancellationToken cancellationToken) { @@ -32,6 +35,13 @@ public async Task LoadSolutionAsync(string solutionPath, CancellationToken cance throw new FileNotFoundException("Solution file not found.", solutionPath); } UnloadSolution(); // Clears previous state including _allLoadedReflectionTypesCache + + // Start file monitoring before loading solution + var solutionDirectory = Path.GetDirectoryName(solutionPath); + if (!string.IsNullOrEmpty(solutionDirectory)) { + _fileMonitoring.StartMonitoring(solutionDirectory); + } + try { _logger.LogInformation("Creating MSBuildWorkspace..."); var properties = new Dictionary { @@ -48,6 +58,7 @@ public async Task LoadSolutionAsync(string solutionPath, CancellationToken cance _currentSolution = await _workspace.OpenSolutionAsync(solutionPath, new ProgressReporter(_logger), cancellationToken); _logger.LogInformation("Solution loaded successfully with {ProjectCount} projects.", _currentSolution.Projects.Count()); InitializeMetadataContextAndReflectionCache(_currentSolution, cancellationToken); + FocusFileMonitoring(_currentSolution); } catch (Exception ex) { _logger.LogError(ex, "Failed to load solution: {SolutionPath}", solutionPath); UnloadSolution(); @@ -194,6 +205,7 @@ private void LoadTypesFromAssembly(string assemblyPath, ref int typesCachedCount } } public void UnloadSolution() { + _fileMonitoring.StopMonitoring(); _logger.LogInformation("Unloading current solution and workspace."); _compilationCache.Clear(); _semanticModelCache.Clear(); @@ -235,6 +247,12 @@ public async Task ReloadSolutionFromDiskAsync(CancellationToken cancellationToke await LoadSolutionAsync(_workspace.CurrentSolution.FilePath!, cancellationToken); _logger.LogDebug("Current solution state has been refreshed from workspace."); } + public async Task ReloadSolutionIfChangedExternallyAsync(CancellationToken cancellationToken) { + if (_fileMonitoring.IsReloadNeeded) { + await ReloadSolutionFromDiskAsync(cancellationToken); + } + } + private void OnWorkspaceFailed(object? sender, WorkspaceDiagnosticEventArgs e) { var diagnostic = e.Diagnostic; var level = diagnostic.Kind == WorkspaceDiagnosticKind.Failure ? LogLevel.Error : LogLevel.Warning; @@ -648,4 +666,48 @@ private static string[] GetCompatibleFrameworks(string targetFramework) { _ => new[] { "net8.0", "net7.0", "net6.0", "net5.0", "netcoreapp3.1", "netcoreapp3.0", "netcoreapp2.1", "netstandard2.1", "netstandard2.0", "netstandard1.6" } }; } + + private void FocusFileMonitoring(Solution solution) + { + // Gather all file paths from the Roslyn solution + var solutionFilePaths = new HashSet(StringComparer.OrdinalIgnoreCase); + + // Add solution file + if (!string.IsNullOrEmpty(solution.FilePath)) + { + solutionFilePaths.Add(solution.FilePath); + } + + foreach (var project in solution.Projects) + { + // Add project file + if (!string.IsNullOrEmpty(project.FilePath)) + solutionFilePaths.Add(project.FilePath); + + // Add all documents + foreach (var document in project.Documents) + { + if (!string.IsNullOrEmpty(document.FilePath)) + solutionFilePaths.Add(document.FilePath); + } + + // Add additional documents (like config files) + foreach (var additionalDoc in project.AdditionalDocuments) + { + if (!string.IsNullOrEmpty(additionalDoc.FilePath)) + solutionFilePaths.Add(additionalDoc.FilePath); + } + + // Add analyzer config documents (like .editorconfig) + foreach (var configDoc in project.AnalyzerConfigDocuments) + { + if (!string.IsNullOrEmpty(configDoc.FilePath)) + solutionFilePaths.Add(configDoc.FilePath); + } + } + + // Provide the definitive set of files to the monitoring service + _fileMonitoring.SetKnownFilePaths(solutionFilePaths); + _logger.LogInformation("File monitoring is now active for {FileCount} files in the solution.", solutionFilePaths.Count); + } } \ No newline at end of file diff --git a/docs/filemon.design.md b/docs/filemon.design.md index a398f1d..7bfb80f 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -6,82 +6,35 @@ The file monitoring system provides intelligent detection of external file chang ## Core Concepts +### Open Concerns + +- Is watching DocumentOperationService.WriteFileAsync sufficient for discovering all expected file changes? +- Comparison of filenames given case insensitivity on Windows? + ### Comprehensive file monitoring The file monitoring service is starts watching the solution folder recursively before the solution build starts. Once the build is complete information from Roslyn is used to control what files can actually trigger a reload. All MCP operations will check if a reload is needed before continuing their operation. -### Tracking Expected Changes To Avoid Reloading - -Operations use an `IFileChangeVisitor` to register files they expect to modify as they discover them during execution. This allows file monitoring to ignore expected changes and prevent unnecessary reloads. - -```csharp -// Operation gets visitor and registers expected changes as discovered -var fileChangeVisitor = _fileMonitoring.BeginOperation(operationId); -fileChangeVisitor.ExpectChange("C:\Solution\MyClass.cs"); // As we discover changes -fileChangeVisitor.ExpectChanges(additionalFiles); // Batch registration -fileChangeVisitor.EndOperation(); -``` - -### External Change Detection - -The system triggers reload only in these scenarios: - -1. **File changes while no operations are active** - Clearly external -2. **Unexpected file changes during operation** - File wasn't in expected list -3. **Multiple changes to same file during operation** - Indicates external interference - -## Architecture Components - -### IFileMonitoringService - -Central service that coordinates file watching and operation tracking: - -```csharp -public interface IFileMonitoringService : IDisposable -{ - // Basic monitoring - void StartMonitoring(string solutionPath); - bool IsReloadNeeded { get; } - void MarkReloadComplete(); - - // Visitor pattern for operations - IFileChangeVisitor BeginOperation(string operationId); -} -``` - -### IFileChangeVisitor +Monitoring always ignores certain directories altogether: ".git", "bin" and "obj". -Visitor that operations use to register expected file changes: +Monitoring is halted when SolutionManager unloads the solution or when singleton is disposed by application close. -```csharp -public interface IFileChangeVisitor -{ - string OperationId { get; } - void ExpectChange(string filePath); - void ExpectChanges(IEnumerable filePaths); - void EndOperation(); -} -``` - -### Integration Points - -**SolutionManager**: Checks `IsReloadNeeded` in `ToolHelpers.EnsureSolutionLoaded()` and `ToolHelpers.EnsureSolutionLoadedWithDetails()` which all MCP tools use +### Tracking Expected Changes To Avoid Reloading -**CodeModificationService**: Uses visitor to register Roslyn solution changes as they're discovered: -- Changed documents -- Added documents -- Removed documents -- Git metadata files +Operations register files they expect to modify as they discover them during execution. This allows file monitoring to ignore expected changes and prevent unnecessary reloads. -**DocumentOperationsService**: Registers non-Roslyn file operations (like `.editorconfig` edits) +### Implementation -**MCP Tools**: Can register additional expected changes for complex operations +A singleton implementation of IFileMonitoringService tracks changes. SolutionManager makes calls indicating when monitoring +should start and what files are known to be relevant. DocumentOperationService notifies when any expected changes are written. +ToolsHelper's EnsureSolutionLoaded* methods, which are called at the start of every MCP operation, notify SolutionManager to +reload the solution if appropriate. ## File Discovery and Monitoring Strategy -To simplify monitoring, the system watches the entire solution's root directory recursively. This automatically handles file additions and deletions without complex tracking. +To minimze monitoring calls made to the operating system, the system watches the entire solution's root directory recursively. This automatically handles file additions and deletions without complex tracking. The monitoring is made efficient and relevant by filtering events against the actual files that make up the solution: @@ -108,47 +61,16 @@ This approach closes the gap and guarantees that any changes occurring during th ### Initial Reconciliation (when known files are set) For each file in the provided set of known solution files: 1. Does this file exist in the backlog of changes recorded since monitoring began? - - Yes -> Set IsReloadNeeded = true, and the backlog can be cleared. + - Yes -> Set IsReloadNeeded = true and stop monitoring - No -> Continue. +2. Clear the backlog ### Real-time Change Detection (after initialization) For each file change detected by FileSystemWatcher: -1. Is any operation currently active? - - No → Set IsReloadNeeded = true (external change) - - Yes → Continue to step 2 - -2. Is this file expected by any active operation? - - No → Set IsReloadNeeded = true (unexpected change) - - Yes → Continue to step 3 - -3. Has this file already changed during this operation? - - No → Mark as occurred, continue monitoring - - Yes → Set IsReloadNeeded = true (external interference) -``` - -## Example Scenarios +1. Was an expected change registered for this file? + - No → Set IsReloadNeeded = true (external change) and stop monitoring + - Yes → Remove file from set of expected changes -### Pure MCP Operation (No Reload) -``` -1. sharp_add_member → BeginOperation with visitor -2. CodeModificationService registers expected file: MyClass.cs -3. File watcher sees MyClass.cs change → Expected, mark as occurred -4. visitor.EndOperation() → No reload needed ✓ -``` - -### External Change During Operation (Triggers Reload) -``` -1. sharp_add_member → BeginOperation, expects MyClass.cs -2. MyClass.cs changes (expected) → Mark as occurred -3. User edits MyClass.cs again → Same file, 2nd change → IsReloadNeeded = true -4. Next MCP request → Reload triggered ✓ -``` - -### External Change While Idle (Triggers Reload) -``` -1. No operations active -2. User edits MyClass.cs → No operation to check against → IsReloadNeeded = true -3. Next MCP request → Reload triggered ✓ ``` ## Error Handling Strategy diff --git a/docs/filemon.plan.md b/docs/filemon.plan.md deleted file mode 100644 index 068b3ec..0000000 --- a/docs/filemon.plan.md +++ /dev/null @@ -1,292 +0,0 @@ -# File Monitoring Implementation Plan - -## Phase 1: Core File Monitoring Service - -### 1.1 Create IFileMonitoringService Interface -```csharp -public interface IFileMonitoringService : IDisposable -{ - // Phase 1: Start monitoring immediately on startup. - void StartMonitoring(string directory); - void StopMonitoring(); - - // Phase 2: Provide the set of files to watch after solution load. - // This will reconcile any changes that happened in the interim. - void SetKnownFilePaths(ISet filePathsToWatch); - - bool IsMonitoring { get; } - bool IsReloadNeeded { get; } - void MarkReloadComplete(); - - // Operation tracking with file change visitor pattern - IFileChangeVisitor BeginOperation(string operationId); - bool IsOperationInProgress(string operationId); - - // For backward compatibility and simple operations - void RegisterExpectedChange(string operationId, string filePath); - bool IsChangeExpected(string filePath); - void MarkExpectedChangeOccurred(string filePath); -} - -public interface IFileChangeVisitor -{ - string OperationId { get; } - void ExpectChange(string filePath); - void ExpectChanges(IEnumerable filePaths); - void EndOperation(); - bool IsChangeExpected(string filePath); -} -``` - -### 1.2 Implement FileMonitoringService -- On `StartMonitoring`, use a `FileSystemWatcher` to watch the root directory recursively. Record all file change events (path and timestamp) into a temporary, thread-safe backlog. -- On `SetKnownFilePaths`: - - Atomically replace the internal set of known file paths. - - Process the backlog of changes. For each change, if the file path is in the new set of known files, set `IsReloadNeeded = true` and stop processing the backlog. - - Clear the backlog. -- For new events arriving after `SetKnownFilePaths` has been called: - - Check if the file path is in the known set. If not, ignore it. - - If it is a known file, proceed with the existing logic (check for active operations, etc.). -- Maintain thread-safe access to the backlog, the known file set, and the `IsReloadNeeded` flag. - -### 1.3 Add to Dependency Injection -- Register `IFileMonitoringService` as singleton in `ServiceCollectionExtensions.cs` -- Ensure proper disposal through DI container - -## Phase 2: Integration with SolutionManager - -### 2.1 Modify SolutionManager -- In `LoadSolutionAsync`, call `_fileMonitoring.StartMonitoring(solutionDirectory)` *before* beginning the Roslyn solution load. -- After the solution is loaded successfully, gather the list of file paths. -- Call `_fileMonitoring.SetKnownFilePaths(solutionFilePaths)` to complete the setup. -- Add lazy reload check to key operations: - - `EnsureSolutionLoaded()` method checks `fileMonitoring.IsReloadNeeded` - - If reload needed, calls `ReloadSolutionFromDiskAsync()` and `fileMonitoring.MarkReloadComplete()` - -### 2.2 Modify CodeModificationService -- Add `IFileMonitoringService` dependency injection -- Use visitor pattern to dynamically register expected file changes as they're discovered: - ```csharp - public async Task ApplyChangesAsync(Solution newSolution, CancellationToken cancellationToken, string commitMessage, IEnumerable? additionalFilePaths = null) - { - var operationId = Guid.NewGuid().ToString(); - var fileChangeVisitor = _fileMonitoring.BeginOperation(operationId); - - try - { - var originalSolution = _solutionManager.CurrentSolution!; - var solutionChanges = newSolution.GetChanges(originalSolution); - - // Register expected changes as we discover them during processing - foreach (var projectChange in solutionChanges.GetProjectChanges()) - { - // Register changed documents - foreach (var changedDocumentId in projectChange.GetChangedDocuments()) - { - var document = newSolution.GetDocument(changedDocumentId); - if (document?.FilePath != null) - { - fileChangeVisitor.ExpectChange(document.FilePath); - _logger.LogTrace("Expecting change to: {FilePath}", document.FilePath); - } - } - - // Register added documents - foreach (var addedDocumentId in projectChange.GetAddedDocuments()) - { - var document = newSolution.GetDocument(addedDocumentId); - if (document?.FilePath != null) - { - fileChangeVisitor.ExpectChange(document.FilePath); - _logger.LogTrace("Expecting new file: {FilePath}", document.FilePath); - } - } - - // Register removed documents - foreach (var removedDocumentId in projectChange.GetRemovedDocuments()) - { - var document = originalSolution.GetDocument(removedDocumentId); - if (document?.FilePath != null) - { - fileChangeVisitor.ExpectChange(document.FilePath); - _logger.LogTrace("Expecting removal of: {FilePath}", document.FilePath); - } - } - } - - if (additionalFilePaths != null) - // Register additional files (non-code files from tools like FindAndReplace) - { - fileChangeVisitor.ExpectChanges(additionalFilePaths); - } - - // Apply changes to workspace - this triggers the actual file writes - if (workspace.TryApplyChanges(finalSolutionToApply)) - { - _solutionManager.RefreshCurrentSolution(); - } - - // Register git files if git operations enabled - if (_gitEnabled && !string.IsNullOrEmpty(commitMessage)) - { - fileChangeVisitor.ExpectChange(".git/index"); - fileChangeVisitor.ExpectChange(".git/logs/HEAD"); - fileChangeVisitor.ExpectChange(".git/COMMIT_EDITMSG"); - await CommitChangesIfEnabled(commitMessage, cancellationToken); - } - } - finally - { - fileChangeVisitor.EndOperation(); - } - } - ``` - -### 2.3 Finalizing Monitor Configuration - -After the solution is loaded, the `SolutionManager` provides the file monitoring service with the definitive list of files to watch. The service can then reconcile any changes that occurred during the load process. - -A conceptual example of the `SolutionManager` logic: -```csharp -public async Task LoadSolutionAsync(string solutionPath) -{ - var solutionDirectory = Path.GetDirectoryName(solutionPath); - _fileMonitoring.StartMonitoring(solutionDirectory); // Start watching immediately - - // ... proceed to load the solution with Roslyn ... - _currentSolution = await workspace.OpenSolutionAsync(solutionPath); - - FinalizeFileMonitoring(); // Now provide the list of files -} - -private void FinalizeFileMonitoring() -{ - var solution = _solutionManager.CurrentSolution; - - // 1. Discover all file paths from the Roslyn solution - var solutionFilePaths = new HashSet(StringComparer.OrdinalIgnoreCase); - if (!string.IsNullOrEmpty(solution.FilePath)) - { - solutionFilePaths.Add(solution.FilePath); - } - - foreach (var project in solution.Projects) - { - if (!string.IsNullOrEmpty(project.FilePath)) - solutionFilePaths.Add(project.FilePath); - - foreach (var document in project.Documents) - { - if (!string.IsNullOrEmpty(document.FilePath)) - solutionFilePaths.Add(document.FilePath); - } - - // Also include AdditionalDocuments, AnalyzerConfigDocuments, etc. - foreach (var additionalDoc in project.AdditionalDocuments) - { - if (!string.IsNullOrEmpty(additionalDoc.FilePath)) - solutionFilePaths.Add(additionalDoc.FilePath); - } - foreach (var configDoc in project.AnalyzerConfigDocuments) - { - if (!string.IsNullOrEmpty(configDoc.FilePath)) - solutionFilePaths.Add(configDoc.FilePath); - } - } - - // 2. Provide the definitive set of files to the monitoring service. - _fileMonitoring.SetKnownFilePaths(solutionFilePaths); - _logger.LogInformation("File monitoring is now active for {FileCount} files in the solution.", solutionFilePaths.Count); -} -``` - -### 2.4 Lazy Reload Integration -- Add reload check to `ToolHelpers.EnsureSolutionLoaded()` method -- All MCP tools use this helper, ensuring automatic reload when needed -- Add logging when reload is triggered -- Handle edge cases (file deletion, temporary files, etc.) - -### 2.5 Integration with Document Operations -- Modify `DocumentOperationsService.WriteFileAsync()` to accept file change visitor for tracking: - ```csharp - public async Task WriteFileAsync(string filePath, string content, bool overwrite, - CancellationToken cancellationToken, string commitMessage, IFileChangeVisitor? fileChangeVisitor = null) - { - // Register expected change just before performing the write - fileChangeVisitor?.ExpectChange(filePath); - - // If git is enabled, also expect git metadata changes - if (_gitEnabled && !string.IsNullOrEmpty(commitMessage)) - { - fileChangeVisitor?.ExpectChange(".git/index"); - fileChangeVisitor?.ExpectChange(".git/logs/HEAD"); - } - - // ... perform file write - } - ``` - -### 2.6 High-Level Tool Integration -- Modify MCP tools to pass file change visitor to sub-operations: - ```csharp - [McpServerTool(Name = "sharp_find_and_replace")] - public static async Task FindAndReplace(/*...*/) - { - var operationId = Guid.NewGuid().ToString(); - var fileChangeVisitor = fileMonitoring.BeginOperation(operationId); - - try - { - // Tools can register expected changes as they discover them - if (isNonCodeFileOperation) - { - fileChangeVisitor.ExpectChange(targetFilePath); - } - - await modificationService.ApplyChangesAsync(newSolution, cancellationToken, commitMessage); - // ApplyChangesAsync will register its own expected changes via the fileChangeVisitor - } - finally - { - fileChangeVisitor.EndOperation(operationId); - } - } - ``` - -## Phase 3: Configuration and Control - -### 3.1 Add Configuration Options -```csharp -public class FileMonitoringOptions -{ - public bool EnableFileMonitoring { get; set; } = true; - public bool LogFileChanges { get; set; } = true; -} -``` - -### 3.2 Add MCP Tool for Control -```csharp -[McpServerTool(Name = "sharp_configure_file_monitoring")] -public static object ConfigureFileMonitoring( - ISolutionManager solutionManager, - [Description("Enable or disable automatic file monitoring")] bool enabled) -``` - -## Phase 4: Testing and Validation - -### 4.1 Unit Tests -- Test FileMonitoringService file detection and flag setting -- Test lazy reload logic in SolutionManager -- Test integration with ToolHelpers.EnsureSolutionLoaded() -- Mock file system changes for reliable testing - -### 4.2 Integration Tests -- Test with real solution files -- Verify lazy reload triggered on MCP requests after file changes -- Test performance impact of monitoring large solutions -- Test edge cases (network drives, permission issues) - -### 4.3 Performance Testing -- Measure impact on solution loading time -- Test with large solutions (100+ projects) -- Memory usage analysis for long-running monitoring \ No newline at end of file From dfe17b03b029b37d0db9a636a890542027af779e Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 14:11:20 -0300 Subject: [PATCH 07/12] adding test project to the solution --- .../SharpTools.Tools.Tests.csproj | 4 ++ SharpTools.Tools.Tests/UnitTest1.cs | 10 ---- SharpTools.sln | 48 +++++++++++++++++-- 3 files changed, 49 insertions(+), 13 deletions(-) delete mode 100644 SharpTools.Tools.Tests/UnitTest1.cs diff --git a/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj index e1b74a9..3170b79 100644 --- a/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj +++ b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj @@ -18,4 +18,8 @@ + + + + diff --git a/SharpTools.Tools.Tests/UnitTest1.cs b/SharpTools.Tools.Tests/UnitTest1.cs deleted file mode 100644 index c86fa18..0000000 --- a/SharpTools.Tools.Tests/UnitTest1.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace SharpTools.Tools.Tests; - -public class UnitTest1 -{ - [Fact] - public void Test1() - { - - } -} diff --git a/SharpTools.sln b/SharpTools.sln index 3adb5cf..f08b507 100644 --- a/SharpTools.sln +++ b/SharpTools.sln @@ -9,26 +9,68 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SharpTools.SseServer", "Sha EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SharpTools.StdioServer", "SharpTools.StdioServer\SharpTools.StdioServer.csproj", "{6DF2B244-8781-4F09-87E7-F5130D03821D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SharpTools.Tools.Tests", "SharpTools.Tools.Tests\SharpTools.Tools.Tests.csproj", "{FB125F6D-C10E-41F8-A5DE-07279F7500D3}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU - EndGlobalSection - GlobalSection(SolutionProperties) = preSolution - HideSolutionNode = FALSE + Release|x64 = Release|x64 + Release|x86 = Release|x86 EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|x64.ActiveCfg = Debug|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|x64.Build.0 = Debug|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|x86.ActiveCfg = Debug|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Debug|x86.Build.0 = Debug|Any CPU {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|Any CPU.ActiveCfg = Release|Any CPU {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|Any CPU.Build.0 = Release|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|x64.ActiveCfg = Release|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|x64.Build.0 = Release|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|x86.ActiveCfg = Release|Any CPU + {43569443-A282-4E77-AD84-9E5813A98E8D}.Release|x86.Build.0 = Release|Any CPU {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|x64.ActiveCfg = Debug|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|x64.Build.0 = Debug|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|x86.ActiveCfg = Debug|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Debug|x86.Build.0 = Debug|Any CPU {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|Any CPU.ActiveCfg = Release|Any CPU {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|Any CPU.Build.0 = Release|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|x64.ActiveCfg = Release|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|x64.Build.0 = Release|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|x86.ActiveCfg = Release|Any CPU + {C3B267BF-EA86-4A06-BAE6-1FEDC5A30213}.Release|x86.Build.0 = Release|Any CPU {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|x64.ActiveCfg = Debug|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|x64.Build.0 = Debug|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|x86.ActiveCfg = Debug|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Debug|x86.Build.0 = Debug|Any CPU {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|Any CPU.ActiveCfg = Release|Any CPU {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|Any CPU.Build.0 = Release|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|x64.ActiveCfg = Release|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|x64.Build.0 = Release|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|x86.ActiveCfg = Release|Any CPU + {6DF2B244-8781-4F09-87E7-F5130D03821D}.Release|x86.Build.0 = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|Any CPU.Build.0 = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|x64.ActiveCfg = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|x64.Build.0 = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|x86.ActiveCfg = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Debug|x86.Build.0 = Debug|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|Any CPU.ActiveCfg = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|Any CPU.Build.0 = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|x64.ActiveCfg = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|x64.Build.0 = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|x86.ActiveCfg = Release|Any CPU + {FB125F6D-C10E-41F8-A5DE-07279F7500D3}.Release|x86.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE EndGlobalSection EndGlobal From 2652031d63279c9b95b4c26b3745f856433f04b7 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 14:30:35 -0300 Subject: [PATCH 08/12] Updating plans and monitoring interface to account for the fact that a single write to a file could cause multiple change events. --- .../Interfaces/IFileMonitoringService.cs | 4 +- .../Services/DocumentOperationsService.cs | 2 +- .../Services/FileMonitoringService.cs | 7 +-- SharpTools.Tools/Services/SolutionManager.cs | 2 +- docs/filemon.design.md | 43 ++++++------------- 5 files changed, 21 insertions(+), 37 deletions(-) diff --git a/SharpTools.Tools/Interfaces/IFileMonitoringService.cs b/SharpTools.Tools/Interfaces/IFileMonitoringService.cs index 08ddbbe..400a6ed 100644 --- a/SharpTools.Tools/Interfaces/IFileMonitoringService.cs +++ b/SharpTools.Tools/Interfaces/IFileMonitoringService.cs @@ -26,11 +26,11 @@ public interface IFileMonitoringService : IDisposable /// /// Whether a solution reload is needed due to external file changes. /// - bool IsReloadNeeded { get; } + Task AssessIfReloadNecessary(); /// /// Register an expected file change for backward compatibility. /// /// File path that will be modified - void RegisterExpectedChange(string filePath); + void RegisterExpectedChange(string filePath, string fileContents); } \ No newline at end of file diff --git a/SharpTools.Tools/Services/DocumentOperationsService.cs b/SharpTools.Tools/Services/DocumentOperationsService.cs index cd3c12a..f870864 100644 --- a/SharpTools.Tools/Services/DocumentOperationsService.cs +++ b/SharpTools.Tools/Services/DocumentOperationsService.cs @@ -83,7 +83,7 @@ public async Task WriteFileAsync(string filePath, string content, bool ove Directory.CreateDirectory(directory); } - _fileMonitoring.RegisterExpectedChange(filePath); + _fileMonitoring.RegisterExpectedChange(filePath, content); // Write the content to the file await File.WriteAllTextAsync(filePath, content, cancellationToken); diff --git a/SharpTools.Tools/Services/FileMonitoringService.cs b/SharpTools.Tools/Services/FileMonitoringService.cs index d40ca51..3e4a92e 100644 --- a/SharpTools.Tools/Services/FileMonitoringService.cs +++ b/SharpTools.Tools/Services/FileMonitoringService.cs @@ -16,8 +16,6 @@ public FileMonitoringService(ILogger logger) _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } - public bool IsReloadNeeded { get; private set; } - public void StartMonitoring(string directory) { _logger.LogInformation("Starting file monitoring for directory: {Directory}", directory); @@ -32,8 +30,11 @@ public void SetKnownFilePaths(ISet filePathsToWatch) { _logger.LogInformation("Setting known file paths: {FileCount} files", filePathsToWatch.Count); } + public Task AssessIfReloadNecessary() { + return Task.FromResult(false); + } - public void RegisterExpectedChange(string filePath) + public void RegisterExpectedChange(string filePath, string fileContents) { _logger.LogTrace("Registering expected change to {FilePath}", filePath); } diff --git a/SharpTools.Tools/Services/SolutionManager.cs b/SharpTools.Tools/Services/SolutionManager.cs index f951563..3fc3c48 100644 --- a/SharpTools.Tools/Services/SolutionManager.cs +++ b/SharpTools.Tools/Services/SolutionManager.cs @@ -248,7 +248,7 @@ public async Task ReloadSolutionFromDiskAsync(CancellationToken cancellationToke _logger.LogDebug("Current solution state has been refreshed from workspace."); } public async Task ReloadSolutionIfChangedExternallyAsync(CancellationToken cancellationToken) { - if (_fileMonitoring.IsReloadNeeded) { + if (await _fileMonitoring.AssessIfReloadNecessary()) { await ReloadSolutionFromDiskAsync(cancellationToken); } } diff --git a/docs/filemon.design.md b/docs/filemon.design.md index 7bfb80f..1405808 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -2,7 +2,8 @@ ## Overview -The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It monitors all solution files for changes while ignoring expected changes. +The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It monitors all solution files for unexpected changes so the solution can be reloaded +when commands are run after external changes to solution files. ## Core Concepts @@ -13,19 +14,20 @@ The file monitoring system provides intelligent detection of external file chang ### Comprehensive file monitoring -The file monitoring service is starts watching the solution folder recursively before the solution build starts. Once the -build is complete information from Roslyn is used to control what files can actually trigger a reload. All MCP operations +The file monitoring service is starts watching the solution folder recursively before the solution is loaded. Once the +solution is loaded information from Roslyn is used to control what files can actually trigger a reload. All MCP operations will check if a reload is needed before continuing their operation. Monitoring always ignores certain directories altogether: ".git", "bin" and "obj". -Monitoring is halted when SolutionManager unloads the solution or when singleton is disposed by application close. +Monitoring is halted when SolutionManager unloads the solution or when the singleton is disposed by application close. ### Tracking Expected Changes To Avoid Reloading -Operations register files they expect to modify as they discover them during execution. This allows file monitoring to ignore expected changes and prevent unnecessary reloads. +Operations register files they expect to modify as they discover them during execution with the expected file contents. +This allows file monitoring to ignore expected changes and prevent unnecessary reloads. -### Implementation +### Integration A singleton implementation of IFileMonitoringService tracks changes. SolutionManager makes calls indicating when monitoring should start and what files are known to be relevant. DocumentOperationService notifies when any expected changes are written. @@ -44,34 +46,15 @@ The monitoring is made efficient and relevant by filtering events against the ac This approach ensures that the system only reacts to changes in files it genuinely cares about, providing a balance of simplicity (one watcher) and precision (filtering by exact file path). -## Handling Race Conditions on Startup +## Change detection while loading the solution -A potential race condition exists where a file could be modified by an external process after the application starts but before the Roslyn solution has been fully loaded and analyzed. To solve this, the monitoring service uses a two-phase initialization: +Before the solution is loaded we do not know exactly what files might necessitate a reload. Once the solution is loaded we do know what files are relevant, and will have expected changes registered before we expect to assess if a reload is need. To support this, monitoring has phases: -1. **Early Monitoring**: The `FileMonitoringService` starts watching the solution directory for all changes *immediately* on application startup, even before it knows which files are part of the solution. It records any file change events, including the file path and the time of the change, into a temporary backlog. +1. **Early Monitoring**: The `FileMonitoringService` starts watching the solution directory for all changes *immediately* on application startup, even before it knows which files are part of the solution. It records the paths of any changed files into a temporary backlog. -2. **Late Binding of Known Files**: After the `SolutionManager` has finished loading the solution, it provides the `FileMonitoringService` with the definitive set of "known" solution file paths. +2. **Transitioning to the next monitoring phase **: After the `SolutionManager` has finished loading the solution, it provides the `FileMonitoringService` with the definitive set of "known" solution file paths. Upon receiving the set of known files, the service reconciles its backlog of recorded changes. If any of the newly provided "known" files appear in the backlog, it means they were changed during the startup process. In this case, an internal flag hadChangesWhileLoadingSolution is set to true which will cause AssessIfReloadNecessary to return true until monitoring is restarted. -3. **Backlog Reconciliation**: Upon receiving the set of known files, the service reconciles its backlog of recorded changes. If any of the newly provided "known" files appear in the backlog, it means they were changed during the startup process. In this case, the `IsReloadNeeded` flag is immediately set to `true` to force a workspace reload on the next request, ensuring synchronization. - -This approach closes the gap and guarantees that any changes occurring during the solution load process are correctly detected. - -## Change Detection Logic - -### Initial Reconciliation (when known files are set) -For each file in the provided set of known solution files: -1. Does this file exist in the backlog of changes recorded since monitoring began? - - Yes -> Set IsReloadNeeded = true and stop monitoring - - No -> Continue. -2. Clear the backlog - -### Real-time Change Detection (after initialization) -For each file change detected by FileSystemWatcher: -1. Was an expected change registered for this file? - - No → Set IsReloadNeeded = true (external change) and stop monitoring - - Yes → Remove file from set of expected changes - -``` +3. ** Regular Monitoring** Once the set of known solution files known then a record is kept of which of those files receive change notifications. At the same time any expected changes that are registered are kept, storing the last expected content for each registered file. When AssessIfReloadNecessary is called the list of changed relevant files are considered. If any of them don't have an expected change registered then a reload is considered necessary. If the changed file has a registered expected change then a reload is considered necessary if the file's actual content doesn't match the last registered expected content. ## Error Handling Strategy From ca8153612ebf90b1e9d43f83ca5fdf113839fb4b Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 14:30:51 -0300 Subject: [PATCH 09/12] adding an empty test file for FileMonitoringService with setup code for a working directory --- .../Services/FileMonitoringServicesTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs diff --git a/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs new file mode 100644 index 0000000..2146725 --- /dev/null +++ b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs @@ -0,0 +1,31 @@ +using Microsoft.Extensions.Logging; +using SharpTools.Tools.Interfaces; +using SharpTools.Tools.Services; +using Xunit; + +namespace SharpTools.Tools.Tests.Services; + +public class FileMonitoringServicesTests : IDisposable +{ + private readonly ILogger _logger; + private readonly string _testDirectory; + private readonly FileMonitoringService _service; + + public FileMonitoringServicesTests() + { + _logger = new LoggerFactory().CreateLogger(); + _testDirectory = Path.Combine(Path.GetTempPath(), "FileMonitoringTests", Guid.NewGuid().ToString()); + Directory.CreateDirectory(_testDirectory); + _service = new FileMonitoringService(_logger); + } + + public void Dispose() + { + _service?.Dispose(); + if (Directory.Exists(_testDirectory)) + { + Directory.Delete(_testDirectory, true); + } + } + +} \ No newline at end of file From 07b11b62419127c18021777270cf7d88fda60204 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 14:33:10 -0300 Subject: [PATCH 10/12] whitespace cleanup for filemon.design.md --- docs/filemon.design.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/docs/filemon.design.md b/docs/filemon.design.md index 1405808..0e26601 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -2,8 +2,7 @@ ## Overview -The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It monitors all solution files for unexpected changes so the solution can be reloaded -when commands are run after external changes to solution files. +The file monitoring system provides intelligent detection of external file changes to keep the MCP server's Roslyn workspace synchronized with the file system. It monitors all solution files for unexpected changes so the solution can be reloaded when commands are run after external changes to solution files. ## Core Concepts @@ -14,9 +13,7 @@ when commands are run after external changes to solution files. ### Comprehensive file monitoring -The file monitoring service is starts watching the solution folder recursively before the solution is loaded. Once the -solution is loaded information from Roslyn is used to control what files can actually trigger a reload. All MCP operations -will check if a reload is needed before continuing their operation. +The file monitoring service is starts watching the solution folder recursively before the solution is loaded. Once the solution is loaded information from Roslyn is used to control what files can actually trigger a reload. All MCP operations will check if a reload is needed before continuing their operation. Monitoring always ignores certain directories altogether: ".git", "bin" and "obj". @@ -29,10 +26,7 @@ This allows file monitoring to ignore expected changes and prevent unnecessary r ### Integration -A singleton implementation of IFileMonitoringService tracks changes. SolutionManager makes calls indicating when monitoring -should start and what files are known to be relevant. DocumentOperationService notifies when any expected changes are written. -ToolsHelper's EnsureSolutionLoaded* methods, which are called at the start of every MCP operation, notify SolutionManager to -reload the solution if appropriate. +A singleton implementation of IFileMonitoringService tracks changes. SolutionManager makes calls indicating when monitoring should start and what files are known to be relevant. DocumentOperationService notifies when any expected changes are written. ToolsHelper's EnsureSolutionLoaded* methods, which are called at the start of every MCP operation, notify SolutionManager to reload the solution if appropriate. ## File Discovery and Monitoring Strategy From 3f0cbd62a715f519b5f26b81dd2a8836f39cc800 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 17:40:31 -0300 Subject: [PATCH 11/12] trying to implement FileMonitoringService (tests are flaky, casing may be an issue) --- .../Services/FileChangeListenerTests.cs | 20 ++ .../Services/FileMonitoringServicesTests.cs | 311 ++++++++++++++++-- .../Services/FileChangeListener.cs | 233 +++++++++++++ .../Services/FileMonitoringService.cs | 119 ++++--- docs/filemon.design.md | 2 +- 5 files changed, 617 insertions(+), 68 deletions(-) create mode 100644 SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs create mode 100644 SharpTools.Tools/Services/FileChangeListener.cs diff --git a/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs b/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs new file mode 100644 index 0000000..1617508 --- /dev/null +++ b/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs @@ -0,0 +1,20 @@ +using SharpTools.Tools.Services; + +namespace SharpTools.Tools.Tests.Services { + public class FileChangeListenerTests { + [Theory] + [InlineData("c:\\root\\obj", true)] + [InlineData("c:\\root\\.git", true)] + [InlineData("c:\\root\\bin", true)] + [InlineData("c:\\root\\BIN", true)] + [InlineData("c:\\root\\foo", false)] + [InlineData("c:\\root\\nested\\obj", true)] + [InlineData("c:\\root\\nested\\.git", true)] + [InlineData("c:\\root\\nested\\bin", true)] + [InlineData("c:\\root\\nested\\BIN", true)] + [InlineData("c:\\root\\nested\\foo", false)] + public void IsPathIgnored_ignores_git_bin_obj(string path, bool isIgnored) { + Assert.Equal(isIgnored, FileChangeListener.IsPathIgnored("c:\\root", path, ['\\'])); + } + } +} \ No newline at end of file diff --git a/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs index 2146725..a6f48d9 100644 --- a/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs +++ b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs @@ -1,31 +1,282 @@ -using Microsoft.Extensions.Logging; -using SharpTools.Tools.Interfaces; -using SharpTools.Tools.Services; -using Xunit; - -namespace SharpTools.Tools.Tests.Services; - -public class FileMonitoringServicesTests : IDisposable -{ - private readonly ILogger _logger; - private readonly string _testDirectory; - private readonly FileMonitoringService _service; - - public FileMonitoringServicesTests() - { - _logger = new LoggerFactory().CreateLogger(); - _testDirectory = Path.Combine(Path.GetTempPath(), "FileMonitoringTests", Guid.NewGuid().ToString()); - Directory.CreateDirectory(_testDirectory); - _service = new FileMonitoringService(_logger); - } - - public void Dispose() - { - _service?.Dispose(); - if (Directory.Exists(_testDirectory)) - { - Directory.Delete(_testDirectory, true); - } - } - +using Microsoft.Extensions.Logging; +using SharpTools.Tools.Interfaces; +using SharpTools.Tools.Services; +using Xunit; + +namespace SharpTools.Tools.Tests.Services; + +public class FileMonitoringServicesTests : IDisposable +{ + private readonly ILogger _logger; + private readonly string _testDirectory; + private readonly FileMonitoringService _service; + + public FileMonitoringServicesTests() + { + _logger = new LoggerFactory().CreateLogger(); + _testDirectory = Path.Combine(Path.GetTempPath(), "FileMonitoringTests", Guid.NewGuid().ToString()); + Directory.CreateDirectory(_testDirectory); + _service = new FileMonitoringService(_logger); + } + + public void Dispose() + { + _service?.Dispose(); + if (Directory.Exists(_testDirectory)) + { + Directory.Delete(_testDirectory, true); + } + } + + [Fact] + public async Task AssessIfReloadNecessary_NoChanges_ReturnsFalse() + { + // Arrange + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(new HashSet()); + + // Act + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_FileChangedBeforeSetKnownFilePaths_ReturnsTrue() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + _service.StartMonitoring(_testDirectory); + File.WriteAllText(filePath, "changed content"); + var knownFilePaths = new HashSet { filePath }; + + // Act + _service.SetKnownFilePaths(knownFilePaths); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + + [Fact] + public async Task AssessIfReloadNecessary_FileDeletedBeforeSetKnownFilePaths_ReturnsTrue() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + _service.StartMonitoring(_testDirectory); + File.Delete(filePath); + var knownFilePaths = new HashSet { filePath }; + + // Act + _service.SetKnownFilePaths(knownFilePaths); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_KnownFileChangedAfterSetKnownFilePaths_ReturnsTrue() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + + // Act + File.WriteAllText(filePath, "changed content"); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_KnownFileDeletedAfterSetKnownFilePaths_ReturnsTrue() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + + // Act + File.Delete(filePath); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_KnownFileDirectoryDeletedAfterSetKnownFilePaths_ReturnsTrue() + { + // Arrange + var subdirectory = Path.Combine(_testDirectory, "bar"); + Directory.CreateDirectory(subdirectory); + var filePath = Path.Combine(subdirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + + // Act + Directory.Delete(subdirectory, true); + + Assert.False(File.Exists(filePath)); + + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_KnownFileDirectoryChangedAfterSetKnownFilePaths_ReturnsTrue() + { + // Renaming the directory is effectively the same as deleting the file from the monitor's perspective. + // Rather than look for directory rename events, the AssessIfReloadNecessary method will simply check + // if any relevant files are missing. + + // Arrange + var subdirectory = Path.Combine(_testDirectory, "bar"); + Directory.CreateDirectory(subdirectory); + var filePath = Path.Combine(subdirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + + // Act + var newSubDirectory = Path.Combine(_testDirectory, "baz"); + Directory.Move(subdirectory, newSubDirectory); + + Assert.True(File.Exists(Path.Combine(newSubDirectory, "test.txt"))); + + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_ExpectedChange_ReturnsFalse() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + var newContent = "changed content"; + _service.RegisterExpectedChange(filePath, newContent); + + // Act + File.WriteAllText(filePath, newContent); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_ExpectedChangeContentMismatch_ReturnsTrue() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + _service.RegisterExpectedChange(filePath, "expected content"); + + // Act + File.WriteAllText(filePath, "unexpected content"); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.True(result); + } + + [Fact] + public async Task AssessIfReloadNecessary_UnknownFileChanged_ReturnsFalse() + { + // Arrange + var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(new HashSet()); + + // Act + File.WriteAllText(filePath, "changed content"); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData("bin")] + [InlineData("obj")] + [InlineData(".git")] + public async Task AssessIfReloadNecessary_IgnoresChangesInIgnoredDirectories_ReturnsFalse(string ignoredDir) + { + // Arrange + var dir = Path.Combine(_testDirectory, ignoredDir); + Directory.CreateDirectory(dir); + var filePath = Path.Combine(dir, "test.txt"); + File.WriteAllText(filePath, "initial content"); + + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); + _service.SetKnownFilePaths(knownFilePaths); + + // Act + File.WriteAllText(filePath, "changed content"); + // Wait for the file watcher to pick up the change + await Task.Delay(100); + var result = await _service.AssessIfReloadNecessary(); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task Reload_is_considered_necessary_if_monitoring_is_not_running() + { + // Reload is necessary if monitoring hasn't even been started. + Assert.True(await _service.AssessIfReloadNecessary()); + + + _service.StartMonitoring(_testDirectory); + + // Verify starting monitoring changes the value we're testing + Assert.False(await _service.AssessIfReloadNecessary()); + + _service.StopMonitoring(); + + // Reload is necessary if monitoring has been stopped. + Assert.True(await _service.AssessIfReloadNecessary()); + } } \ No newline at end of file diff --git a/SharpTools.Tools/Services/FileChangeListener.cs b/SharpTools.Tools/Services/FileChangeListener.cs new file mode 100644 index 0000000..348a5ad --- /dev/null +++ b/SharpTools.Tools/Services/FileChangeListener.cs @@ -0,0 +1,233 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; + +namespace SharpTools.Tools.Services +{ + public class FileChangeListener : IDisposable { + private static readonly char[] pathSeparators = [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar]; + private readonly ILogger _logger; + private readonly object _lock = new(); + private readonly FileSystemWatcher _watcher; + private bool _enabled = true; + + private readonly List _knownFilePaths = new (); + private readonly List _changedFiles = new(); + private readonly Dictionary _expectedChanges = new(StringComparer.OrdinalIgnoreCase); + private bool _reloadIsNecessary; + + public FileChangeListener(ILogger logger, string directory) + { + _logger = logger; + _watcher = new FileSystemWatcher(directory) + { + NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName, + IncludeSubdirectories = true, + EnableRaisingEvents = true + }; + + _watcher.Changed += OnFileChanged; + _watcher.Created += OnFileChanged; + _watcher.Deleted += OnFileChanged; + _watcher.Renamed += OnFileRenamed; + _watcher.Error += OnError; + } + + public void SetKnownFilePaths(ISet filePathsToWatch) + { + lock (_lock) { + if (_knownFilePaths.Any()) + throw new InvalidOperationException($"{nameof(SetKnownFilePaths)} should only be called once."); + + _logger.LogInformation("Setting known file paths: {FileCount} files", filePathsToWatch.Count); + _knownFilePaths.AddRange(filePathsToWatch); + + if (_changedFiles.Any(changedFile => _knownFilePaths.Contains(changedFile))) + { + _reloadIsNecessary = true; + _logger.LogWarning("File changed during solution load."); + } + + _changedFiles.Clear(); + } + } + + public async Task AssessIfReloadNecessary() { + + Dictionary? expectedChanges = null; + HashSet? knownFilePaths = null; + List? changedFiles = null; + + bool needReload; + + lock (_lock) { + + needReload = _reloadIsNecessary; + + if (!needReload) { + expectedChanges = _expectedChanges.ToDictionary(); + knownFilePaths = new HashSet(_knownFilePaths); + changedFiles = _changedFiles.ToList(); + } + } + + if (!needReload) { + needReload = await AssessIfReloadNecessaryInternal(_logger, expectedChanges!, knownFilePaths!, changedFiles!); + } + + if (needReload) { + // Disable this listener as it no longer needs to monitor files. + Disable(); + } + + return needReload; + } + + private static async Task AssessIfReloadNecessaryInternal( + ILogger logger, + IReadOnlyDictionary expectedChanges, + IReadOnlyCollection knownFilePaths, + IReadOnlyCollection changedFiles) + { + foreach (var changedFile in changedFiles.Distinct(StringComparer.OrdinalIgnoreCase)) + { + var isKnown = knownFilePaths.Contains(changedFile);; + + if (isKnown) + { + if (!File.Exists(changedFile)) + { + logger.LogWarning("Known file {File} was deleted.", changedFile); + return true; + } + + var hasExpectedChange = expectedChanges.TryGetValue(changedFile, out var expectedContent); + + if (hasExpectedChange) + { + try + { + var actualContent = await File.ReadAllTextAsync(changedFile); + if (actualContent != expectedContent) + { + logger.LogWarning("File content mismatch for {File}", changedFile); + return true; + } + } + catch (Exception ex) + { + logger.LogWarning(ex, "Error reading file {File} for comparison", changedFile); + return true; + } + } + else + { + logger.LogWarning("Unexpected file change for {File}", changedFile); + return true; + } + } + } + + return false; + } + + public void RegisterExpectedChange(string filePath, string fileContents) + { + lock (_lock) + { + _logger.LogTrace("Registering expected change to {FilePath}", filePath); + _expectedChanges[filePath] = fileContents; + } + } + + public static bool IsPathIgnored(string basePath, string path, char[] pathSeparators) + { + var relativePath = Path.GetRelativePath(basePath, path); + var pathParts = relativePath.Split(pathSeparators); + if (pathParts.Length > 0) + { + + if (pathParts.Any(part => part.Equals(".git", StringComparison.OrdinalIgnoreCase) || + part.Equals("bin", StringComparison.OrdinalIgnoreCase) || + part.Equals("obj", StringComparison.OrdinalIgnoreCase))) + { + return true; + } + } + return false; + } + + private void OnFileChanged(object sender, FileSystemEventArgs e) + { + lock (_lock) + { + if (!_enabled) return; + if (IsPathIgnored(_watcher.Path, e.FullPath, pathSeparators)) + { + _logger.LogTrace("Ignoring change in {File} because it is in an ignored directory.", e.FullPath); + return; + } + _logger.LogTrace("File change event: {ChangeType} for {File}", e.ChangeType, e.FullPath); + _changedFiles.Add(e.FullPath); + } + } + + private void OnFileRenamed(object sender, RenamedEventArgs e) + { + lock (_lock) + { + if (!_enabled) return; + _logger.LogTrace("File rename event: {OldFile} to {NewFile}", e.OldFullPath, e.FullPath); + + var oldPath = e.OldFullPath; + + if (_knownFilePaths.Any(p => p.StartsWith(oldPath + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))) + { + _logger.LogInformation("Directory with known files renamed from {OldPath}. Forcing reload.", oldPath); + _reloadIsNecessary = true; + return; + } + + if (!IsPathIgnored(_watcher.Path, e.OldFullPath, pathSeparators)) + { + _changedFiles.Add(e.OldFullPath); + } + if (!IsPathIgnored(_watcher.Path, e.FullPath, pathSeparators)) + { + _changedFiles.Add(e.FullPath); + } + } + } + + private void OnError(object sender, ErrorEventArgs e) + { + lock (_lock) + { + if (!_enabled) return; + _logger.LogError(e.GetException(), "File system watcher error."); + _reloadIsNecessary = true; + } + } + + public void Disable() + { + _watcher.EnableRaisingEvents = false; + _enabled = false; + _reloadIsNecessary = true; + lock (_lock) + { + // Wait for any in-progress events to complete + } + } + + public void Dispose() + { + _watcher.Dispose(); + } + } +} \ No newline at end of file diff --git a/SharpTools.Tools/Services/FileMonitoringService.cs b/SharpTools.Tools/Services/FileMonitoringService.cs index 3e4a92e..f3da8de 100644 --- a/SharpTools.Tools/Services/FileMonitoringService.cs +++ b/SharpTools.Tools/Services/FileMonitoringService.cs @@ -1,49 +1,94 @@ +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using SharpTools.Tools.Interfaces; -namespace SharpTools.Tools.Services; - -/// -/// Stub implementation of file monitoring service. -/// This implementation has no side effects and does not trigger reloads. -/// -public sealed class FileMonitoringService : IFileMonitoringService +namespace SharpTools.Tools.Services { - private readonly ILogger _logger; - private bool _disposed; - - public FileMonitoringService(ILogger logger) + public sealed class FileMonitoringService : IFileMonitoringService { - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - } + private readonly ILogger _logger; + private readonly object _lock = new(); + + // _listener has its own lock. To prevent deadlock we never pick up _lock while _listener + // might be locked. We may hold _lock while calling into _listener, which is necessary for + // threadsafe cleanup. + + private FileChangeListener? _listener; + private bool _disposed; - public void StartMonitoring(string directory) - { - _logger.LogInformation("Starting file monitoring for directory: {Directory}", directory); - } + public FileMonitoringService(ILogger logger) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } - public void StopMonitoring() - { - _logger.LogInformation("Stopping file monitoring"); - } + public void StartMonitoring(string directory) + { + lock (_lock) + { + StopMonitoring(); - public void SetKnownFilePaths(ISet filePathsToWatch) - { - _logger.LogInformation("Setting known file paths: {FileCount} files", filePathsToWatch.Count); - } - public Task AssessIfReloadNecessary() { - return Task.FromResult(false); - } + _logger.LogInformation("Starting file monitoring for directory: {Directory}", directory); + _listener = new FileChangeListener(_logger, directory); + } + } - public void RegisterExpectedChange(string filePath, string fileContents) - { - _logger.LogTrace("Registering expected change to {FilePath}", filePath); - } + public void StopMonitoring() + { + lock (_lock) + { + var listenerToDisable = _listener; + _listener = null; + + // StopMonitoring is called from StartMonitoring within the lock so there is no point in doing + // this cleanup outside the lock. + if (listenerToDisable != null) + { + listenerToDisable.Disable(); + listenerToDisable.Dispose(); + } + } + } + + private FileChangeListener? ThreadSafeGetCurrentListener() + { + FileChangeListener? listener; + lock (_lock) { + listener = _listener; + } + return listener; + } - public void Dispose() - { - if (_disposed) return; + public void SetKnownFilePaths(ISet filePathsToWatch) { + FileChangeListener? listener = ThreadSafeGetCurrentListener(); + + listener?.SetKnownFilePaths(filePathsToWatch); + } + + public async Task AssessIfReloadNecessary() + { + FileChangeListener? listener = ThreadSafeGetCurrentListener(); + + if (listener is null) + return true; + + return await listener.AssessIfReloadNecessary(); + } + + public void RegisterExpectedChange(string filePath, string fileContents) + { + FileChangeListener? listener = ThreadSafeGetCurrentListener(); + + listener?.RegisterExpectedChange(filePath, fileContents); + } + + public void Dispose() + { + if (_disposed) return; - StopMonitoring(); - _disposed = true; + StopMonitoring(); + _disposed = true; + } } } \ No newline at end of file diff --git a/docs/filemon.design.md b/docs/filemon.design.md index 0e26601..6a00cd5 100644 --- a/docs/filemon.design.md +++ b/docs/filemon.design.md @@ -9,7 +9,7 @@ The file monitoring system provides intelligent detection of external file chang ### Open Concerns - Is watching DocumentOperationService.WriteFileAsync sufficient for discovering all expected file changes? -- Comparison of filenames given case insensitivity on Windows? +- Comparison of filenames given case insensitivity on Windows? (Note WSL on Windows is case sensitive) ### Comprehensive file monitoring From db489217e66448b14bae4a6f10d6c26d688d1223 Mon Sep 17 00:00:00 2001 From: fschwiet Date: Tue, 16 Sep 2025 18:08:03 -0300 Subject: [PATCH 12/12] - fix to FileChangeListener.IsPathIgnored to check all path parts - replace arbitrary waits in tests with waits for change events from FileSystemWatcher --- .../CancellationTokenUtils.cs | 30 +++++++ SharpTools.Tools.Tests/Retry.cs | 80 +++++++++++++++++++ .../Services/FileChangeListenerTests.cs | 34 ++++---- .../Services/FileMonitoringServicesTests.cs | 52 +++++++++--- .../SharpTools.Tools.Tests.csproj | 7 +- .../Services/FileChangeListener.cs | 12 ++- .../Services/FileMonitoringService.cs | 11 ++- 7 files changed, 191 insertions(+), 35 deletions(-) create mode 100644 SharpTools.Tools.Tests/CancellationTokenUtils.cs create mode 100644 SharpTools.Tools.Tests/Retry.cs diff --git a/SharpTools.Tools.Tests/CancellationTokenUtils.cs b/SharpTools.Tools.Tests/CancellationTokenUtils.cs new file mode 100644 index 0000000..51cab62 --- /dev/null +++ b/SharpTools.Tools.Tests/CancellationTokenUtils.cs @@ -0,0 +1,30 @@ +using System.Collections.Immutable; + +namespace SharpTools.Tools.Tests; +public static class CancellationTokenUtils +{ + public static TimeoutWatcher ApplyTimeout(int seconds, ref CancellationToken cancellationToken) + { + var cts = new CancellationTokenSource(); + cts.CancelAfter(TimeSpan.FromSeconds(seconds)); + + var ct = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, cts.Token); + + cancellationToken = ct.Token; + + return new TimeoutWatcher(cts.Token, [cts, ct]); + } + + public record TimeoutWatcher( + CancellationToken CancellationToken, + ImmutableArray Disposables + ) : IDisposable + { + public bool TimedOut => CancellationToken.IsCancellationRequested; + + public void Dispose() { + foreach(var disposable in Disposables) + disposable.Dispose(); + } + } +} diff --git a/SharpTools.Tools.Tests/Retry.cs b/SharpTools.Tools.Tests/Retry.cs new file mode 100644 index 0000000..9b412eb --- /dev/null +++ b/SharpTools.Tools.Tests/Retry.cs @@ -0,0 +1,80 @@ +using System.Diagnostics; + +namespace SharpTools.Tools.Tests; + +[DebuggerStepThrough] +public static class Retry +{ + public static async Task Until(int timeLimit, Func> assertion) + { + var cancellationToken = TestContext.Current.CancellationToken; + + using var timeoutWatcher = CancellationTokenUtils.ApplyTimeout( + timeLimit, + ref cancellationToken + ); + + try + { + while (!cancellationToken.IsCancellationRequested) + { + var result = await assertion(); + + if (result) + return; + + await Task.Delay(TimeSpan.FromMilliseconds(200), cancellationToken); + } + } + catch (TaskCanceledException) + { + if (!timeoutWatcher.TimedOut) + throw; + } + + await assertion(); + } + + public static async Task UntilPasses(Action condition) + { + await UntilPasses(5, condition); + } + + public static async Task UntilPasses(Func condition) + { + await UntilPasses(5, condition); + } + + public static async Task UntilPasses(int timeLimit, Func condition) + { + await Until( + timeLimit, + async () => + { + try + { + await condition(); + return true; + } + catch + { + return false; + } + } + ); + + await condition(); + } + + public static async Task UntilPasses(int timeLimit, Action condition) + { + await UntilPasses( + timeLimit, + () => + { + condition(); + return Task.CompletedTask; + } + ); + } +} diff --git a/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs b/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs index 1617508..a8c48ae 100644 --- a/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs +++ b/SharpTools.Tools.Tests/Services/FileChangeListenerTests.cs @@ -1,20 +1,20 @@ using SharpTools.Tools.Services; -namespace SharpTools.Tools.Tests.Services { - public class FileChangeListenerTests { - [Theory] - [InlineData("c:\\root\\obj", true)] - [InlineData("c:\\root\\.git", true)] - [InlineData("c:\\root\\bin", true)] - [InlineData("c:\\root\\BIN", true)] - [InlineData("c:\\root\\foo", false)] - [InlineData("c:\\root\\nested\\obj", true)] - [InlineData("c:\\root\\nested\\.git", true)] - [InlineData("c:\\root\\nested\\bin", true)] - [InlineData("c:\\root\\nested\\BIN", true)] - [InlineData("c:\\root\\nested\\foo", false)] - public void IsPathIgnored_ignores_git_bin_obj(string path, bool isIgnored) { - Assert.Equal(isIgnored, FileChangeListener.IsPathIgnored("c:\\root", path, ['\\'])); - } +namespace SharpTools.Tools.Tests.Services; + +public class FileChangeListenerTests { + [Theory] + [InlineData("c:\\root\\obj", true)] + [InlineData("c:\\root\\.git", true)] + [InlineData("c:\\root\\bin", true)] + [InlineData("c:\\root\\BIN", true)] + [InlineData("c:\\root\\foo", false)] + [InlineData("c:\\root\\nested\\obj", true)] + [InlineData("c:\\root\\nested\\.git", true)] + [InlineData("c:\\root\\nested\\bin", true)] + [InlineData("c:\\root\\nested\\BIN", true)] + [InlineData("c:\\root\\nested\\foo", false)] + public void IsPathIgnored_ignores_git_bin_obj(string path, bool isIgnored) { + Assert.Equal(isIgnored, FileChangeListener.IsPathIgnored("c:\\root", path, ['\\'])); } -} \ No newline at end of file +} diff --git a/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs index a6f48d9..24ac856 100644 --- a/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs +++ b/SharpTools.Tools.Tests/Services/FileMonitoringServicesTests.cs @@ -48,12 +48,18 @@ public async Task AssessIfReloadNecessary_FileChangedBeforeSetKnownFilePaths_Ret // Arrange var filePath = Path.Combine(_testDirectory, "test.txt"); File.WriteAllText(filePath, "initial content"); + _service.StartMonitoring(_testDirectory); + + File.WriteAllText(filePath, "changed content"); var knownFilePaths = new HashSet { filePath }; // Act _service.SetKnownFilePaths(knownFilePaths); + + await Retry.UntilPasses(() => Assert.True( _service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -67,12 +73,18 @@ public async Task AssessIfReloadNecessary_FileDeletedBeforeSetKnownFilePaths_Ret // Arrange var filePath = Path.Combine(_testDirectory, "test.txt"); File.WriteAllText(filePath, "initial content"); + _service.StartMonitoring(_testDirectory); + File.Delete(filePath); + var knownFilePaths = new HashSet { filePath }; // Act _service.SetKnownFilePaths(knownFilePaths); + + await Retry.UntilPasses(() => Assert.True( _service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -84,15 +96,21 @@ public async Task AssessIfReloadNecessary_KnownFileChangedAfterSetKnownFilePaths { // Arrange var filePath = Path.Combine(_testDirectory, "test.txt"); + File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); _service.SetKnownFilePaths(knownFilePaths); + + Assert.Equal(0, _service.ChangeCount); // Act File.WriteAllText(filePath, "changed content"); - // Wait for the file watcher to pick up the change - await Task.Delay(100); + + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -106,13 +124,15 @@ public async Task AssessIfReloadNecessary_KnownFileDeletedAfterSetKnownFilePaths var filePath = Path.Combine(_testDirectory, "test.txt"); File.WriteAllText(filePath, "initial content"); var knownFilePaths = new HashSet { filePath }; + _service.StartMonitoring(_testDirectory); _service.SetKnownFilePaths(knownFilePaths); // Act File.Delete(filePath); - // Wait for the file watcher to pick up the change - await Task.Delay(100); + + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -127,6 +147,7 @@ public async Task AssessIfReloadNecessary_KnownFileDirectoryDeletedAfterSetKnown Directory.CreateDirectory(subdirectory); var filePath = Path.Combine(subdirectory, "test.txt"); File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; _service.StartMonitoring(_testDirectory); _service.SetKnownFilePaths(knownFilePaths); @@ -137,7 +158,8 @@ public async Task AssessIfReloadNecessary_KnownFileDirectoryDeletedAfterSetKnown Assert.False(File.Exists(filePath)); // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -156,6 +178,7 @@ public async Task AssessIfReloadNecessary_KnownFileDirectoryChangedAfterSetKnown Directory.CreateDirectory(subdirectory); var filePath = Path.Combine(subdirectory, "test.txt"); File.WriteAllText(filePath, "initial content"); + var knownFilePaths = new HashSet { filePath }; _service.StartMonitoring(_testDirectory); _service.SetKnownFilePaths(knownFilePaths); @@ -167,7 +190,8 @@ public async Task AssessIfReloadNecessary_KnownFileDirectoryChangedAfterSetKnown Assert.True(File.Exists(Path.Combine(newSubDirectory, "test.txt"))); // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -188,8 +212,10 @@ public async Task AssessIfReloadNecessary_ExpectedChange_ReturnsFalse() // Act File.WriteAllText(filePath, newContent); + // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -209,8 +235,10 @@ public async Task AssessIfReloadNecessary_ExpectedChangeContentMismatch_ReturnsT // Act File.WriteAllText(filePath, "unexpected content"); + // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -228,8 +256,10 @@ public async Task AssessIfReloadNecessary_UnknownFileChanged_ReturnsFalse() // Act File.WriteAllText(filePath, "changed content"); + // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert @@ -254,8 +284,10 @@ public async Task AssessIfReloadNecessary_IgnoresChangesInIgnoredDirectories_Ret // Act File.WriteAllText(filePath, "changed content"); + // Wait for the file watcher to pick up the change - await Task.Delay(100); + await Retry.UntilPasses(() => Assert.True(_service.ChangeCount >= 1)); + var result = await _service.AssessIfReloadNecessary(); // Assert diff --git a/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj index 3170b79..adb0f0e 100644 --- a/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj +++ b/SharpTools.Tools.Tests/SharpTools.Tools.Tests.csproj @@ -9,9 +9,10 @@ - - - + + + + diff --git a/SharpTools.Tools/Services/FileChangeListener.cs b/SharpTools.Tools/Services/FileChangeListener.cs index 348a5ad..5a51f35 100644 --- a/SharpTools.Tools/Services/FileChangeListener.cs +++ b/SharpTools.Tools/Services/FileChangeListener.cs @@ -20,6 +20,8 @@ public class FileChangeListener : IDisposable { private readonly List _changedFiles = new(); private readonly Dictionary _expectedChanges = new(StringComparer.OrdinalIgnoreCase); private bool _reloadIsNecessary; + + public int ChangeCount { get; private set; } // For testing purposes public FileChangeListener(ILogger logger, string directory) { @@ -164,8 +166,9 @@ public static bool IsPathIgnored(string basePath, string path, char[] pathSepara private void OnFileChanged(object sender, FileSystemEventArgs e) { - lock (_lock) - { + lock (_lock) { + ChangeCount++; + if (!_enabled) return; if (IsPathIgnored(_watcher.Path, e.FullPath, pathSeparators)) { @@ -179,8 +182,9 @@ private void OnFileChanged(object sender, FileSystemEventArgs e) private void OnFileRenamed(object sender, RenamedEventArgs e) { - lock (_lock) - { + lock (_lock) { + ChangeCount++; + if (!_enabled) return; _logger.LogTrace("File rename event: {OldFile} to {NewFile}", e.OldFullPath, e.FullPath); diff --git a/SharpTools.Tools/Services/FileMonitoringService.cs b/SharpTools.Tools/Services/FileMonitoringService.cs index f3da8de..bf2402b 100644 --- a/SharpTools.Tools/Services/FileMonitoringService.cs +++ b/SharpTools.Tools/Services/FileMonitoringService.cs @@ -17,7 +17,16 @@ public sealed class FileMonitoringService : IFileMonitoringService private FileChangeListener? _listener; private bool _disposed; - + + /* ChangeCount is exported so tests can wait for events to come in */ + public int ChangeCount { + get { + var listener = ThreadSafeGetCurrentListener(); + + return listener?.ChangeCount ?? -1; + } + } + public FileMonitoringService(ILogger logger) { _logger = logger ?? throw new ArgumentNullException(nameof(logger));