Skip to content

Added Main Panel Tab Support#7

Merged
pedramamini merged 45 commits intomainfrom
tab-support
Nov 30, 2025
Merged

Added Main Panel Tab Support#7
pedramamini merged 45 commits intomainfrom
tab-support

Conversation

@pedramamini
Copy link
Collaborator

  • MAESTRO: Add AITab interface for multi-tab support
  • MAESTRO: Add ClosedTab interface for tab undo functionality
  • MAESTRO: Update Session interface with aiTabs, activeTabId, closedTabHistory fields
  • MAESTRO: Add migration logic for legacy sessions to aiTabs format
  • MAESTRO: Add getActiveTab helper function for AI tab support
  • MAESTRO: Add createTab helper function for AI tab support
  • MAESTRO: Add closeTab helper function with history tracking
  • MAESTRO: Add reopenClosedTab helper function with duplicate detection
  • MAESTRO: Add setActiveTab helper function for AI tab support
  • MAESTRO: Add getWriteModeTab and getBusyTabs helper functions for AI tab support
  • MAESTRO: Add TabBar component for AI multi-tab support
  • MAESTRO: Import and integrate TabBar component into MainPanel
  • MAESTRO: Update TerminalOutput to use active tab's logs for AI mode
  • MAESTRO: Wire up tab handler props from App.tsx to MainPanel
  • MAESTRO: Update input handling to use active tab's inputValue
  • MAESTRO: Remove session view icon button from main panel header
  • MAESTRO: Remove deprecated recentClaudeSessions from Session interface
  • MAESTRO: Add TAB_SHORTCUTS constants for AI session tab navigation
  • MAESTRO: Add Cmd+T keyboard shortcut for new AI session tab
  • MAESTRO: Add Cmd+W keyboard shortcut to close active AI session tab
  • MAESTRO: Add Cmd+Shift+T keyboard shortcut to reopen closed AI session tab
  • MAESTRO: Add Cmd+] and Cmd+[ keyboard shortcuts for AI session tab navigation
  • MAESTRO: Add Cmd+1-8 keyboard shortcuts for direct AI session tab navigation
  • MAESTRO: Update resume session to create new AI tab instead of replacing current session
  • MAESTRO: Add onOpenSessionAsTab prop to HistoryPanel for tab-based session opening
  • MAESTRO: Add AI tab persistence with log truncation and migration support
  • MAESTRO: Track AI tab write mode state (busy/idle)
  • MAESTRO: Complete tab busy/idle state synchronization for write-mode tracking
  • MAESTRO: Disable input when another AI tab is in write mode
  • MAESTRO: Add visible waiting indicator pill when another tab is in write mode
  • MAESTRO: Allow typing in write-mode locked input while blocking send
  • MAESTRO: Fix write-mode lock release to set all busy tabs to idle
  • MAESTRO: Add busy tab indicator to bottom of message history
  • MAESTRO: Enhance batch processor with per-task usage stats and AI-generated synopses
  • MAESTRO: Add multi-tab synchronization for web/mobile interface
  • bugfixes in separating streaming logic between tabs
  • in deep, must plow forward
  • fix: Improve multi-tab state management and prevent duplicate emissions
  • fix: Route slash commands through active tab and add per-tab usage tracking
  • fix: Use correct tab for toast notifications during parallel execution
  • fix: Unify slash command spawn logic between processInput and handleInputKeyDown
  • focus tweak

Add the AITab interface to types/index.ts as the first step in implementing
tab support for AI agent sessions. Each tab represents a separate Claude Code
conversation with its own session ID, logs, input state, and usage stats.
Add ClosedTab interface to support reopening closed tabs via Cmd+Shift+T.
The interface stores the original tab data, its position in the array,
and the timestamp when closed, enabling proper restoration of tabs.
Migrates existing sessions without aiTabs array to the new multi-tab format.
During migration, looks up starred status from starredClaudeSessions settings
and session names from Claude session origins store.
Creates utility function to retrieve the active AI tab from a session,
with fallback to first tab if activeTabId doesn't match any existing tab.
Adds createTab() function to tabHelpers.ts that creates new AITab
instances and returns the updated session with the new tab added.
Supports optional claudeSessionId, logs, name, starred, and usageStats
parameters. New tabs are automatically set as the active tab.
Implements the closeTab function for AI multi-tab support:
- Removes specified tab from session's aiTabs array
- Stores closed tab in closedTabHistory for undo (max 25 entries)
- Updates activeTabId to next/previous tab if closed tab was active
- Prevents closing the only tab (returns null)
- Returns CloseTabResult with closed tab data and updated session
Implements the reopenClosedTab function for AI tab support:
- Pops most recent tab from closedTabHistory
- Detects duplicates by claudeSessionId (switches to existing tab)
- Restores at original index position when possible
- Returns wasDuplicate flag for UI feedback
Adds setActiveTab(session, tabId) helper function that:
- Changes the active tab in a session
- Returns the target tab and updated session
- Returns null if tab not found
- Handles no-op case when tab is already active
…tab support

Add two new helper functions to tabHelpers.ts for write-mode locking:
- getWriteModeTab(session): Returns the busy tab if any (for single write-mode enforcement)
- getBusyTabs(session): Returns all busy tabs (for busy tab indicator display)

These functions support the write-mode locking feature that prevents multiple
Claude sessions from simultaneously writing to the same project directory.
Create TabBar component with support for:
- Tab display with name > UUID octet > "New" priority
- Hover state for close button (X appears on hover/active)
- Plus button for new tab creation
- Middle-click to close tabs
- Right-click context menu (Rename, Close, Close Others)
- Drag-to-reorder functionality
- Horizontal scroll for overflow tabs
- Active tab accent color highlight
- Star indicator for starred sessions
- Import TabBar component from './TabBar'
- Add tab management props to MainPanelProps interface:
  - onTabSelect, onTabClose, onNewTab for basic tab operations
  - onTabRename, onTabReorder, onCloseOtherTabs for advanced features
- Extract tab handler props in component function
- Insert TabBar between top bar and content area (AI mode only)
- TabBar renders when inputMode is 'ai' and aiTabs array exists

This completes Phase 3 task 1 of the AI multi-tab support feature.
Modified TerminalOutput.tsx to use getActiveTab() helper to get the active
tab's logs when in AI mode, instead of directly accessing session.aiLogs.
This supports the new multi-tab feature while maintaining backwards
compatibility by falling back to session.aiLogs if no tabs exist.
- Import setActiveTab, createTab, closeTab from tabHelpers.ts
- Add onTabSelect handler that uses setActiveTab helper
- Add onTabClose handler that uses closeTab helper
- Add onNewTab handler that uses createTab helper
- Add onTabRename handler for inline tab renaming
- Add onTabReorder handler for drag-to-reorder tabs
- Add onCloseOtherTabs handler for context menu action
Changed AI mode input handling from global state to per-tab storage:
- Removed global aiInputValue useState, now derived from activeTab.inputValue
- Added setAiInputValue callback that updates the active tab's inputValue in session state
- Terminal mode still uses local terminalInputValue state (shared across tabs)

This enables each AI tab to preserve its own pending input text when switching tabs.
Remove the List icon button (Agent Sessions) and its associated Recent
Sessions hover overlay from the top right of the main panel header.
The tab bar now serves this purpose, making this UI redundant.

Removed:
- Agent Sessions button (List icon) and click handler
- Recent Sessions hover overlay with session list
- sessionsTooltipOpen state and sessionsTooltipTimeout ref
- recentClaudeSessions and onResumeRecentSession props from MainPanel
- Unused imports (List, Play, RecentClaudeSession)
Remove the recentClaudeSessions field and RecentClaudeSession interface
that were used for the Recent Sessions overlay. This functionality is
now replaced by the tab bar in AI mode.

Changes:
- Remove RecentClaudeSession interface from types/index.ts
- Remove recentClaudeSessions field from Session interface
- Remove recentClaudeSessions tracking logic from App.tsx
- Remove sessionName parameter from onResumeClaudeSession callback
- Remove initialization of recentClaudeSessions in useSessionManager.ts
- Add TAB_SHORTCUTS constant with all tab navigation shortcuts:
  - Cmd+T: New tab
  - Cmd+W: Close tab
  - Cmd+Shift+T: Reopen closed tab
  - Cmd+[/]: Previous/Next tab (repurposed from right panel cycling)
  - Cmd+1-8: Jump to specific tabs
  - Cmd+9: Jump to last tab
- Remove prevRightTab/nextRightTab shortcuts from DEFAULT_SHORTCUTS
- Remove Cmd+[] right panel tab cycling handlers from App.tsx
  (Right bar panels have dedicated shortcuts: Cmd+Shift+F/H/S)
Implements the newTab keyboard shortcut (Cmd+T / Ctrl+T) that creates
a new AI session tab when in AI mode. Added isTabShortcut helper function
to check against TAB_SHORTCUTS constants, and imported reopenClosedTab
for future use.
…n tab

Implements the reopenClosedTab shortcut (Cmd+Shift+T) for AI mode tabs. The
shortcut uses the existing reopenClosedTab helper function which handles
duplicate detection (switches to existing tab if the closed session is
already open in another tab) and restores tabs at their original position.
…vigation

Add navigateToNextTab() and navigateToPrevTab() helper functions that
cycle through tabs with wrap-around behavior. Wire up keyboard shortcuts
(Cmd+] for next tab, Cmd+[ for previous tab) to these helpers. Shortcuts
only work in AI mode when tabs are available.
…igation

Added two new helper functions to tabHelpers.ts:
- navigateToTabByIndex: Navigate to a specific tab by 0-based index
- navigateToLastTab: Navigate to the last tab (used by Cmd+9)

The shortcuts constants (goToTab1-8, goToLastTab) were already defined
in shortcuts.ts. This commit implements the handlers in App.tsx.
…ing current session

- Modified handleResumeSession in App.tsx to create a new tab when resuming a Claude session
- Added duplicate detection: if session is already open in a tab, switches to that tab instead
- Updated AgentSessionsBrowser and MainPanel interfaces to pass sessionName and starred status
- Session metadata (name, starred status) is now preserved when resuming sessions
- Automatically looks up starred/name from settings if not provided
…ssion opening

- Add onOpenSessionAsTab prop to HistoryPanelProps interface
- Update session ID pill click handler to use onOpenSessionAsTab
- Wire up the new prop through RightPanel to App.tsx
- Reuse handleResumeSession as the handler (already handles tab creation)
…port

- Add migration logic to convert old session format (without aiTabs) to new format on load
- Limit persisted logs to 100 entries per tab to prevent unbounded storage growth
- Exclude closedTabHistory from persistence (runtime-only, resets on app restart)
- Ensure aiTabs and activeTabId are properly persisted and loaded
Update the AITab.state field to reflect write mode status:
- Set tab state to 'busy' when sending messages in AI mode (processInput, remote commands, batch processing)
- Set tab state to 'idle' when Claude process exits
- This enables write-mode locking where only one tab can be in write mode per session
…tracking

Ensure AITab.state is updated to 'idle' in all code paths where session state
transitions to idle. This completes the write-mode locking foundation by ensuring
tab state stays synchronized with session state in error handlers and clear
session operations.

Updated locations:
- startNewClaudeSession helper
- Claude batch spawn error handler
- Non-batch AI mode (process.write) error handler
- Remote command error handler
- processQueuedMessage error handler
- MainPanel clear session handler
- AgentSessionsBrowser clear session handler
Implement write-mode locking in InputArea to prevent file clobbering when
multiple Claude sessions could write to the same project directory:

- Add WriteModeLockedInfo interface to track lock state
- Calculate lock info in MainPanel using getWriteModeTab helper
- Disable textarea input when another tab is busy
- Show "Waiting for [tab name] to finish..." placeholder
- Visual feedback: dimmed input with warning border color
- Send button switches to busy tab when clicked while locked
…ite mode

Added a centered pill indicator in the input area that shows "Waiting for
[session name]..." when another AI tab is busy. The pill:
- Has a pulsing dot indicator for visibility
- Shows the name of the busy tab (or UUID octet if unnamed)
- Is clickable to switch to the busy tab
- Uses warning color styling consistent with the READ-ONLY indicator
- Only shows in AI mode when write-mode is locked by another tab
- Remove disabled={isWriteModeLocked} from textarea to allow typing
- Add write-mode lock check in processInput() to block sending when another tab is busy
- Show flash notification when attempting to send while locked
- Update placeholder to indicate user can type anyway: "Waiting for X to finish... (type anyway)"
- Remove opacity reduction from input container since typing is now allowed
When AI process completes, now correctly releases write-mode lock on ALL
busy tabs rather than just the active tab. This fixes a bug where if the
user switched tabs while AI was running, the original busy tab would
never have its lock released because the completion handler was checking
activeTabId instead of tab.state.
Extends the status indicator at the bottom of TerminalOutput to show
ALL busy tabs in the current Maestro session, not just the current tab.

Key changes:
- Added BusyTabInfo interface for passing tab info to TerminalOutput
- Compute busyTabsInfo in MainPanel using getBusyTabs helper
- Added new AI busy tabs indicator section that shows:
  - Pulsing yellow dot with "Agent is thinking..." status
  - Elapsed time timer
  - Clickable session pills for each busy tab
  - Token counts (In/Out/Total) from active tab
- Session pill format matches main panel header:
  - Named session: show name
  - Starred session: show star + name (e.g., [★ My Feature])
  - Unnamed session: show first UUID octet (e.g., [A1B2C3D4])
- Pills are clickable to switch to that tab
- Indicator shows even when viewing a different (idle) tab
…erated synopses

- Track and accumulate usage stats (tokens, cost) per task in spawnAgentForSession
- Generate AI synopsis after each batch task completion with Summary/Details format
- Display elapsed time and cost in history entry footer
- Parse synopsis into short summary (list/toast) and full synopsis (detail view)

Claude ID: 6ce4daad-843d-4b26-b46e-902e47efccfb
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
Sync AI tab state bidirectionally between desktop and web clients:
- Tab operations (select, new, close) over WebSocket
- Broadcast tab changes (state, name, starred) to web clients
- New TabBar component for mobile web interface
- Session/tab metadata sync including busy state
- Star/rename updates propagate to open tabs

Claude ID: 6ce4daad-843d-4b26-b46e-902e47efccfb
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
- Add resultEmitted flag to prevent duplicate result emissions in process manager
- Prefer 'result' messages over streaming 'assistant' chunks for cleaner output
- Fix regex patterns for parsing session IDs containing multiple hyphens
- Fix stale closure bug when switching tabs quickly during spawn
- Add thinkingStartTime to AITab for accurate per-tab elapsed time tracking
- Enhance ThinkingStatusPill to display write-mode tab's name and session ID
- Add tabName field to toast notifications for better traceability
- Auto-populate tab names from Claude session UUID (first 8 chars uppercase)
- Persist tab names to Claude session metadata for cross-session continuity
- Improve queue processing to correctly target tabs and set states
- Add global.d.ts for comprehensive MaestroAPI type definitions
- Log session size in KB for debugging context window issues

Claude ID: b37141f5-b0b0-4a18-af57-6788f685f604
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
…acking

- Slash commands now properly target the active tab with correct session ID
- Usage stats now accumulate per-tab in addition to session-level tracking
- Header widgets (cost, context) only display for tabs with Claude session
- Removed time tracking widget from header for cleaner UI
- Moved session UUID pill to right side of header with other stats

Claude ID: f8f1f215-750c-4bad-97c0-eed067d98ff3
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
When multiple AI tabs complete tasks independently, toast notifications
were incorrectly using the active tab instead of the specific tab that
completed. This caused wrong session IDs and summaries in notifications.

Now uses tabIdFromSession (parsed from the process session ID) to find
the exact completed tab rather than falling back to getActiveTab().

Also added debug logging for tab/session ID parsing to aid future
debugging of multi-tab synchronization issues.

Claude ID: 3dc545ba-20a3-4610-b2d9-3ba4e0e58c50
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
…nputKeyDown

- Add activeSessionIdRef for consistent session tracking
- Filter isSystemCommand entries from autocomplete (already in slashCommands.ts)
- Reuse activeTab variable instead of redundant commandActiveTab
- Fix handleInputKeyDown to use tab-based session ID pattern matching processInput

Claude ID: 3dc545ba-20a3-4610-b2d9-3ba4e0e58c50
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
@claude
Copy link

claude bot commented Nov 30, 2025

Code Review: Multi-Tab Support Feature

This is a substantial PR that adds multi-tab support to the main panel, enabling users to work with multiple AI agent sessions simultaneously. Overall, the implementation is well-structured with good separation of concerns. Here's my detailed feedback:


🎯 Architecture & Design - Excellent

Strengths:

  • Clean separation of concerns: The tabHelpers.ts utility functions follow functional programming principles with immutable state updates
  • Well-documented code: Comprehensive JSDoc comments throughout, especially in tabHelpers.ts (lines 1-524)
  • Proper migration logic: Thoughtful migration from legacy session format to aiTabs format in App.tsx:264-315
  • Consistent naming: Good use of the standardized vernacular (AI Tab, Session, etc.)
  • Type safety: Strong TypeScript interfaces (AITab, ClosedTab, CreateTabOptions, etc.)

Design Patterns:

  • Helper functions return result objects ({ tab, session, wasDuplicate }) making state updates explicit
  • Portal usage for context menus and overlays prevents z-index stacking issues
  • Event-driven architecture for web/mobile sync via WebSocket broadcasts

🔒 Security - Good

Strengths:

  • Input validation for web server tab operations (sessionId, tabId checks at web-server.ts:1273-1280)
  • No use of eval() or innerHTML
  • Proper error boundaries in place

Recommendations:

  1. Add rate limiting for web tab operations to prevent abuse:

    // In web-server.ts message handler
    case 'new_tab': {
      // Add: Check rate limit per client (e.g., max 10 tabs/minute)
    }
  2. Sanitize session/tab names when displaying in UI to prevent XSS through tab names

  3. Consider adding maximum tab limit per session to prevent memory exhaustion


🐛 Potential Bugs & Edge Cases

Critical:

  1. Race condition in duplicate emission prevention (process-manager.ts:305-315):

    • The sessionIdEmitted and resultEmitted flags are reset when? If a process is reused, these flags could prevent legitimate emissions
    • Recommendation: Reset flags when spawning new process or add timestamp-based deduplication
  2. Tab state synchronization issue:

    • When a tab is closed on desktop, web clients receive broadcast but there's no confirmation
    • Recommendation: Add acknowledgment pattern or optimistic update with rollback
  3. Memory leak potential in TabBar.tsx:83-117:

    • Hover timeout refs are cleared on unmount but isOverOverlayRef could accumulate
    • Recommendation: Clear all refs in cleanup effect

Medium:

  1. Drag-and-drop edge case (TabBar.tsx:500-515):

    • No validation that source/target indices are within bounds before reordering
    • Recommendation: Add bounds checking
  2. Missing null check in reopenClosedTab (tabHelpers.ts:264):

    • session.aiTabs could theoretically be undefined even after the check on line 226
    • Recommendation: Add defensive check or use default empty array
  3. Infinite loop risk in migration logic (App.tsx:266):

    • If session.aiTabs is an empty array (not null/undefined), migration won't run but system expects at least one tab
    • Recommendation: Change condition to !session.aiTabs || session.aiTabs.length === 0

Low:

  1. Console.log statements: Found 300+ console statements across 37 files

    • While useful for debugging, production builds should use proper logger
    • Recommendation: Replace with window.maestro.logger calls or add build-time stripping
  2. TODO comments found at App.tsx:3082 and useSessionManager.ts:347

    • Both say "TODO: Show error to user" but errors are silently swallowed
    • Recommendation: Add toast notifications for these error cases

⚡ Performance Considerations

Concerns:

  1. Log truncation during persistence (App.tsx - not visible in diff but mentioned in PR description):

    • Good that logs are truncated for storage, but ensure the truncation limit is configurable
    • Recommendation: Add setting for max log entries per tab (current appears to be 100)
  2. Full session broadcasts on tab changes:

    • broadcastTabsChange sends entire aiTabs array with usage stats
    • Recommendation: Consider delta updates for large tab arrays (>10 tabs)
  3. O(n) tab lookups:

    • getActiveTab, getWriteModeTab use .find() which is fine for <20 tabs
    • Recommendation: If supporting >50 tabs, consider Map-based lookup
  4. React re-renders in TabBar:

    • Every tab re-renders when any tab state changes
    • Recommendation: Use React.memo on Tab component with custom comparison

Strengths:

  • Good use of useCallback in TabBar.tsx to prevent unnecessary re-renders
  • Portal rendering for overlays prevents re-rendering parent components
  • Lazy evaluation with hover delays (400ms) reduces unnecessary work

🧪 Test Coverage - Needs Improvement

Missing Tests:

  1. Unit tests for tabHelpers.ts:

    • Critical functions like closeTab, reopenClosedTab, createTab have no tests
    • Recommendation: Add Jest tests covering:
      • Closing last tab creates fresh tab
      • Duplicate detection in reopenClosedTab
      • Tab reordering edge cases
      • History limit (MAX_CLOSED_TAB_HISTORY = 25)
  2. Integration tests for multi-tab workflows:

    • Create tab → write mode → close → reopen
    • Tab state persistence across app restarts
    • Web/mobile sync when tabs change
  3. E2E tests:

    • Keyboard shortcuts (Cmd+T, Cmd+W, Cmd+1-9, Cmd+[/])
    • Drag-and-drop reordering
    • Write-mode locking across tabs

📝 Code Quality Notes

Excellent:

  • Consistent code style throughout
  • Proper TypeScript usage with no any types in critical paths
  • Good use of interfaces over types where appropriate
  • Meaningful variable names (closedTabEntry, aggregatedInputTokens, etc.)

Minor Issues:

  1. Magic number: MAX_CLOSED_TAB_HISTORY = 25 (tabHelpers.ts:8)

    • Consider making this configurable via settings
  2. Inconsistent error handling:

    • Some functions return null on error, others return result with error field
    • Recommendation: Standardize on one pattern
  3. Comment clarity (process-manager.ts:302-303):

    • Comment says "skip 'assistant' type" but the code doesn't explicitly skip it
    • Recommendation: Add else if (msg.type === 'assistant') { continue; } for clarity

🎨 UI/UX Observations

Strengths:

  • Browser-style tab design is familiar and intuitive
  • Keyboard shortcuts follow platform conventions
  • Hover overlays provide quick access to actions
  • Visual feedback for busy/starred states

Suggestions:

  1. Tab overflow handling: What happens with 50+ tabs? Consider:

    • Horizontal scrolling indicators
    • Tab condensing (show icons only when crowded)
    • Virtual scrolling for extreme cases
  2. Confirmation for "Close Others":

    • Potentially destructive action with no confirmation
    • Recommendation: Add confirmation modal

✅ Best Practices Adherence

Follows CLAUDE.md conventions:

  • ✅ Uses correct terminology (AI Tab, Main Window, etc.)
  • ✅ Inline styles for theme colors, Tailwind for layout
  • ✅ Proper modal registration with layer stack and priorities
  • ✅ Settings persistence pattern in useSettings.ts
  • ✅ Uses execFileNoThrow for security (no shell injection)

Room for improvement:

  • ⚠️ Many console.log statements should use window.maestro.logger
  • ⚠️ TODO comments should be addressed or converted to issues

📊 Summary

Category Rating Notes
Architecture ⭐⭐⭐⭐⭐ Excellent design with clear separation of concerns
Security ⭐⭐⭐⭐ Good foundations, needs rate limiting and sanitization
Bug Risk ⭐⭐⭐ Several edge cases to address, especially race conditions
Performance ⭐⭐⭐⭐ Well optimized, minor concerns with broadcasts
Test Coverage ⭐⭐ Missing critical unit and integration tests
Code Quality ⭐⭐⭐⭐⭐ Clean, well-documented, type-safe code

Overall: Strong implementation that needs test coverage and edge case hardening before merge.


🚀 Recommendations for Merge

Must Fix:

  1. Add unit tests for tabHelpers.ts core functions
  2. Fix race condition with emission flags in process-manager.ts
  3. Address migration logic edge case (empty array vs undefined)
  4. Implement error toasts for the TODO comments

Should Fix:

  1. Add rate limiting for web tab operations
  2. Add bounds checking in drag-and-drop
  3. Replace console.log with proper logger in new code
  4. Add confirmation for "Close Others" action

Nice to Have:

  1. Make max closed tab history configurable
  2. Optimize broadcasts for large tab arrays
  3. Add tab overflow/condensing UI
  4. Memoize Tab component for better performance

Great work on this feature! The architecture is solid and the code is clean. With the recommended fixes, this will be production-ready. 🎉

- Update session detail callback to fetch logs from active tab (not deprecated aiLogs)
- Add optional tabId query param to /api/session/:id endpoint to avoid race conditions
- Track activeTabId state in mobile app and sync when tabs change on desktop
- Re-fetch logs when activeTabId changes to show correct tab content

Claude ID: 3b710c7e-cf5d-46b4-bba1-dc895fec94e1
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
@claude
Copy link

claude bot commented Nov 30, 2025

Pull Request Review: Multi-Tab Support for AI Sessions

This PR adds 5,531 lines implementing browser-style tabs for AI sessions. Well-structured overall, but critical issues need addressing.

🟢 Strengths

Architecture

  • Clean separation: Tab helpers in tabHelpers.ts are pure, testable functions
  • Well-documented types: AITab, ClosedTab, QueuedItem interfaces are thorough
  • Migration-friendly: Legacy sessions auto-migrate to new tab format
  • Immutable patterns: Helper functions return new objects vs mutation

Code Quality

  • Comprehensive JSDoc with examples on all helpers
  • Strong TypeScript typing, no any types in critical paths
  • Great UX: Duplicate detection, write-mode locking, keyboard shortcuts

🟡 Issues & Concerns

1. Security - Command Injection Risk (src/main/index.ts:783-790)

  • Extracts claudeSessionId from args without validation
  • Recommendation: Add UUID format validation

2. Race Condition (src/main/process-manager.ts:302-314)

  • Emission flags checked then set non-atomically
  • Recommendation: Set flags BEFORE emitting

3. Memory Leak - Tab Logs (src/renderer/App.tsx)

  • Logs accumulate indefinitely, no truncation despite PR description
  • Recommendation: Implement log rotation (max 1000 per tab)

4. Inconsistent State Management (src/renderer/App.tsx:2156-2184)

  • Multiple sequential state updates cause unnecessary re-renders
  • Recommendation: Consolidate to single update

5. Performance (src/renderer/components/TabBar.tsx)

  • No React.memo, re-renders on every parent update
  • Recommendation: Add memoization with custom comparison

6. Missing Error Handling (src/main/web-server.ts:1318-1355)

  • WebSocket handlers don't validate IDs from clients
  • Recommendation: Add format validation for untrusted inputs

7. Tab Name Issues (src/renderer/components/TabBar.tsx:44-53)

  • No length limits on user-provided names
  • Recommendation: Max 50 chars, sanitize special chars

8. Deprecated Field (src/renderer/types/index.ts:190-192)

  • Keeping claudeSessionId increases tech debt
  • Recommendation: Remove after migration or add timeline

🔴 Critical Issues

9. Write-Mode Lock Race Condition ⚠️ BLOCKER

  • Location: src/renderer/App.tsx
  • Problem: Locking relies on async React state. Two tabs can acquire lock simultaneously via rapid keyboard shortcuts
  • Impact: FILE CORRUPTION from concurrent writes
  • Fix: Use useRef for synchronous lock before state updates

10. Missing Test Coverage ⚠️ BLOCKER

  • 5,531 lines added, ZERO tests
  • Critical gaps: Tab helpers (pure functions!), write-mode locking, migration
  • Fix: Add unit tests for tabHelpers.ts, integration tests for locking

📝 Code Style

Follows CLAUDE.md ✅

  • Correct terminology (AI Tab)
  • No shell execution added
  • Settings persistence pattern correct
  • Modal priorities added

Minor issues:

  • Inconsistent Tab/tab capitalization
  • MAX_CLOSED_TAB_HISTORY=25 hardcoded
  • TODO comments in production code

🎯 Recommendations

Before Merging (REQUIRED)

  1. Add tests for tab helpers and write-mode locking
  2. Fix write-mode race condition (Stability, Nuances, Tweaks #9) - data integrity risk
  3. Implement tab log truncation (Raychaser/claude path 01 #3)
  4. Add UUID validation (Internal Logging #1)

Post-Merge (Track as issues)

  1. Optimize TabBar with React.memo
  2. Add usage telemetry
  3. Tab groups/workspaces
  4. Remove deprecated claudeSessionId

📊 Overall Assessment

Code Quality: 7/10
Security: 6/10 (validation gaps)
Performance: 7/10
Test Coverage: 2/10 ⚠️
Documentation: 9/10

Verdict: NEEDS WORK - Write-mode race condition (#9) and missing tests (#10) are blockers. Feature is well-designed, but these issues must be resolved before merge.

Great work on tab helper architecture! 🎉

- Show insertion/deletion counts (+/-) in git log viewer entries
- Fix log deletion to use active tab's claudeSessionId and logs
- ThinkingStatusPill now jumps to the busy tab when clicking session
- Add Cmd+[/] keyboard shortcuts for tab navigation in mobile web
- Hide tab bar in terminal mode on mobile web interface
- Improve tab tooltip styling and minimum tab width for readability

Claude ID: 3b710c7e-cf5d-46b4-bba1-dc895fec94e1
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
@claude
Copy link

claude bot commented Nov 30, 2025

Pull Request Review: Multi-Tab Support

This is a substantial and well-architected feature that adds multi-tab support to Maestro AI sessions.

Strengths

  • Excellent separation of concerns with tabHelpers.ts providing pure, well-documented functions
  • Comprehensive type safety with new AITab and ClosedTab interfaces
  • Smart migration strategy for legacy sessions
  • Exceptional JSDoc documentation with examples
  • Immutable patterns throughout
  • Browser-style UX with drag-and-drop
  • Keyboard-first design with comprehensive shortcuts
  • Write-mode locking prevents file clobbering
  • Desktop to web/mobile tab sync via WebSocket

Issues to Address

1. Race Condition in Session ID Assignment (High Priority)

Location: src/main/process-manager.ts:302-315, App.tsx:1086-1099

Issue: When multiple tabs spawn processes simultaneously, session IDs could be assigned to wrong tabs due to async arrival and fallback to active tab.

Recommendation: Verify all spawn calls consistently encode tab ID in session ID suffix.

2. Memory Management (Medium Priority)

Issue: Each tab stores full conversation logs. 8 tabs x 1000 entries = 4MB+ per session. PR mentions log truncation but implementation needs verification.

Recommendation: Verify truncation is implemented in session save handler.

3. Error Handling (Medium Priority)

Several async operations lack error handlers:

  • window.maestro.web.broadcastTabsChange()
  • window.maestro.claude.updateSessionStarred()

Recommendation: Add try-catch blocks.

Minor Issues

  1. Magic numbers: Extract hardcoded limits to constants
  2. Git log COMMIT_START delimiter could appear in messages
  3. App.tsx now 2355 lines - consider extracting tab logic to custom hook

Testing Recommendations

  1. Multi-tab race conditions with simultaneous messages
  2. Session resume conflicts (same session in 2 tabs)
  3. Web sync under load
  4. Large sessions (1000+ entries per tab)
  5. Migration from pre-PR sessions
  6. Write-mode locking behavior

Performance

Good: O(1) tab switching, efficient web payload
Concern: setSessions() with many tabs/logs could cause re-render lag

Security

  • Uses execFileNoThrow correctly
  • WebSocket operations require security token
  • Good session isolation

Summary

High-quality work with excellent architecture. Tab helpers are exemplary.

Grade: A- (A+ after session ID routing verification)

Recommendation: Approve with minor changes requested

Main concerns are session ID race condition and memory management verification. Other issues are minor.

Excellent work setting Maestro up for future enhancements!

@pedramamini pedramamini merged commit abcf75c into main Nov 30, 2025
1 check passed
@pedramamini pedramamini deleted the tab-support branch November 30, 2025 07:35
@claude
Copy link

claude bot commented Nov 30, 2025

Pull Request Review: Added Main Panel Tab Support

Overview

This is a major feature addition implementing multi-tab support for AI sessions within Maestro. The changes are extensive (+5674/-1286 lines across 39 files) and introduce a significant architectural shift from single-session to multi-tab conversation management.

Strengths

Architecture & Design

Well-structured tab abstraction - The AITab interface and helper functions in tabHelpers.ts provide a clean, functional API for tab management with comprehensive JSDoc documentation.

Comprehensive tab operations - All essential operations are covered: create, close, reopen, navigate, with duplicate detection and history management (max 25 closed tabs).

Write-mode locking - Smart implementation preventing file conflicts when multiple Claude sessions operate on the same project. Only one tab can be in "write mode" (busy state) at a time.

Keyboard-first design - Extensive shortcuts (Cmd+T, Cmd+W, Cmd+Shift+T, Cmd+[/], Cmd+1-9) align with the project's keyboard-first philosophy.

Migration support - Legacy sessions without aiTabs are properly migrated to the new format with sensible defaults.

UI/UX

Browser-like tab interface - Clean Safari/Chrome-inspired design with hover states, drag-to-reorder, and context menus.

Session resumption - Tabs can resume existing Claude sessions from the pool, preventing duplicate sessions via claudeSessionId matching.

Visual feedback - Busy indicators (pulsing dots), star badges, shortcut hints, and elapsed time displays provide excellent user awareness.

Issues & Concerns

🔴 Critical Security Issue

Location: src/main/process-manager.ts:309-315

// Capture session_id from the first message only (prevents duplicate emissions)
// Claude includes session_id in every message, but we only want to emit once
if (msg.session_id && !managedProcess.sessionIdEmitted) {
  managedProcess.sessionIdEmitted = true;
  this.emit('session-id', sessionId, msg.session_id);
}

Problem: The sessionIdEmitted and resultEmitted flags are reset across different processes, but there's no guarantee they won't be re-emitted if the same process is reused. This could lead to duplicate session tracking and state corruption.

Recommendation: Verify that process cleanup properly resets these flags or use a more robust deduplication mechanism (e.g., tracking emitted IDs per session in a Set).


🟡 Race Condition Risk

Location: src/main/index.ts:244-248 and src/main/web-server.ts:747

The web server's getSessionDetailCallback now accepts an optional tabId parameter to fetch logs for a specific tab:

server.setGetSessionDetailCallback((sessionId: string, tabId?: string) => {
  const targetTabId = tabId || session.activeTabId;
  const targetTab = session.aiTabs.find((t: any) => t.id === targetTabId) || session.aiTabs[0];
  aiLogs = targetTab?.logs || [];

Problem: While this helps avoid race conditions, there's still a TOCTOU (time-of-check-time-of-use) issue. The activeTabId could change between when the web client requests logs and when they're retrieved.

Recommendation: Consider returning both the tabId used and a timestamp/version number so clients can detect stale data.


🟡 Memory Concerns

Location: src/renderer/utils/tabHelpers.ts:177

const MAX_CLOSED_TAB_HISTORY = 25;

Problem: Each ClosedTab entry stores the entire AITab including full conversation logs. With 25 closed tabs, this could consume significant memory, especially for long-running sessions with thousands of log entries.

Recommendation:

  • Consider reducing MAX_CLOSED_TAB_HISTORY to 10-15
  • Or truncate logs in closed tabs (keep last N entries)
  • Add memory usage tracking

🟡 Data Persistence Issues

Location: src/renderer/App.tsx (session persistence logic)

The PR description mentions "AI tab persistence with log truncation" but I don't see explicit truncation logic in the persistence code. Long-running sessions with many tabs could lead to very large persisted session files.

Recommendation:

  • Implement aggressive log truncation for persisted tabs (e.g., keep last 100 entries per tab)
  • Document the truncation strategy
  • Consider moving full logs to a separate storage layer

🟠 Error Handling Gaps

Location: src/renderer/utils/tabHelpers.ts:224-280 (reopenClosedTab)

The duplicate detection logic assumes claudeSessionId uniqueness but doesn't handle edge cases:

  • What if two tabs have the same claudeSessionId but different conversation states?
  • What happens if the "duplicate" tab has been closed since the check?

Recommendation: Add defensive checks and potentially emit warnings when duplicates are detected.


🟠 Performance Considerations

Location: src/renderer/components/TabBar.tsx:530-577

The tab rendering iterates through all tabs on every render, and each tab has hover timers, portal overlays, and drag handlers. With many tabs (>20), this could cause jank.

Recommendation:

  • Use React.memo for the Tab component
  • Consider virtualizing tabs if >20 tabs are expected
  • Profile rendering performance with 50+ tabs

🟠 TypeScript Strictness

Location: Multiple files using any types

// src/main/index.ts
const sessions = sessionsStore.get('sessions', []);
const session = sessions.find((s: any) => s.id === sessionId);

Problem: Extensive use of any defeats TypeScript's safety guarantees and could hide type errors.

Recommendation: Define proper interfaces for persisted session data and use them consistently.


🔵 Code Quality Issues

Location: src/renderer/App.tsx (massive 1855 line addition)

The App.tsx file is becoming a god component. With tab management, session management, keyboard shortcuts, and IPC handling all in one place, it's becoming difficult to maintain.

Recommendation:

  • Extract tab management logic to a custom hook (useTabManager)
  • Extract keyboard shortcut handling to a separate hook
  • Consider splitting into smaller components

🔵 Testing & Documentation

Missing:

  • No unit tests for tabHelpers.ts functions
  • No integration tests for tab synchronization
  • No tests for migration logic
  • Limited error scenario testing

Recommendation:

  • Add comprehensive unit tests for all tab operations
  • Add tests for edge cases (closing last tab, reopening with duplicates, etc.)
  • Document the tab state machine and lifecycle

Minor Issues

  1. Inconsistent naming: aiTabs vs AI Tabs vs Agent Tabs - standardize terminology
  2. Magic numbers: 400ms hover delay in TabBar.tsx:93 should be a named constant
  3. Hardcoded limits: max-w-[120px] for tab names in TabBar.tsx:232 should be configurable
  4. Missing null checks: Several places assume session.aiTabs exists without checking

Security Assessment

No injection vulnerabilities detected - command execution uses safe execFileNoThrow
No XSS risks in tab names or session IDs - React handles escaping
⚠️ Session ID tracking needs hardening (see critical issue above)


Performance Impact

🟡 Moderate concern - The multi-tab architecture adds overhead:

  • Each tab maintains separate logs, state, and usage stats
  • Web server now serializes all tabs for each session
  • Tab switching triggers full state updates

Recommendations:

  • Profile memory usage with 10-20 active tabs
  • Monitor IPC message sizes (tab broadcasts could be large)
  • Consider lazy-loading tab logs

Adherence to CLAUDE.md

✅ Uses standardized vernacular (AI Terminal, Main Window)
✅ Follows theme color patterns (inline styles for theme colors)
✅ Uses execFileNoThrow for external commands
✅ Settings persistence pattern followed correctly
✅ Modal integration with layer stack

⚠️ App.tsx becoming too complex (violates "keep components focused" principle)


Recommendations Summary

Before Merge (if not already merged)

  1. ✅ Fix the sessionIdEmitted/resultEmitted tracking in process-manager.ts
  2. ✅ Add TypeScript interfaces for persisted session data (remove any)
  3. ✅ Implement log truncation for persisted tabs
  4. ✅ Add unit tests for tabHelpers.ts

Post-Merge Improvements

  1. Refactor App.tsx to extract tab management logic
  2. Add integration tests for tab synchronization
  3. Profile and optimize tab rendering performance
  4. Document the tab architecture in ARCHITECTURE.md

Overall Assessment

This is high-quality, production-ready code with a well-thought-out architecture. The tab abstraction is clean, the UX is polished, and the feature is comprehensive. However, there are some concerns around memory usage, state synchronization, and code complexity that should be addressed.

Rating: 8/10 - Excellent feature implementation with room for refinement.

The multi-tab support significantly enhances Maestro's usability and aligns perfectly with the project's vision of managing multiple AI coding assistants simultaneously.


Note: Since this PR is already merged, these comments serve as guidance for future improvements and potential bug fixes.

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

Comments