Skip to content

fix: file explorer auto-refresh with visual feedback and legacy migration#335

Merged
pedramamini merged 1 commit intomainfrom
fix-file-auto-refresh
Feb 15, 2026
Merged

fix: file explorer auto-refresh with visual feedback and legacy migration#335
pedramamini merged 1 commit intomainfrom
fix-file-auto-refresh

Conversation

@pedramamini
Copy link
Copy Markdown
Collaborator

Summary

  • Auto-refresh in File Explorer now shows a brief spinner pulse so users can see it's working
  • Legacy sessions without fileTreeAutoRefreshInterval are migrated to the 180s default on load
  • Right panel re-renders when auto-refresh interval changes (added to useMemo deps)

Changes

File What
FileExplorerPanel.tsx Auto-refresh callback now awaits the refresh and shows a 500ms spin animation
App.tsx Migration: backfill fileTreeAutoRefreshInterval: 180 for legacy sessions
useRightPanelProps.ts Added fileTreeAutoRefreshInterval to memo dependency array
FileExplorerPanel.test.tsx Upgraded timer tests to async (advanceTimersByTimeAsync); added spinner start/stop coverage
useInputProcessing.test.ts Isolated fake timers per test to prevent cross-test timer leaks

Test plan

  • All 18,585 tests pass (0 failures)
  • Open File Explorer, set auto-refresh interval, confirm spinner pulses on each cycle
  • Load a session created before this change, verify it gets the 180s default
  • Change the auto-refresh interval in settings while the right panel is open — confirm it takes effect immediately

- Added visible auto-refresh spinner pulse in File Explorer refresh cycles 🔄
- Migrated legacy sessions to default 180s file-tree auto-refresh interval 🧭
- Auto-refresh now awaits refresh completion before stopping indicator ⏱️
- Right panel props now react to auto-refresh interval changes instantly 🧩
- File explorer timer tests upgraded to async timer advancement for stability 🧪
- New coverage ensures spinner starts and stops correctly after auto-refresh 💫
- Input processing tests now isolate fake timers to prevent cross-test leaks 🧯
@claude
Copy link
Copy Markdown

claude bot commented Feb 10, 2026

Code Review

Summary

This PR adds visual feedback for file explorer auto-refresh and includes migration logic for legacy sessions. The implementation is clean and well-tested.


Strengths

  1. Excellent test coverage: All timer tests properly upgraded to async (advanceTimersByTimeAsync), preventing potential race conditions in the test suite.

  2. Proper test isolation: The useInputProcessing.test.ts changes fix cross-test timer leaks by adding useFakeTimers()/useRealTimers() pairs. This is a solid preventive fix.

  3. Migration logic: Legacy sessions get a sensible default (180s) on load, preventing undefined behavior.

  4. User feedback: The 500ms spinner pulse gives users visual confirmation that auto-refresh is working.

  5. Proper cleanup: The try/finally pattern ensures setIsRefreshing(false) runs even if refresh fails.


Issues Found

Critical: Missing test infrastructure

The test at line 737 expects screen.getByTestId('refresh-icon') but no data-testid attribute exists in the component (FileExplorerPanel.tsx:1129). Without this attribute, the test will fail.

Fix needed: Add data-testid="refresh-icon" to the RefreshCw element at line 1129.


Minor: Potential timer leak

The setTimeout in line 499 is not tracked, so it could fire after unmount. Consider storing the timeout ID in a ref and clearing it in the cleanup function to prevent React warnings if the component unmounts during the 500ms window.


Minor: Unnecessary async wrapper

The interval callback is async but does not need to be. setInterval does not care about the return value. The await is fine (ensures refresh completes before spinner stops), but the async keyword on the arrow function has no effect. Not harmful, but unnecessary.


Checklist Before Merge

  • Add data-testid="refresh-icon" to the RefreshCw element (line 1129)
  • Verify all tests pass after adding the test ID
  • (Optional) Clean up the timeout reference to prevent post-unmount setState

Verdict

Approve with required fix. The missing data-testid will cause test failures. Once that is added, this is a solid improvement with good test hygiene and user experience enhancements.

@pedramamini pedramamini merged commit 443f903 into main Feb 15, 2026
2 checks passed
@pedramamini pedramamini deleted the fix-file-auto-refresh branch March 18, 2026 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant