jlengrand/Maestro
1
0
Fork 0
mirror of https://github.com/jlengrand/Maestro.git synced 2026-03-10 08:31:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: Add jump to bottom shortcut and show full commit messages in git log

Browse Source 
- Add Cmd+Shift+J global shortcut to jump to bottom of output
- Display full commit message body in git log detail view
- Add scratch/ to .gitignore for local notes
- Document new shortcut in README.md

Claude ID: 900964ee-78bd-47d3-af6a-f8ef6112e80c
Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682
This commit is contained in:
Pedram Amini
2025-11-30 02:00:31 -06:00
parent 2eba61f5d0
commit b37423abf8
7 changed files with 61 additions and 514 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@@ -10,6 +10,7 @@ dist/
release/
*.log
tmp/
scratch/
# Environment
.env

1
README.md
View File

@@ -137,6 +137,7 @@ Each session shows a color-coded status indicator:
| Show Shortcuts Help | `Cmd+/` | `Ctrl+/` |
| Open Settings | `Cmd+,` | `Ctrl+,` |
| View Agent Sessions | `Cmd+Shift+L` | `Ctrl+Shift+L` |
| Jump to Bottom | `Cmd+Shift+J` | `Ctrl+Shift+J` |
| Cycle Focus Areas | `Tab` | `Tab` |
| Cycle Focus Backwards | `Shift+Tab` | `Shift+Tab` |

1
scratch Symbolic link
View File

@@ -0,0 +1 @@
tmp

514
scratch/ASSESSMENT.md
View File

