Skip to content

feat: Add auto-refresh for file tree with configurable interval#14

Closed
pedramamini wants to merge 1 commit intomainfrom
claude/issue-11-20251203-1821
Closed

feat: Add auto-refresh for file tree with configurable interval#14
pedramamini wants to merge 1 commit intomainfrom
claude/issue-11-20251203-1821

Conversation

@pedramamini
Copy link
Collaborator

Summary

Implements auto-refresh for the Files tab in the right panel with a configurable interval setting.

Changes

  • Added fileTreeRefreshInterval setting (3s to 5min range, default 3s)
  • Added slider UI in Settings modal under General section
  • Implemented auto-refresh logic that:
    • Only runs when Files tab is active
    • Uses debouncing to prevent overlapping refreshes
    • Pauses when switching to other tabs
  • Manual refresh button remains available

Testing

  • Open Settings (Cmd+Comma) and navigate to General
  • Locate "File Tree Refresh Interval" slider
  • Adjust the interval between 3 seconds and 5 minutes
  • Switch to the Files tab and observe auto-refresh
  • Switch to History/Scratchpad to verify refresh pauses

Closes #11

Generated with Claude Code

- Add fileTreeRefreshInterval setting (3s to 5min range, default 3s)
- Add slider UI in Settings modal under General section
- Implement auto-refresh logic in FileExplorerPanel that:
  - Only runs when Files tab is active
  - Uses debouncing to prevent overlapping refreshes
  - Respects user-configured refresh interval
- Manual refresh button remains available for immediate updates

Resolves #11

Co-authored-by: Pedram Amini <pedramamini@users.noreply.github.com>
@pedramamini pedramamini self-assigned this Dec 3, 2025
@claude
Copy link

claude bot commented Dec 3, 2025

Code Review

✅ Overall Assessment

This is a well-implemented feature that follows the repository's coding conventions and patterns. The implementation is clean, properly structured, and addresses a real need for auto-refreshing file trees. Good work!

🎯 Strengths

  1. Follows CLAUDE.md conventions perfectly

    • Settings persistence pattern correctly implemented in useSettings.ts
    • Props properly threaded through component hierarchy ✅
    • Inline styles used for theme colors ✅
  2. Smart implementation details

    • Auto-refresh only runs when Files tab is active (lines 84-88 in FileExplorerPanel.tsx)
    • Debouncing via refreshInProgressRef prevents overlapping refreshes (lines 94-96)
    • Proper cleanup with interval clearing (line 109)
    • Manual refresh button retained for user control
  3. Good UX

    • Sensible default (3 seconds) and range (3s-5min)
    • Visual slider with current value display
    • Clear descriptive text in settings

🐛 Potential Issues

1. Missing dependency cleanup (FileExplorerPanel.tsx:110)

The useEffect hook includes refreshFileTree in the dependency array, but this function is likely recreated on every render in the parent component. This could cause the interval to restart unnecessarily.

Recommendation:

// In App.tsx, wrap refreshFileTree with useCallback
const refreshFileTree = useCallback(async (sessionId: string) => {
  // ... existing implementation
}, [/* stable dependencies */]);

Or handle it locally:

const refreshFileTreeRef = useRef(refreshFileTree);
refreshFileTreeRef.current = refreshFileTree;

useEffect(() => {
  if (activeRightTab !== 'files') return;
  
  const intervalMs = fileTreeRefreshInterval * 1000;
  const intervalId = setInterval(async () => {
    if (refreshInProgressRef.current) return;
    
    try {
      refreshInProgressRef.current = true;
      await refreshFileTreeRef.current(session.id);
    } catch (error) {
      console.error('Auto-refresh failed:', error);
    } finally {
      refreshInProgressRef.current = false;
    }
  }, intervalMs);
  
  return () => clearInterval(intervalId);
}, [activeRightTab, fileTreeRefreshInterval, session.id]);

2. Race condition on rapid interval changes

If a user rapidly adjusts the slider, the old interval keeps running until the next execution cycle, potentially causing a refresh burst.

Current flow:

  • Interval set to 3s → runs
  • User changes to 60s → old interval cleanup happens, new one starts
  • But old interval's pending callback still fires

Recommendation: Add early return check:

const activeIntervalRef = useRef<NodeJS.Timeout | null>(null);

useEffect(() => {
  if (activeRightTab !== 'files') return;
  
  const intervalMs = fileTreeRefreshInterval * 1000;
  const intervalId = setInterval(async () => {
    if (refreshInProgressRef.current) return;
    if (activeIntervalRef.current !== intervalId) return; // Added: prevent stale callbacks
    
    try {
      refreshInProgressRef.current = true;
      await refreshFileTree(session.id);
    } catch (error) {
      console.error('Auto-refresh failed:', error);
    } finally {
      refreshInProgressRef.current = false;
    }
  }, intervalMs);
  
  activeIntervalRef.current = intervalId;
  return () => {
    clearInterval(intervalId);
    if (activeIntervalRef.current === intervalId) {
      activeIntervalRef.current = null;
    }
  };
}, [activeRightTab, fileTreeRefreshInterval, session.id, refreshFileTree]);

🔒 Security Considerations

No security issues detected

  • No user input passed to shell commands
  • Settings properly sanitized (numeric range enforced in UI)
  • No injection vulnerabilities

⚡ Performance Considerations

  1. Current implementation is good, but consider:

    • The 3-second minimum might be aggressive for large projects
    • No throttling if file tree is very large
  2. Future enhancement idea (not blocking):

    • Add file watcher integration (chokidar/fs.watch) instead of polling
    • Only refresh when actual filesystem changes detected
    • Would be more efficient and provide instant updates

🧪 Test Coverage

No tests present - The repository has no test infrastructure (*.test.ts or *.spec.ts files).

Recommendation for future: Consider adding integration tests for:

  • Settings persistence
  • Auto-refresh behavior
  • Interval changes
  • Tab switching

📝 Minor Suggestions

  1. TypeScript strictness (SettingsModal.tsx:1175)
    Consider extracting magic numbers to constants:

    const FILE_TREE_REFRESH = {
      MIN: 3,
      MAX: 300,
      DEFAULT: 3,
    } as const;
  2. Accessibility (SettingsModal.tsx:1183)
    Add aria-label to the slider:

    <input
      type="range"
      aria-label="File tree refresh interval in seconds"
      // ... rest of props
    />
  3. Console.error usage (FileExplorerPanel.tsx:102)
    Consider using the app's logger for consistency:

    } catch (error) {
      window.maestro.logger.error('Auto-refresh failed:', error);
    }

🎉 Summary

This PR is ready to merge with minor considerations. The implementation is solid and follows best practices. The identified issues are edge cases that may or may not manifest in practice.

Priority fixes:

  • Consider the dependency cleanup issue (medium priority)
  • The race condition is low priority but worth addressing

Nice-to-haves:

  • Add constants for magic numbers
  • Improve accessibility
  • Use consistent logging

Great work implementing this feature! The debouncing logic and tab-awareness show thoughtful design. 🚀

@pedramamini
Copy link
Collaborator Author

Figured out a good way of doing this for now. WIll be included in next release.

@pedramamini pedramamini closed this Dec 4, 2025
@pedramamini pedramamini deleted the claude/issue-11-20251203-1821 branch December 4, 2025 12:16
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.

Feature Request: Files Tab Auto Refresh

1 participant

Comments