Director's Notes

[pedramamini]

• MAESTRO: Add Director's Notes settings types, useSettings integration, and SettingsModal tab
• MAESTRO: Extract shared History components from HistoryPanel
• MAESTRO: Wire up ActivityGraph import from History/ and remove local definition from HistoryPanel
• MAESTRO: Wire up HistoryEntryItem import from History/ and remove local definition from HistoryPanel
• MAESTRO: Wire up HistoryFilterToggle import from History/ and remove inline filter rendering from HistoryPanel
• MAESTRO: Wire up DoubleCheck import from History/ and remove local definition from HistoryDetailModal
• MAESTRO: Add Director's Notes IPC handlers for unified history and synopsis generation
• MAESTRO: Add DirectorNotesModal shell with tab switching between Unified History and AI Overview
• MAESTRO: Implement UnifiedHistoryTab with virtualized list, agent name display, and sessionless modal support
• MAESTRO: Fix HistoryDetailModal tests and add sessionless context test coverage for Director's Notes
• MAESTRO: Implement AIOverviewTab with synopsis generation, progress bar, and save-to-disk
• MAESTRO: Integrate Director's Notes into Command Palette, Hamburger Menu, and keyboard shortcuts
• MAESTRO: Add comprehensive tests for extracted History components (ActivityGraph, HistoryEntryItem, HistoryFilterToggle)
• MAESTRO: Add comprehensive tests for DirectorNotesModal component
• MAESTRO: Add comprehensive tests for Director's Notes IPC handlers
• MAESTRO: Add comprehensive tests for UnifiedHistoryTab component
• MAESTRO: Add Director's Notes settings tab tests and fix tab wrap assertions
• MAESTRO: Add Director's Notes Command Palette integration tests
• MAESTRO: Add Director's Notes Hamburger Menu integration tests
• MAESTRO: Add Director's Notes entries to CLAUDE.md key files table
• MAESTRO: Clean up TODO comments in Director's Notes implementation
• Fix infinite re-render loop and keyboard focus in DirectorNotesModal
• Fix keyboard focus in UnifiedHistoryTab list after data loads
• Implement AI synopsis generation via groomContext in Director's Notes
• Add custom agent configuration and session name resolution to Director's Notes
• Enhance Director's Notes UI with Help tab, search, and keyboard navigation
• Improve history entry display hierarchy for Director's Notes unified view
• Add Director's Notes agent configuration panel to Settings
• Refine Director's Notes modal layout, focus management, and keyboard shortcut migration
• Replace inline JSON data with file-path manifest for Director's Notes synopsis generation
• Add paginated loading, lookback filtering, and validation toggle to Director's Notes
• Standardize agent vs session terminology across docs and fix Director's Notes shortcut display
• Replace hardcoded keyboard shortcuts with platform-aware formatShortcutKeys across 25 components

[claude]

Code Review: Director's Notes Feature

This is an impressive implementation of a comprehensive cross-agent history aggregation and AI synopsis generation feature. The code quality is excellent overall, with strong adherence to the project's architectural patterns and coding standards. Here's my detailed feedback:

---

✅ Strengths

Architecture & Design

• Clean separation of concerns: IPC handlers, UI components, and business logic are well-organized
• Proper use of existing patterns: Follows established Maestro patterns (layer stack, modal priorities, theme system, settings persistence)
• Progressive loading: UnifiedHistoryTab uses pagination (100 entries/page) with infinite scroll instead of loading everything at once - excellent for performance
• Module-level caching: AIOverviewTab caches synopsis results across tab switches to avoid regeneration - smart UX optimization
• File-based history approach: Passing file paths to the AI agent instead of inline JSON is elegant and reduces prompt bloat

Code Quality

• Comprehensive test coverage: 649 lines of IPC handler tests, 528 lines for DirectorNotesModal, 654 for UnifiedHistoryTab, 337 for AIOverviewTab, plus shared History component tests
• Strong TypeScript usage: Well-defined interfaces, proper type safety across IPC boundaries
• Memory leak prevention: Proper use of mountedRef guards in AIOverviewTab to prevent setState on unmounted components
• Accessibility: Good use of ARIA attributes (role="dialog", aria-modal, aria-labelledby)
• Focus management: useImperativeHandle pattern for focus control, proper tab focus forwarding
• Keyboard navigation: Comprehensive keyboard support (Cmd+Shift+[/], Cmd+F search, arrow navigation)

Performance

• Virtualization: Uses @tanstack/react-virtual for efficient list rendering
• Memoization: Proper use of useMemo and useCallback to prevent unnecessary re-renders
• Lazy loading: Components are lazy-loaded with Suspense boundaries
• Debounced search: Search filtering happens client-side without thrashing

---

🔍 Potential Issues & Improvements

1. Security: AI Prompt Injection Risk (Medium Priority)

Location: src/main/ipc/handlers/director-notes.ts:264-276

The prompt construction concatenates user-controlled data (session names, file paths) directly into the AI prompt without sanitization. If a session name contains markdown or prompt injection sequences, it could manipulate the AI's behavior.

Recommendation: Sanitize displayName and paths before inserting into the prompt.

2. Error Handling: Missing IPC Registration Check (Low Priority)

Location: src/main/index.ts

Verify that the dependency injection pattern (getProcessManager, getAgentDetector) properly handles null cases and that the handlers are registered after all dependencies are initialized.

3. Performance: Unbounded Graph Data (Low Priority)

Location: src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx:98-99

Consider sampling or limiting the graph data to a reasonable size (e.g., max 1000 entries) to ensure the graph remains responsive even for power users.

4. UX: Unclear Empty State (Low Priority)

Location: src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx:383-387

The empty state message doesn't distinguish between no history, filtered out history, or history outside lookback window. Make the message contextual.

5. Type Safety: Generic Provider Type (Low Priority)

Location: src/main/ipc/handlers/director-notes.ts:83

The provider field is typed as string instead of the union type of valid agent IDs. Use ToolType for better type safety.

6. Documentation: Missing SSH Remote Awareness (Low Priority)

Add a note in the documentation about SSH remote support (or explicitly test it if not covered).

---

📝 Minor Nits

1. Magic number: The "500px from bottom" threshold for infinite scroll should be a named constant
2. Hardcoded opacity: opacity-70 could use the theme's textDim color instead

---

🎯 Summary

Overall Assessment: This is production-ready code with excellent architecture, comprehensive testing, and strong adherence to Maestro's conventions. The feature is well-designed and solves a real UX problem (cross-agent visibility) elegantly.

Blocking Issues: None

Recommended Before Merge:

1. Address the prompt injection security concern (sanitize user input in AI prompts)
2. Verify IPC handler registration error handling

Nice-to-Have:
3. Improve empty state messaging
4. Add SSH remote documentation
5. Consider graph data sampling for large datasets

Great work! The 33 commits show a thoughtful, iterative development process with excellent commit messages. The test coverage is outstanding, and the feature is very well documented.

---

Files Reviewed: 91 changed files, focusing on IPC handlers, Modal components, Settings integration, Test coverage, and Documentation