Merged
Conversation
6 tasks
claude Bot
pushed a commit
that referenced
this pull request
Oct 4, 2025
…king Comprehensive fixes for issues identified in PR review: - Fix #1: Race condition in file switching - synchronous state capture before async operations - Fix #2: Cache invalidation on external file changes - file watcher now clears stale cache - Fix #3: Auto-save disabled to prevent conflicts with cache restoration - Fix #4: Memory leak prevention - cache cleaned on directory change and file watcher events - Fix #5: Document dirty state persistence behavior (intentionally not persisted) - Fix #6: Loading gate prevents concurrent file loading operations - Fix #7: True LRU eviction using timestamps instead of FIFO - Fix #8: Cache integrity validation via timestamps - Fix #9: Improved ESLint comment explaining dependency exclusions Key improvements: - Added timestamp field to FileContentCache for true LRU and validation - Added loadingFileRef gate to serialize file loading and prevent race conditions - Re-enabled file watcher with cache invalidation on external changes - Updated timestamps on cache access for proper LRU behavior - Comprehensive documentation in comments explaining design decisions Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com>
claude Bot
pushed a commit
that referenced
this pull request
Oct 4, 2025
…king Critical fixes: - Fix #1: Auto-save race condition - capture file path when setting timer and validate before saving - Fix #2: Cache invalidation race condition - use synchronous cleanup with validPaths from file tree - Fix #3: Memory leak - cache now clears when directory closed (rootPath becomes null) Medium priority fixes: - Fix #4: Inconsistent dirty state - always clear cache and dirty state for externally modified files - Fix #5: Loading state error handling - added try/catch with toast notifications for file load errors - Fix #6: Cache validation error handling - wrapped validation in try/catch with proper error messages Minor fixes: - Fix #7: Removed unused cleanupDeletedFilesFromCache function - Fix #8: Improved ESLint comment to list specific excluded dependencies - Fix #9: Extracted auto-save delay to AUTO_SAVE_DELAY_MS constant - Fix #10: Fixed FileWatcherEvent type signature to accept undefined Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com>
saadiq
added a commit
that referenced
this pull request
Oct 4, 2025
#7) * feat: implement multi-file dirty state tracking with visual indicators Add comprehensive dirty state management across multiple files to support a true multi-document editing experience. Changes persist when switching between files and are visually indicated in the file tree. Key changes: - Added dirtyPaths Set to FileTreeState for tracking unsaved changes across all files - Implemented file content cache in App.tsx to preserve unsaved edits during file switching - Added setFileDirty action to FileTreeActions for dirty state management - Enhanced FileTreeItem to display orange dot indicator and italic text for dirty files - Integrated dirty state synchronization on save operations and file changes - Cache automatically clears on successful file save This enables users to work on multiple files simultaneously without losing unsaved changes when switching contexts. * fix: improve multi-file cache management and dirty state cleanup - Clear file content cache on directory change to prevent stale data - Add cache size limit (10 files) with LRU eviction policy - Clean up dirty paths when files are deleted externally - Fix race condition by capturing file state synchronously before async operations Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address all code review concerns for multi-file dirty state tracking Comprehensive fixes for issues identified in PR review: - Fix #1: Race condition in file switching - synchronous state capture before async operations - Fix #2: Cache invalidation on external file changes - file watcher now clears stale cache - Fix #3: Auto-save disabled to prevent conflicts with cache restoration - Fix #4: Memory leak prevention - cache cleaned on directory change and file watcher events - Fix #5: Document dirty state persistence behavior (intentionally not persisted) - Fix #6: Loading gate prevents concurrent file loading operations - Fix #7: True LRU eviction using timestamps instead of FIFO - Fix #8: Cache integrity validation via timestamps - Fix #9: Improved ESLint comment explaining dependency exclusions Key improvements: - Added timestamp field to FileContentCache for true LRU and validation - Added loadingFileRef gate to serialize file loading and prevent race conditions - Re-enabled file watcher with cache invalidation on external changes - Updated timestamps on cache access for proper LRU behavior - Comprehensive documentation in comments explaining design decisions Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address all code review concerns for multi-file dirty state tracking This commit implements comprehensive fixes for all issues identified in the code review: Critical Fixes: - Re-enabled auto-save with cache-aware implementation that prevents saving cached content to wrong files - Fixed race condition in cache update by adding switchingFileRef to track file switch intent - Added cache cleanup for deleted files via file watcher validation - Implemented external file change handling with conflict detection for active file Medium Priority Fixes: - Added cache validation on restoration by comparing disk content with cached originalContent - Implemented loading state (isLoadingFile) during file switches for better UX Minor Fixes: - Improved type safety by importing and using FileWatcherEvent type instead of 'any' Implementation Details: - Auto-save timer validates file path before saving to prevent wrong-file saves - Cache restoration now checks if disk content matches cached originalContent - File watcher detects active file changes and shows conflict warning if dirty - Cache entries for deleted files are cleaned up asynchronously after directory reload - Loading spinner shows during file switch operations - All race conditions addressed with proper ref-based state tracking 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address all code review concerns for multi-file dirty state tracking Critical fixes: - Fix #1: Auto-save race condition - capture file path when setting timer and validate before saving - Fix #2: Cache invalidation race condition - use synchronous cleanup with validPaths from file tree - Fix #3: Memory leak - cache now clears when directory closed (rootPath becomes null) Medium priority fixes: - Fix #4: Inconsistent dirty state - always clear cache and dirty state for externally modified files - Fix #5: Loading state error handling - added try/catch with toast notifications for file load errors - Fix #6: Cache validation error handling - wrapped validation in try/catch with proper error messages Minor fixes: - Fix #7: Removed unused cleanupDeletedFilesFromCache function - Fix #8: Improved ESLint comment to list specific excluded dependencies - Fix #9: Extracted auto-save delay to AUTO_SAVE_DELAY_MS constant - Fix #10: Fixed FileWatcherEvent type signature to accept undefined Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address critical and medium priority code review concerns Critical fixes: - Fix stale closure in auto-save by using currentFilePathRef (useFileContent.ts) - Fix file watcher closure issues by using refs for activePath, fileContent, and toast (App.tsx) Medium priority fixes: - Optimize cache validation to avoid double file reads by reusing diskResult - Add cache cleanup when reloading active file from disk - Check all cached files (not just active) in beforeunload handler Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address critical and medium priority code review concerns - Fix Critical Issue #1: Remove state.filePath from updateContent dependencies - Prevents unnecessary re-creation of updateContent callback - Uses currentFilePathRef instead for path validation - Fix Critical Issue #2: Clear auto-save timer before manual content updates - Expose clearAutoSaveTimer from useFileContent hook - Call clearAutoSaveTimer before restoring cached content - Prevents stale auto-save timers from firing for wrong file - Remove unused switchingFileRef throughout App.tsx - Cleanup of tracking ref that was set but never read - Set up Vitest testing infrastructure - Add vitest.config.ts with React and happy-dom support - Create test setup file with Electron API mocks - Add App.cache.test.tsx with placeholder tests for cache management - Add test scripts to package.json (test, test:ui, test:coverage) - Add testing README with setup instructions and patterns Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address high priority code review concerns - Add useEffect cleanup for saveTimeoutRef to prevent memory leak - Implement comprehensive cache management tests (not placeholders) - Add tests for cache invalidation, dirty state sync, auto-save timer management - Add tests for loading states and memory leak prevention - Update README with complete test dependency list Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> * fix: address critical race conditions and file watcher issues - Fix #1: Add getCurrentState() method to useFileContent to avoid stale closures in cache state capture during async operations - Fix #2: Update saveFile to accept pathOverride parameter for auto-save validation, ensuring saved content goes to the correct file - Fix #3: Queue file watcher events during save window instead of dropping them, then process queued events after save completes These fixes prevent data corruption from race conditions in multi-file editing with auto-save enabled. Co-authored-by: Saadiq Rodgers-King <saadiq@users.noreply.github.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Installing Claude Code GitHub App
This PR adds a GitHub Actions workflow that enables Claude Code integration in our repository.
What is Claude Code?
Claude Code is an AI coding agent that can help with:
How it works
Once this PR is merged, we'll be able to interact with Claude by mentioning @claude in a pull request or issue comment.
Once the workflow is triggered, Claude will analyze the comment and surrounding context, and execute on the request in a GitHub action.
Important Notes
Security
There's more information in the Claude Code action repo.
After merging this PR, let's try mentioning @claude in a comment on any PR to get started!