@@ -1,514 +0,0 @@
# Maestro Codebase Assessment
**Date:** November 30, 2025
**Scope:** Full codebase review covering architecture, security, performance, code quality, and maintainability
---
## Executive Summary
Maestro is a well-architected Electron desktop application for managing multiple AI coding assistants. The codebase demonstrates solid fundamentals including proper security practices (context isolation, safe command execution), thoughtful design patterns (layer stack for modals, dual-process architecture), and strong TypeScript usage. However, there are several areas requiring attention, primarily around code organization, performance optimization, and one critical security issue.
**Key Findings:**
- Several high-priority refactoring opportunities (monolithic App.tsx, prop drilling)
- Performance optimizations needed (missing memoization, re-render cascades)
- 244 console.log/error/warn calls that should use the logger abstraction
- 103 occurrences of `: any` type that could benefit from proper typing
> **Note:** Security issues are deprioritized as this is a personal-use lockdown app. Test generation will be handled in a separate phase.
---
## Table of Contents
1. [Security Issues](#1-security-issues)
2. [Architecture Refactoring](#2-architecture-refactoring)
3. [Performance Issues](#3-performance-issues)
4. [Code Quality](#4-code-quality)
5. [Dead Code & Cleanup](#5-dead-code--cleanup)
6. [Dependency Review](#6-dependency-review)
7. [Implementation Phases](#7-implementation-phases)
---
## 1. Security Issues
### 1.1 CRITICAL: TTS Feature Shell Injection
**Location:** `src/main/index.ts:2436-2438`
**Issue:** The Text-to-Speech (TTS) feature uses `shell: true` when spawning processes with a user-configurable command. This creates a shell injection vulnerability if the `audioFeedbackCommand` setting is manipulated.
```typescript
// VULNERABLE CODE
const child = spawn(fullCommand, [], {
stdio: ['pipe', 'ignore', 'pipe'],
shell: true, // Dangerous with user-controlled command
});
```
**Risk Level:** Critical - allows arbitrary shell command execution
**Remediation:**
- Validate and sanitize `audioFeedbackCommand` before use
- Use an allowlist of known-safe TTS commands (e.g., `say`, `espeak`, `festival`)
- Parse the command and arguments separately, avoiding `shell: true`
- Consider removing shell mode entirely by executing TTS binaries directly
```typescript
// RECOMMENDED APPROACH
const ALLOWED_TTS_COMMANDS = ['say', 'espeak', 'festival', 'piper'];
const commandParts = fullCommand.split(' ');
const baseCommand = commandParts[0];
if (!ALLOWED_TTS_COMMANDS.includes(path.basename(baseCommand))) {
throw new Error('TTS command not in allowlist');
}
const child = spawn(commandParts[0], commandParts.slice(1), {
stdio: ['pipe', 'ignore', 'pipe'],
shell: false, // Safe: no shell interpretation
});
```
### 1.2 Web Server Security Considerations
**Location:** `src/main/web-server.ts`
**Current Mitigations (Good):**
- Security token regenerated on each app restart
- Rate limiting implemented via `@fastify/rate-limit`
- CORS properly configured
- Token validation on all routes
**Remaining Concerns:**
1. **Token Entropy:** UUID v4 provides 122 bits of randomness (sufficient), but ensure secure generation
2. **No HTTPS:** Traffic between mobile device and desktop is unencrypted on local network
3. **Session Timeout:** No automatic token rotation during long sessions
4. **Audit Logging:** Consider logging all web interface commands for security review
**Recommendations:**
- Add optional HTTPS support for security-conscious users
- Implement connection state persistence between app restarts (convenience vs security trade-off)
- Add audit logging for all command executions from web interface
### 1.3 Process Manager Security (Good)
The `ProcessManager` class properly avoids shell injection:
- Uses `shell: false` explicitly in `src/main/process-manager.ts:232`
- PATH augmentation is safe (prepending known paths)
- The `execFileNoThrow` utility provides safe command execution
---
## 2. Architecture Refactoring
### 2.1 HIGH: Monolithic App.tsx
**Location:** `src/renderer/App.tsx` (~5,843 lines)
**Issue:** The main application component contains:
- 40+ `useState` calls
- Multiple complex `useEffect` hooks
- All IPC event listeners
- Business logic for session management, queuing, notifications
**Impact:**
- Difficult to test individual features
- Hard to reason about state changes
- Risk of bugs from state interdependencies
- Challenging onboarding for new developers
**Recommended Extraction:**
| New Hook | Responsibility | Estimated Complexity |
|----------|---------------|---------------------|
| `useIpcListeners` | All process event subscriptions (onData, onExit, onUsage) | Medium |
| `useMessageQueue` | Execution queue management | Medium |
| `useNotifications` | Toast/OS notification logic | Low |
| `useClaudeIntegration` | Claude session ID tracking, history entries | Medium |
| `useLiveMode` | Web interface state management | Low |
| `useKeyboardNavigation` | Session cycling, tab navigation | Low |
### 2.2 HIGH: MainPanel Prop Drilling
**Location:** `src/renderer/components/MainPanel.tsx`
**Issue:** MainPanel receives 100+ props passed from App.tsx, creating:
- Excessive re-renders (any parent state change re-renders entire subtree)
- Brittle component interface
- Difficult testing and refactoring
**Current Pattern:**
```typescript
interface MainPanelProps {
// 50+ state props
// 30+ setter callbacks
// 10+ handler functions
// 5+ refs
}
```
**Recommended Solution:**
1. **Create dedicated contexts** for related state groups:
```typescript
// contexts/SessionContext.tsx
interface SessionContextValue {
activeSession: Session;
sessions: Session[];
setActiveSessionId: (id: string) => void;
updateSession: (id: string, updates: Partial<Session>) => void;
}
// contexts/UIContext.tsx
interface UIContextValue {
theme: Theme;
fontSize: number;
fontFamily: string;
isLeftSidebarOpen: boolean;
toggleLeftSidebar: () => void;
}
```
2. **Split MainPanel into subcomponents:**
- `InputArea` - Text input, image staging, send button
- `OutputArea` - Terminal output, markdown rendering
- `TabBar` - AI tab management
- `StatusBar` - Usage stats, thinking indicator
### 2.3 MEDIUM: Session State Structure
**Issue:** Session interface mixes legacy fields with new aiTabs architecture:
- `aiLogs` (deprecated) vs `aiTabs[].logs` (current)
- `claudeSessionId` at session level vs `aiTabs[].claudeSessionId`
- Migration code in `restoreSession` adds complexity
**Recommendation:**
- Create a v2 Session interface that removes legacy fields
- Write a one-time migration script
- Remove conditional migration logic from runtime code
### 2.4 MEDIUM: Multiple Electron Stores
**Location:** `src/main/index.ts:33-161`
**Current:** 6 separate electron-store instances:
- `store` (settings)
- `sessionsStore`
- `groupsStore`
- `agentConfigsStore`
- `windowStateStore`
- `historyStore`
- `claudeSessionOriginsStore`
**Risk:** Potential race conditions if stores aren't synced atomically
**Recommendation:**
- Consider consolidating into fewer stores with namespaced keys
- Add a transactional wrapper for multi-store updates
- Implement `beforeunload` handler to ensure writes complete
---
## 3. Performance Issues
### 3.1 HIGH: Missing React.memo on Large Components
**Issue:** Only 1 component uses `React.memo` (`HistoryPanel.tsx`), despite many components receiving numerous props.
**Components Needing Memoization:**
| Component | Props Count | Impact |
|-----------|-------------|--------|
| `MainPanel` | 100+ | Every parent update re-renders entire main area |
| `SessionList` | 20+ | Every session state change re-renders list |
| `TerminalOutput` | 15+ | Re-renders on any log update |
| `RightPanel` | 15+ | Re-renders on file tree changes |
**Implementation:**
```typescript
// Before
export function MainPanel(props: MainPanelProps) { ... }
// After
export const MainPanel = React.memo(function MainPanel(props: MainPanelProps) { ... });
```
### 3.2 MEDIUM: Re-render Cascades in Session Updates
**Location:** `src/renderer/App.tsx` - `setSessions()` calls
**Issue:** Every session update creates new arrays/objects, triggering re-renders across the entire session list even when only one session changed.
**Example Problematic Pattern:**
```typescript
setSessions(prev => prev.map(s => {
if (s.id !== targetId) return s; // Returns same reference - good
return { ...s, state: 'busy' }; // Creates new object - triggers re-render
}));
```
**Recommendation:**
- Use `immer` for immutable updates with better reference equality
- Or implement selective update helpers that only recreate changed paths
### 3.3 MEDIUM: No Debouncing on Window Resize
**Issue:** File explorer reloads and panel width adjustments happen on every resize event.
**Recommendation:**
```typescript
// Add debouncing to expensive operations
const debouncedResize = useMemo(
() => debounce((cols: number, rows: number) => {
window.maestro.process.resize(sessionId, cols, rows);
}, 100),
[sessionId]
);
```
### 3.4 LOW: Log Truncation Configuration
**Current:** Fixed at 100 entries per tab
**Issue:** May be insufficient for long debugging sessions; not configurable
**Recommendation:**
- Make `maxOutputLines` configurable in settings
- Consider implementing virtual scrolling for logs (already using `react-virtuoso`)
---
## 4. Code Quality
### 4.1 Console Logging
**Finding:** 244 `console.log/error/warn` calls across the codebase
**Distribution:**
- `src/main/index.ts`: 17 occurrences
- `src/renderer/App.tsx`: 124 occurrences
- `src/main/process-manager.ts`: 49 occurrences
- Various other files: 54 occurrences
**Impact:**
- Inconsistent logging format
- No log levels in development output
- Performance overhead from string interpolation
**Recommendation:**
- Replace `console.*` with the existing `logger` utility
- Add development-only flag for verbose logging
- Use structured logging for easier debugging
### 4.2 Type Safety
**Finding:** 103 occurrences of `: any` across the codebase
**Top Offenders:**
| File | Count |
|------|-------|
| `src/main/index.ts` | 36 |
| `src/main/preload.ts` | 22 |
| `src/renderer/App.tsx` | 10 |
**High-Impact Fixes:**
```typescript
// Before
const sessions = store.get('sessions', []) as any[];
// After
interface PersistedSession { /* ... */ }
const sessions = store.get('sessions', []) as PersistedSession[];
```
**Recommendation:**
- Create proper types for all stored data structures
- Add strict type definitions for IPC message payloads
- Enable `noImplicitAny` in tsconfig (may require significant work)
### 4.3 Error Handling Consistency
**Good Practices Found:**
- Process spawn errors properly caught and emitted
- IPC handlers wrapped in try/catch
- Graceful fallbacks in service layer
**Areas for Improvement:**
- Some async operations missing error boundaries
- Toast notifications don't always surface errors
- Consider adding error boundary around MainPanel
---
## 5. Dead Code & Cleanup
### 5.1 Legacy Session Fields
**Location:** `src/renderer/types/index.ts` - Session interface
The following fields appear to be deprecated in favor of `aiTabs`:
- `aiLogs: LogEntry[]` - Now stored in `aiTabs[].logs`
- `claudeSessionId?: string` - Now in `aiTabs[].claudeSessionId`
- `inputValue?: string` - Now in `aiTabs[].inputValue`
- `usageStats?: UsageStats` - Now in `aiTabs[].usageStats`
**Recommendation:**
- After verifying no remaining references, remove legacy fields
- Update migration code to not preserve these fields
### 5.2 BACKBURNER.md
**Finding:** Presence of `BACKBURNER.md` suggests abandoned/deferred features
**Recommendation:**
- Review and either implement, document, or remove backburner items
- Track deferred features in GitHub Issues instead
### 5.3 Unused Imports
**Recommendation:**
- Run ESLint with `no-unused-vars` and `no-unused-imports` rules
- Add to pre-commit hooks to prevent future accumulation
---
## 6. Dependency Review
### 6.1 Current Dependencies (package.json)
**Production Dependencies:** 19 packages
**Dev Dependencies:** 17 packages
**Security Notes:**
- All major dependencies are actively maintained
- `node-pty` requires native rebuild (handled via `postinstall`)
- `electron` 28.1.0 is recent and patched
**Potential Optimizations:**
| Dependency | Size | Alternative |
|------------|------|-------------|
| `mermaid` | Large | Consider lazy loading |
| `react-syntax-highlighter` | Large | Consider lazy loading |
| `@emoji-mart/react` | Medium | Could be lazy loaded |
### 6.2 Missing Dependencies
Consider adding:
- `immer` - Immutable state updates
- `lodash.debounce` or `use-debounce` - Event throttling
- `@types/diff` - Type definitions for diff package
---
## 7. Implementation Phases
### Phase 1: Performance Quick Wins
**Priority:** P1 - High impact, low effort
**Effort:** 2-3 days
1. **Add React.memo to Large Components**
- `MainPanel`
- `SessionList`
- `TerminalOutput`
- `RightPanel`
2. **Add Debouncing**
- Window resize handlers
- File tree filter input
- Search inputs
3. **Replace Console Logging**
- Create find-and-replace script
- Convert top 50 console.* calls to logger
### Phase 2: Architecture Improvements
**Priority:** P2 - Maintainability
**Effort:** 1-2 weeks
1. **Extract Custom Hooks from App.tsx**
- Start with `useIpcListeners` (lowest risk)
- Then `useMessageQueue`
- Then `useNotifications`
2. **Create UI Context**
- Theme and font settings
- Sidebar state
- Reduces prop drilling for common values
3. **Create Session Context**
- Active session management
- Session CRUD operations
- Reduces MainPanel props by ~30%
### Phase 3: Type Safety & Code Quality
**Priority:** P3 - Technical debt
**Effort:** 1 week
1. **Add Proper Types**
- Define `PersistedSession`, `PersistedGroup` types
- Type all IPC payloads
- Remove high-impact `any` usages
2. **Remove Legacy Code**
- Remove deprecated session fields after migration
- Clean up unused imports
- Address BACKBURNER items
### Phase 4: Advanced Optimizations
**Priority:** P4 - Nice to have
**Effort:** 1 week
1. **Implement Immer**
- Reduce re-render cascades
- Cleaner state update syntax
2. **Lazy Load Heavy Components**
- Mermaid renderer
- Syntax highlighter
- Emoji picker
3. **Consolidate Electron Stores**
- Reduce from 6 stores to 2-3
- Add transactional wrapper
---
## Appendix: File Reference
### Largest Files (Lines of Code)
| File | Lines | Priority |
|------|-------|----------|
| `src/renderer/App.tsx` | 5,843 | Phase 3 |
| `src/main/index.ts` | 2,716 | Phase 1 |
| `src/main/web-server.ts` | 1,691 | - |
| `src/renderer/components/SettingsModal.tsx` | 1,590 | - |
| `src/renderer/components/TerminalOutput.tsx` | 1,526 | Phase 2 |
| `src/renderer/components/Scratchpad.tsx` | 1,394 | - |
| `src/renderer/components/SessionList.tsx` | 1,378 | Phase 2 |
### Key Configuration Files
- `tsconfig.json` - TypeScript configuration
- `vite.config.ts` - Vite build configuration
- `electron-builder.json` - Electron packaging
- `tailwind.config.js` - CSS framework configuration
---
## Summary
The Maestro codebase is fundamentally well-designed with proper security practices for an Electron application. The main areas requiring attention are:
1. **Security:** Fix the TTS shell injection vulnerability immediately
2. **Performance:** Add memoization to prevent unnecessary re-renders
3. **Architecture:** Extract logic from App.tsx to improve maintainability
4. **Code Quality:** Replace console logging, improve type safety
The recommended 4 implementation phases are prioritized by impact and effort, focusing on performance and maintainability improvements.

5
src/renderer/App.tsx
View File

@@ -2713,6 +2713,11 @@ export default function MaestroConsole() {
e.preventDefault();
setProcessMonitorOpen(true);
}
else if (isShortcut(e, 'jumpToBottom')) {
e.preventDefault();
// Jump to the bottom of the current main panel output (AI logs or terminal output)
logsEndRef.current?.scrollIntoView({ behavior: 'smooth' });
}
// Tab shortcuts (AI mode only)
if (activeSession?.inputMode === 'ai' && activeSession?.aiTabs) {

52
src/renderer/components/GitLogViewer.tsx
View File

@@ -201,6 +201,49 @@ export function GitLogViewer({ cwd, theme, onClose }: GitLogViewerProps) {
return parseGitDiff(diffText);
}, [selectedCommitDiff]);
// Extract the full commit message (body) from the show output
const commitBody = useMemo(() => {
if (!selectedCommitDiff) return null;
const lines = selectedCommitDiff.split('\n');
const bodyLines: string[] = [];
let foundDate = false;
let foundBody = false;
for (const line of lines) {
// Skip until we find the Date: line
if (line.startsWith('Date:')) {
foundDate = true;
continue;
}
// After Date:, skip empty lines until we find content
if (foundDate && !foundBody) {
if (line.trim() === '') continue;
foundBody = true;
}
// Stop when we hit the stats separator (---)
if (foundBody && line.startsWith('---')) {
break;
}
// Collect body lines (they're usually indented with 4 spaces)
if (foundBody) {
// Remove the leading indentation (usually 4 spaces)
const trimmedLine = line.startsWith(' ') ? line.slice(4) : line;
bodyLines.push(trimmedLine);
}
}
// Return null if we only have the subject line (already shown in header)
// Body is meaningful if it has more than just one line
const body = bodyLines.join('\n').trim();
// Check if body has actual content beyond the subject
const hasMultipleLines = bodyLines.filter(l => l.trim()).length > 1;
return hasMultipleLines ? body : null;
}, [selectedCommitDiff]);
// Extract commit stats from the show output
const commitStats = useMemo(() => {
if (!selectedCommitDiff) return null;
@@ -391,6 +434,15 @@ export function GitLogViewer({ cwd, theme, onClose }: GitLogViewerProps) {
</div>
</div>
{/* Full commit message body */}
{commitBody && (
<div className="mb-6 p-3 rounded" style={{ backgroundColor: theme.colors.bgActivity }}>
<div className="text-xs font-mono space-y-1 whitespace-pre-wrap" style={{ color: theme.colors.textDim }}>
{commitBody}
</div>
</div>
)}
{/* File stats */}
{commitStats && (
<div className="mb-6 p-3 rounded" style={{ backgroundColor: theme.colors.bgActivity }}>

1
src/renderer/constants/shortcuts.ts
View File

@@ -24,6 +24,7 @@ export const DEFAULT_SHORTCUTS: Record<string, Shortcut> = {
agentSessions: { id: 'agentSessions', label: 'View Agent Sessions', keys: ['Meta', 'Shift', 'l'] },
systemLogs: { id: 'systemLogs', label: 'System Log Viewer', keys: ['Alt', 'Meta', 'l'] },
processMonitor: { id: 'processMonitor', label: 'System Process Monitor', keys: ['Alt', 'Meta', 'p'] },
jumpToBottom: { id: 'jumpToBottom', label: 'Jump to Bottom', keys: ['Meta', 'Shift', 'j'] },
};
// Tab navigation shortcuts (AI mode only)