maestro-cue-spinout: 48 tasks across 2026-02-14-Maestro-Cue/MAESTRO-CUE-01, 2026-02-14-Maestro-Cue/MAESTRO-CUE-02 +12 more#488
Open
pedramamini wants to merge 17 commits intomainfrom
Open
maestro-cue-spinout: 48 tasks across 2026-02-14-Maestro-Cue/MAESTRO-CUE-01, 2026-02-14-Maestro-Cue/MAESTRO-CUE-02 +12 more#488pedramamini wants to merge 17 commits intomainfrom
pedramamini wants to merge 17 commits intomainfrom
Conversation
…r, and Encore feature flag - Register maestroCue as an Encore Feature flag (EncoreFeatureFlags, DEFAULT_ENCORE_FEATURES) - Create src/main/cue/cue-types.ts with CueEventType, CueSubscription, CueSettings, CueConfig, CueEvent, CueRunStatus, CueRunResult, CueSessionStatus, and related constants - Add 'CUE' to HistoryEntryType across shared types, global.d.ts, preload, IPC handlers, and hooks - Add cueTriggerName, cueEventType, cueSourceSession optional fields to HistoryEntry - Add 'cue' log level to MainLogLevel, LOG_LEVEL_PRIORITY, logger switch/case, and LogViewer with teal color (#06b6d4), always-enabled filter, and agent name pill - Add 10 Cue-specific template variables (CUE_EVENT_TYPE, CUE_TRIGGER_NAME, etc.) with cueOnly flag - Extend TemplateContext with cue? field and substituteTemplateVariables with Cue replacements - Update TEMPLATE_VARIABLES_GENERAL filter to exclude cueOnly variables
…ovider Implements the three core modules for the Cue event-driven automation engine: - cue-yaml-loader.ts: Discovers and parses maestro-cue.yaml files with js-yaml, validates config structure, watches for file changes via chokidar with 1-second debounce - cue-file-watcher.ts: Wraps chokidar for file.changed subscriptions with per-file debouncing (5s default), constructs CueEvent instances with full file metadata payloads - cue-engine.ts: Main coordinator class with dependency injection, manages time.interval timers (fires immediately then on interval), file watchers, agent.completed listeners with fan-in tracking, activity log ring buffer (max 500), and run lifecycle management Added js-yaml and @types/js-yaml dependencies. 57 tests across 3 test files.
…story recording Implements the Cue executor module that spawns background agent processes when Cue triggers fire, following the same spawn pattern as Auto Run's process:spawn IPC handler. Key exports: - executeCuePrompt(): Full 10-step pipeline (prompt resolution, template substitution, agent arg building, SSH wrapping, process spawn with stdout/stderr capture, timeout enforcement with SIGTERM→SIGKILL) - stopCueRun(): Graceful process termination by runId - recordCueHistoryEntry(): Constructs HistoryEntry with type 'CUE' and all Cue-specific fields (trigger name, event type, source session) - getActiveProcesses(): Monitor running Cue processes Test coverage: 31 tests in cue-executor.test.ts covering execution paths, SSH remote, timeout escalation, history entry construction, and edge cases. Full suite: 21,635 tests passing across 512 files, zero regressions.
…zation for Maestro Cue
Add CUE entry support across all History components: - HistoryFilterToggle: CUE filter button with teal (#06b6d4) color and Zap icon - HistoryEntryItem: CUE pill, success/failure badges, and trigger metadata subtitle - HistoryPanel & UnifiedHistoryTab: CUE included in default activeFilters - HistoryDetailModal: CUE pill color, icon, success/failure indicator, trigger metadata display - Comprehensive test coverage for all CUE rendering paths (205 new/updated tests pass)
…nd activity log Add the Maestro Cue dashboard modal with full Encore Feature gating: - CueModal component with sessions table, active runs list, and activity log - useCue hook for state management, event subscriptions, and 10s polling - Settings toggle in Encore tab, command palette entry, keyboard shortcut (Cmd+Shift+U) - SessionList hamburger menu entry, modal store integration, lazy loading - 30 tests covering hook behavior and modal rendering
Add CueYamlEditor component for creating and editing maestro-cue.yaml files. Features split-view layout with AI assist (left panel for description + clipboard copy) and YAML editor (right panel with line numbers, debounced validation, Tab indentation). Integrates into CueModal via Edit YAML button on each session row.
…yaml Task 1: CueHelpModal component with 7 content sections (What is Maestro Cue, Getting Started, Event Types, Template Variables, Multi-Agent Orchestration, Timeouts & Failure Handling, AI YAML Editor). Wired to CueModal ? button. Registered with layer stack at MODAL_PRIORITIES.CUE_HELP (465). Task 2: useCueAutoDiscovery hook that calls cue:refreshSession when sessions are created/restored/removed, gated by encoreFeatures.maestroCue. Full scan on feature enable, engine disable on feature off. Tests: 38 CueHelpModal tests + 10 useCueAutoDiscovery tests, all passing. Lint clean. No existing test regressions (21,778 tests pass).
… session bridging Implement agent completion event chaining in the Cue engine: - Fan-out: subscriptions dispatch prompts to multiple target sessions simultaneously - Fan-in: subscriptions wait for all source sessions to complete before firing, with timeout handling (break clears tracker, continue fires with partial data) - Session bridging: user session completions trigger Cue subscriptions via exit listener - Add AgentCompletionData type for rich completion event payloads - Add hasCompletionSubscribers() optimization to skip unneeded notifications - Wire getCueEngine/isCueEnabled into ProcessListenerDependencies
…ure gated) Add teal Zap icon next to session names in the Left Bar for sessions with active Maestro Cue subscriptions. The indicator is gated behind the maestroCue Encore Feature flag and shows a tooltip with the subscription count on hover. - Add cueSubscriptionCount prop to SessionItem with Zap icon rendering - Add lightweight Cue status fetching in SessionListInner via cue:getStatus IPC, refreshed on cue:activityUpdate events - Add cue namespace to global test setup mock - 6 unit tests + 3 integration tests; all 21,815 tests pass; lint clean
Add Maestro Cue entries across all developer documentation: - CLAUDE.md: Key Files table (4 entries), Architecture tree (cue/ dir), Standardized Vernacular (Cue + Cue Modal terms) - CLAUDE-PATTERNS.md: Encore Feature section lists maestroCue as second reference implementation alongside directorNotes - CLAUDE-IPC.md: cue namespace in Automation section, full Cue API reference with all endpoints and event documentation
…t journal - Add cue-db.ts: SQLite-backed event journal (cue_events table) and single-row heartbeat table (cue_heartbeat) using better-sqlite3 with WAL mode - Add cue-reconciler.ts: time event catch-up logic that fires exactly one reconciliation event per missed subscription (no flooding), with payload.reconciled and payload.missedCount metadata - Update cue-engine.ts: heartbeat writer (30s interval), sleep detection (2-minute gap threshold), database pruning (7 days), and clean shutdown - Update CueHelpModal: new "Sleep & Recovery" section with Moon icon - Update CueModal: amber "catch-up" badge on reconciled activity log entries - Tests: 41 new tests across cue-db (17), cue-reconciler (11), cue-sleep-wake (13)
Add filter field to CueSubscription for narrowing when subscriptions fire. Supports exact match, negation (!), numeric comparison (>/</>=/<=), glob patterns (picomatch), and boolean matching with AND logic. Filter checks integrated at all three dispatch points (file.changed, time.interval, agent.completed). Includes help modal docs, AI prompt updates, and 80 new tests (43 filter engine + 37 YAML loader).
… awareness Add pattern presets (Scheduled Task, File Enrichment, Reactive, Research Swarm, Sequential Chain, Debate) to the YAML editor as clickable cards. Enhance the AI system prompt with pattern recognition guidance. Add a Coordination Patterns section with ASCII flow diagrams to the help modal.
📝 WalkthroughWalkthroughThis PR introduces Maestro Cue, a comprehensive event-driven automation system for Maestro. It adds a CueEngine coordinator managing per-session subscriptions (time intervals, file changes, agent completions), fan-out/fan-in orchestration, concurrency controls, YAML configuration, database persistence, sleep detection, and complete renderer integration via IPC and preload APIs. The feature is gated behind a maestroCue Encore flag and includes extensive test coverage and UI components. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Electron App<br/>(main)
participant Engine as CueEngine
participant DB as CueDB
participant Exec as CueExecutor
participant Proc as Agent Process
participant Renderer as Renderer<br/>(IPC/Preload)
App->>Engine: start() on boot
Engine->>DB: initCueDb()
Engine->>DB: pruneCueEvents()
Engine->>DB: updateHeartbeat()
loop Periodic (every 30s)
Engine->>DB: updateHeartbeat()
end
loop Timer-based subscriptions
Engine->>Exec: executeCueRun(subscription, event)
Exec->>Proc: spawn agent process
Proc-->>Exec: stdout/stderr
Proc-->>Exec: exit code
Exec->>DB: recordCueEvent(status)
Exec-->>Engine: CueRunResult
end
loop File watcher
Engine->>Engine: onFileChanged(path)
Engine->>Exec: executeCueRun(subscription, event)
end
loop Agent completion
Proc-->>Engine: notifyAgentCompleted(sessionId)
Engine->>Engine: handleFanIn/fanOut
Engine->>Exec: executeCueRun(target_session)
end
Renderer->>Engine: getStatus() via IPC
Engine-->>Renderer: CueSessionStatus[]
Renderer->>Engine: onActivityUpdate subscribe via IPC
Engine->>Renderer: push cue:activityUpdate events
sequenceDiagram
participant User as User
participant UI as CueModal<br/>UI
participant Editor as CueYamlEditor
participant IPC as IPC<br/>(main)
participant Engine as CueEngine
participant FS as File System
User->>UI: Opens Cue Modal
UI->>IPC: getStatus()
IPC->>Engine: getStatus()
Engine-->>IPC: CueSessionStatus[]
IPC-->>UI: sessions loaded
UI->>UI: render sessions table
User->>UI: Clicks "Edit YAML" for session
UI->>Editor: open with projectRoot
Editor->>IPC: readYaml(projectRoot)
IPC->>FS: read maestro-cue.yaml
FS-->>IPC: content
IPC-->>Editor: content or template
User->>Editor: edits YAML
Editor->>Editor: debounced validateYaml()
Editor->>IPC: validateYaml(content)
IPC-->>Editor: {valid, errors}
Editor->>UI: display validation
User->>Editor: clicks Save
Editor->>IPC: writeYaml(projectRoot, content)
IPC->>FS: write maestro-cue.yaml
IPC->>Engine: refreshSession(sessionId, projectRoot)
Engine->>Engine: reload config, restart watchers
Engine-->>IPC: success
IPC-->>Editor: saved
Editor->>UI: close, refresh status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes The review requires attention to: CueEngine's intricate state management (fan-out/fan-in, queuing, concurrency), database schema and queries, IPC handler delegation patterns, sleep detection and reconciliation logic, async lifecycle management across multiple components, and integration points with process listeners and history. Test coverage is extensive and validates most paths, but logic density in the engine warrants careful verification. The changes are largely additive (many new files) rather than scattered, which reduces review complexity, but scope and interconnectedness across main/renderer/IPC layers demands thorough integration validation. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/components/History/HistoryEntryItem.tsx (1)
98-107:⚠️ Potential issue | 🟡 MinorAdd
tabIndexfor keyboard accessibility.The clickable entry card div lacks a
tabIndexattribute, which would prevent keyboard-only users from focusing and activating it. As per coding guidelines, components that need keyboard focus should havetabIndexand focus event handlers.🔧 Proposed fix
<div onClick={() => onOpenDetailModal(entry, index)} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onOpenDetailModal(entry, index); + } + }} className="p-3 rounded border transition-colors cursor-pointer hover:bg-white/5"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/HistoryEntryItem.tsx` around lines 98 - 107, The HistoryEntryItem clickable div is not keyboard-accessible; add tabIndex={0} to the div and implement an onKeyDown handler that calls onOpenDetailModal(entry, index) when Enter or Space is pressed; also wire onFocus and onBlur handlers (or existing selection handlers) to mirror mouse hover/selection behavior so keyboard users receive the same focus styling as isSelected — update the div in HistoryEntryItem that currently uses onClick, className and style to include tabIndex, onKeyDown, onFocus and onBlur and keep using onOpenDetailModal(entry, index) as the activation action.src/main/ipc/handlers/director-notes.ts (1)
78-85:⚠️ Potential issue | 🟠 MajorCUE filter is correctly added, but stats tracking is incomplete.
The
'CUE'filter option works as expected. However,UnifiedHistoryStatsonly tracksautoCountanduserCount, withtotalCountbeing their sum. Since CUE is a legitimate, first-class entry type (recorded byrecordCueHistoryEntry()in cue-executor.ts and displayed in the UI), the stats should track all three types consistently.The comment at line 163 states "Track stats from all entries (before type filter)" but the code only increments
autoCountanduserCount(lines 166-167), missing CUE entries entirely. Add acueCountfield toUnifiedHistoryStatsand update the counting logic to include CUE entries so the stats accurately reflect all history data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/director-notes.ts` around lines 78 - 85, The stats object UnifiedHistoryStats is missing CUE tracking: add a numeric cueCount field to UnifiedHistoryStats and update any code that aggregates entry types (the block that currently increments autoCount and userCount) to also increment cueCount when entry.type === 'CUE'; then compute totalCount as autoCount + userCount + cueCount. Ensure this aligns with the CUE entries produced by recordCueHistoryEntry() (cue-executor.ts) and update any places that construct or return UnifiedHistoryStats so the new cueCount is included.
🟠 Major comments (15)
src/renderer/hooks/useCue.ts-35-40 (1)
35-40:⚠️ Potential issue | 🟠 MajorAlign
CueSessionStatus.projectRootwith the API contract (optional).Line 39 makes
projectRootrequired, but Cue status payloads can omit it. This creates type drift and unsafe assumptions in consumers.🛠️ Suggested fix
export interface CueSessionStatus { sessionId: string; sessionName: string; toolType: string; - projectRoot: string; + projectRoot?: string; enabled: boolean; subscriptionCount: number; activeRuns: number;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useCue.ts` around lines 35 - 40, The CueSessionStatus type currently marks projectRoot as required which contradicts the API payload that can omit it; update the CueSessionStatus interface (symbol: CueSessionStatus) to make projectRoot optional (e.g., add the optional marker or allow undefined) so consumers no longer assume it is always present and handle its absence safely.src/__tests__/main/cue/cue-yaml-loader.test.ts-138-142 (1)
138-142:⚠️ Potential issue | 🟠 MajorDon’t lock in exception-throwing behavior for malformed YAML.
Line 141 currently enforces throws for user-edited config errors. That makes YAML hot-reload brittle. Prefer graceful handling (e.g., return
null/validation errors) and adjust this test accordingly.🛠️ Suggested test expectation change
- it('throws on malformed YAML', () => { + it('returns null for malformed YAML', () => { mockExistsSync.mockReturnValue(true); mockReadFileSync.mockReturnValue('{ invalid yaml ['); - expect(() => loadCueConfig('/projects/test')).toThrow(); + expect(loadCueConfig('/projects/test')).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-yaml-loader.test.ts` around lines 138 - 142, The test currently asserts that loadCueConfig('/projects/test') throws on malformed YAML, which forces throwing behavior; update the test to expect graceful handling instead (e.g., return null or a validation error) and modify the assertion accordingly. Locate the test case around the function loadCueConfig in cue-yaml-loader.test.ts (the "throws on malformed YAML" test) and change the expectation from expect(() => loadCueConfig(...)).toThrow() to an assertion that loadCueConfig returns null or a specific error/validation object (matching whatever loadCueConfig now returns for parse errors), and ensure mockExistsSync and mockReadFileSync remain set to simulate the malformed YAML input.src/main/process-listeners/exit-listener.ts-450-457 (1)
450-457:⚠️ Potential issue | 🟠 MajorGuard Cue completion notification against engine failures.
A throw inside
getCueEngine(),hasCompletionSubscribers(), ornotifyAgentCompleted()can escape the exit handler path.🛡️ Suggested hardening
- if (isCueEnabled?.() && getCueEngine) { - const cueEngine = getCueEngine(); - if (cueEngine?.hasCompletionSubscribers(sessionId)) { - cueEngine.notifyAgentCompleted(sessionId, { - status: code === 0 ? 'completed' : 'failed', - exitCode: code, - }); - } - } + try { + if (isCueEnabled?.() && getCueEngine) { + const cueEngine = getCueEngine(); + if (cueEngine?.hasCompletionSubscribers(sessionId)) { + cueEngine.notifyAgentCompleted(sessionId, { + status: code === 0 ? 'completed' : 'failed', + exitCode: code, + }); + } + } + } catch (err) { + logger.warn('[Cue] Failed to notify completion subscribers', 'ProcessListener', { + sessionId, + error: String(err), + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-listeners/exit-listener.ts` around lines 450 - 457, Wrap the entire Cue notification sequence in a protective try/catch so exceptions from getCueEngine(), hasCompletionSubscribers(), or notifyAgentCompleted() cannot escape the exit handler; first call isCueEnabled?.() and then attempt to getCueEngine(), but guard calls with null/undefined checks and handle any thrown error by catching it and logging (or silently ignoring) without rethrowing; specifically update the block using isCueEnabled, getCueEngine, cueEngine.hasCompletionSubscribers(sessionId), and cueEngine.notifyAgentCompleted(sessionId, ...) so all three calls are executed inside a single try/catch and never allow an exception to propagate out of the exit listener.src/main/process-listeners/exit-listener.ts-448-457 (1)
448-457:⚠️ Potential issue | 🟠 MajorUse
baseSessionIdwhen notifying cue engine of agent completion.The exit listener extracts
baseSessionIdfor web broadcasting (line 439), but passes the raw suffixedsessionIdto cue notifications (lines 452-453). SincehasCompletionSubscribers()andnotifyAgentCompleted()match subscriptions against rawsessionIdor session name, completion subscribers configured with a canonical (base) session ID will be missed for sessions with suffixes like-ai-terminal,-batch-*, or-synopsis-*.Pass
baseSessionIdto bothhasCompletionSubscribers()andnotifyAgentCompleted()to align with the web broadcasting normalization and ensure subscribers receive notifications regardless of session suffix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-listeners/exit-listener.ts` around lines 448 - 457, The code calls hasCompletionSubscribers(sessionId) and notifyAgentCompleted(sessionId, ...) with the raw suffixed sessionId; change both calls to use the normalized baseSessionId extracted earlier so Cue subscriptions registered against canonical session IDs are matched. In the exit listener where isCueEnabled, getCueEngine, hasCompletionSubscribers and notifyAgentCompleted are used, pass baseSessionId instead of sessionId to both hasCompletionSubscribers(...) and notifyAgentCompleted(...).src/renderer/components/CueHelpModal.tsx-162-162 (1)
162-162:⚠️ Potential issue | 🟠 MajorFix JSX comment-text lint blockers in example strings.
Line 162, Line 211, and Line 310 include
/*-like content in JSX text nodes, which triggers BiomenoCommentText.Suggested fix
- src/**/*.ts + {'src/**/*.ts'} @@ - {' '}watch: "src/**/*.ts" + {' watch: "src/**/*.ts"'} @@ - {' '}watch: "src/**/*" + {' watch: "src/**/*"'}Also applies to: 211-211, 310-310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueHelpModal.tsx` at line 162, In CueHelpModal (the CueHelpModal component), replace raw "/*...*/" sequences inside JSX text nodes (occurrences noted around the example strings at the current lines ~162, ~211, ~310) so they don't look like comments to the Biome linter; wrap those example snippets as JS expressions or code nodes (for example: <code>{'/* example comment */'}</code> or <code>{`/* example comment */`}</code>) or split the characters (e.g. {'/*'} and {'*/'}) so the literal "/*" and "*/" never appear directly in JSX text; update the example strings in those three places accordingly.src/main/cue/cue-filter.ts-57-63 (1)
57-63:⚠️ Potential issue | 🟠 MajorNegated glob filters are currently evaluated as exact-string negation.
At Line 57–Line 63,
!*.test.tswon’t behave as a negated glob; it only checks exact equality with"*.test.ts".Suggested fix
} else if (filterValue.startsWith('!')) { const remainder = filterValue.slice(1); - if (String(payloadValue) === remainder) return false; + if (remainder.includes('*')) { + const isMatch = picomatch(remainder); + if (isMatch(String(payloadValue))) return false; + } else { + if (String(payloadValue) === remainder) return false; + } } else if (filterValue.includes('*')) { const isMatch = picomatch(filterValue); if (!isMatch(String(payloadValue))) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-filter.ts` around lines 57 - 63, The negation branch currently treats any value starting with '!' as an exact-string negation, so patterns like '!*.test.ts' are not recognized as negated globs; update the logic in cue-filter handling (look for filterValue, payloadValue, and picomatch usage) so that when filterValue startsWith('!') you strip the '!' into remainder and then if remainder.includes('*') use picomatch(remainder) and return false when the matcher matches payloadValue (i.e., negate the glob match); otherwise keep the existing exact-string comparison for non-glob remainders.src/renderer/components/CueModal.tsx-285-293 (1)
285-293:⚠️ Potential issue | 🟠 MajorDon’t infer global Cue engine state from session rows.
Line 285 uses
sessions.some((s) => s.enabled)for the master toggle. This fails when the engine is enabled but there are zero configured sessions (status list empty), leaving the toggle stuck in a misleading disabled state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueModal.tsx` around lines 285 - 293, The master toggle currently computes isEnabled from sessions.some((s) => s.enabled), which breaks when there are zero sessions; instead read the global Cue engine state directly (e.g., a prop or selector like cueEngineEnabled / getCueEngineStatus()) and use that for isEnabled and handleToggle logic; update the component to replace the sessions-derived isEnabled with the global engine state (keep handleToggle using enable/disable but depend on the new cueEngineEnabled symbol rather than sessions) so the toggle reflects the real engine status even when sessions array is empty.src/main/cue/cue-yaml-loader.ts-163-192 (1)
163-192:⚠️ Potential issue | 🟠 MajorReject unknown
eventvalues and malformedsource_sessionarrays in validation.Line 163 only enforces
eventas a string, so typos are treated as valid config. Also,source_sessionarrays are not validated for string elements, allowing structurally invalidagent.completedconfigs to pass.✅ Suggested validator tightening
+const VALID_EVENTS = new Set(['time.interval', 'file.changed', 'agent.completed']); + if (!sub.event || typeof sub.event !== 'string') { errors.push(`${prefix}: "event" is required and must be a string`); +} else if (!VALID_EVENTS.has(sub.event)) { + errors.push( + `${prefix}: "event" must be one of "time.interval", "file.changed", or "agent.completed"` + ); } @@ } else if (event === 'agent.completed') { if (!sub.source_session) { errors.push(`${prefix}: "source_session" is required for agent.completed events`); } else if (typeof sub.source_session !== 'string' && !Array.isArray(sub.source_session)) { errors.push( `${prefix}: "source_session" must be a string or array of strings for agent.completed events` ); + } else if ( + Array.isArray(sub.source_session) && + sub.source_session.some((v) => typeof v !== 'string' || v.trim() === '') + ) { + errors.push(`${prefix}: "source_session" array must contain only non-empty strings`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-yaml-loader.ts` around lines 163 - 192, Tighten validation by rejecting unknown event values and ensuring arrays contain only strings: after extracting event from sub, validate it against the allowed set ['time.interval','file.changed','agent.completed'] and push an error if it’s not one of those; for the agent.completed branch, require sub.source_session to be either a non-empty string or an array where every element is a non-empty string (iterate the array and push an error if any element is not a string or is empty). Use the existing symbols sub, event, source_session and prefix to locate and update the checks and error messages accordingly.src/main/cue/cue-yaml-loader.ts-76-94 (1)
76-94:⚠️ Potential issue | 🟠 MajorHarden settings parsing to reject invalid numeric values during load.
Line 79, Line 87, and Line 91 accept any JS number (
NaN,Infinity, negative values, non-integers). That allows invalid runtime settings to bypass constraints and destabilize timeout/concurrency/queue behavior.🛡️ Suggested bounds-safe parsing
+const asPositiveFiniteNumber = (value: unknown, fallback: number): number => + typeof value === 'number' && Number.isFinite(value) && value > 0 ? value : fallback; + +const asBoundedInt = (value: unknown, fallback: number, min: number, max: number): number => + typeof value === 'number' && + Number.isInteger(value) && + value >= min && + value <= max + ? value + : fallback; + const rawSettings = parsed.settings as Record<string, unknown> | undefined; const settings: CueSettings = { - timeout_minutes: - typeof rawSettings?.timeout_minutes === 'number' - ? rawSettings.timeout_minutes - : DEFAULT_CUE_SETTINGS.timeout_minutes, + timeout_minutes: asPositiveFiniteNumber( + rawSettings?.timeout_minutes, + DEFAULT_CUE_SETTINGS.timeout_minutes + ), timeout_on_fail: rawSettings?.timeout_on_fail === 'break' || rawSettings?.timeout_on_fail === 'continue' ? rawSettings.timeout_on_fail : DEFAULT_CUE_SETTINGS.timeout_on_fail, - max_concurrent: - typeof rawSettings?.max_concurrent === 'number' - ? rawSettings.max_concurrent - : DEFAULT_CUE_SETTINGS.max_concurrent, - queue_size: - typeof rawSettings?.queue_size === 'number' - ? rawSettings.queue_size - : DEFAULT_CUE_SETTINGS.queue_size, + max_concurrent: asBoundedInt(rawSettings?.max_concurrent, DEFAULT_CUE_SETTINGS.max_concurrent, 1, 10), + queue_size: asBoundedInt(rawSettings?.queue_size, DEFAULT_CUE_SETTINGS.queue_size, 0, 50), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-yaml-loader.ts` around lines 76 - 94, The settings parsing currently accepts any JS number (NaN/Infinity/negatives/floats); update the validation for rawSettings fields (used to build CueSettings from parsed) to explicitly reject non-finite, non-integer, or out-of-range values: for timeout_minutes require Number.isFinite(value) && Number.isInteger(value) && value >= 0, and for max_concurrent and queue_size require Number.isFinite(value) && Number.isInteger(value) && value >= 1, falling back to DEFAULT_CUE_SETTINGS when validation fails; keep the existing timeout_on_fail string check as-is.src/renderer/components/CueModal.tsx-336-345 (1)
336-345:⚠️ Potential issue | 🟠 MajorBackdrop click-to-close is currently unreachable.
Line 340 checks
e.target === e.currentTarget, but the fullscreen backdrop is a child element (Line 344). Clicking outside the modal hits the backdrop, soonClosedoes not run.🧩 Suggested backdrop close fix
-<div - className="fixed inset-0 flex items-center justify-center" - style={{ zIndex: MODAL_PRIORITIES.CUE_MODAL }} - onClick={(e) => { - if (e.target === e.currentTarget) onClose(); - }} -> +<div className="fixed inset-0 flex items-center justify-center" style={{ zIndex: MODAL_PRIORITIES.CUE_MODAL }}> {/* Backdrop */} - <div className="absolute inset-0 bg-black/50" /> + <div className="absolute inset-0 bg-black/50" onClick={onClose} /> {/* Modal */} <div className="relative rounded-xl shadow-2xl flex flex-col" + onClick={(e) => e.stopPropagation()} style={{ width: 780, maxHeight: '85vh',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueModal.tsx` around lines 336 - 345, The backdrop click handler is unreachable because the fullscreen backdrop is a child element, so the parent div's onClick check (e.target === e.currentTarget) never fires when clicking the backdrop; to fix, move the click-to-close logic onto the backdrop element (the div with className "absolute inset-0 bg-black/50") so clicks on the backdrop call onClose directly (or alternatively set the parent to pointer-events-none and keep the backdrop handling); update the handler references in CueModal.tsx around the MODAL_PRIORITIES.CUE_MODAL container and ensure onClose is invoked from the backdrop element.src/main/ipc/handlers/cue.ts-10-11 (1)
10-11:⚠️ Potential issue | 🟠 MajorReplace synchronous filesystem calls with async alternatives in IPC handlers.
The
cue:readYamlandcue:writeYamlhandlers use blocking filesystem operations (fs.existsSync,fs.readFileSync,fs.writeFileSync) on the Electron main thread, which freezes the UI during disk I/O on slow or network-mounted project roots.⚡ Suggested async rewrite
-import * as fs from 'fs'; +import { promises as fs } from 'fs'; import * as path from 'path'; @@ ipcMain.handle( 'cue:readYaml', withIpcErrorLogging( handlerOpts('readYaml'), async (options: { projectRoot: string }): Promise<string | null> => { const filePath = path.join(options.projectRoot, CUE_YAML_FILENAME); - if (!fs.existsSync(filePath)) { - return null; - } - return fs.readFileSync(filePath, 'utf-8'); + try { + return await fs.readFile(filePath, 'utf-8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + throw error; + } } ) ); @@ ipcMain.handle( 'cue:writeYaml', withIpcErrorLogging( handlerOpts('writeYaml'), async (options: { projectRoot: string; content: string }): Promise<void> => { const filePath = path.join(options.projectRoot, CUE_YAML_FILENAME); - fs.writeFileSync(filePath, options.content, 'utf-8'); + await fs.writeFile(filePath, options.content, 'utf-8'); // The file watcher in CueEngine will automatically detect the change and refresh } ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/cue.ts` around lines 10 - 11, The IPC handlers "cue:readYaml" and "cue:writeYaml" perform blocking disk I/O via fs.existsSync, fs.readFileSync and fs.writeFileSync on the main thread; make both handlers async and replace synchronous calls with async fs.promises (or import from 'fs/promises'): use fs.promises.readFile and fs.promises.writeFile (await them), replace existsSync by either fs.promises.access inside try/catch or by catching ENOENT from readFile, and ensure directories before write with fs.promises.mkdir(dir, { recursive: true }); preserve the existing return/error shapes and rethrow or send back the same error messages so callers behave unchanged.src/main/cue/cue-executor.ts-115-130 (1)
115-130:⚠️ Potential issue | 🟠 MajorAvoid mutating shared
templateContextin place.If a shared context object is reused across concurrent runs, this can leak Cue payload across executions.
🛠️ Proposed fix
- // 3. Populate the template context with Cue event data - templateContext.cue = { + // 3. Populate template context with Cue event data (without mutating shared state) + const runTemplateContext: TemplateContext = { + ...templateContext, + cue: { eventType: event.type, eventTimestamp: event.timestamp, triggerName: subscription.name, runId, filePath: String(event.payload.path ?? ''), fileName: String(event.payload.filename ?? ''), fileDir: String(event.payload.directory ?? ''), fileExt: String(event.payload.extension ?? ''), sourceSession: String(event.payload.sourceSession ?? ''), sourceOutput: String(event.payload.sourceOutput ?? ''), - }; + }, + }; @@ - const substitutedPrompt = substituteTemplateVariables(promptContent, templateContext); + const substitutedPrompt = substituteTemplateVariables(promptContent, runTemplateContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-executor.ts` around lines 115 - 130, The current code mutates the shared templateContext by assigning templateContext.cue = {...}, which can leak Cue payload between concurrent runs; instead create a fresh context (e.g., const localContext = {...templateContext} or use a deep clone/structuredClone) and assign localContext.cue = { eventType: event.type, eventTimestamp: event.timestamp, triggerName: subscription.name, runId, filePath: String(event.payload.path ?? ''), fileName: String(event.payload.filename ?? ''), fileDir: String(event.payload.directory ?? ''), fileExt: String(event.payload.extension ?? ''), sourceSession: String(event.payload.sourceSession ?? ''), sourceOutput: String(event.payload.sourceOutput ?? '') }; then pass that new context into substituteTemplateVariables(promptContent, localContext) instead of mutating templateContext.src/renderer/components/CueYamlEditor.tsx-193-201 (1)
193-201:⚠️ Potential issue | 🟠 MajorSave failures are silently swallowed.
Line [199] drops write/refresh errors, so the user gets no feedback and the failure path is hard to detect.
🛠️ Proposed fix
const handleSave = useCallback(async () => { if (!isValid) return; try { await window.maestro.cue.writeYaml(projectRoot, yamlContent); await window.maestro.cue.refreshSession(sessionId, projectRoot); onClose(); - } catch { - // Let Sentry capture unexpected errors + } catch (error) { + setValidationErrors([ + error instanceof Error ? error.message : 'Failed to save maestro-cue.yaml', + ]); + setIsValid(false); + throw error; } }, [isValid, projectRoot, yamlContent, sessionId, onClose]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueYamlEditor.tsx` around lines 193 - 201, The save handler handleSave currently swallows exceptions from window.maestro.cue.writeYaml and window.maestro.cue.refreshSession so users get no feedback; update handleSave to catch the error into a variable, call a user-visible error path (e.g., setLocal state or call a provided onError/onNotification) to show an error toast/dialog, log the error to Sentry or processLogger (preserving the caught error), and avoid calling onClose when save/refresh failed; reference handleSave, window.maestro.cue.writeYaml, window.maestro.cue.refreshSession and onClose when implementing these changes.src/renderer/components/CueYamlEditor.tsx-131-173 (1)
131-173:⚠️ Potential issue | 🟠 MajorValidation flow has two correctness gaps (stale async results + no initial validation).
Line [136] loads content without validating it, and Line [164] can apply out-of-order async validation responses after subsequent edits.
🛠️ Proposed fix
export function CueYamlEditor({ @@ const [copied, setCopied] = useState(false); const validateTimerRef = useRef<ReturnType<typeof setTimeout>>(); + const validateRequestRef = useRef(0); const yamlTextareaRef = useRef<HTMLTextAreaElement>(null); @@ useEffect(() => { if (!isOpen) return; let cancelled = false; @@ async function loadYaml() { setLoading(true); try { const content = await window.maestro.cue.readYaml(projectRoot); if (cancelled) return; const initial = content ?? YAML_TEMPLATE; setYamlContent(initial); setOriginalContent(initial); + const result = await window.maestro.cue.validateYaml(initial); + if (!cancelled) { + setIsValid(result.valid); + setValidationErrors(result.errors); + } } catch { if (cancelled) return; setYamlContent(YAML_TEMPLATE); setOriginalContent(YAML_TEMPLATE); + setIsValid(true); + setValidationErrors([]); } finally { if (!cancelled) setLoading(false); } } @@ const validateYaml = useCallback((content: string) => { if (validateTimerRef.current) { clearTimeout(validateTimerRef.current); } + const requestId = ++validateRequestRef.current; validateTimerRef.current = setTimeout(async () => { try { const result = await window.maestro.cue.validateYaml(content); + if (requestId !== validateRequestRef.current) return; setIsValid(result.valid); setValidationErrors(result.errors); } catch { + if (requestId !== validateRequestRef.current) return; setIsValid(false); setValidationErrors(['Failed to validate YAML']); } }, 500); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/CueYamlEditor.tsx` around lines 131 - 173, When loading YAML in the CueYamlEditor component (loadYaml), immediately run the same validation path used by validateYaml for the initial content (initial = content ?? YAML_TEMPLATE) so setIsValid and setValidationErrors are populated on mount; also prevent stale/out‑of‑order validation responses by stamping validations with a token/counter (e.g., validationIdRef) captured by each async validateYaml invocation and only applying setIsValid/setValidationErrors when the token matches the latest, and ensure validateTimerRef is cleared on unmount and when starting a new validation to avoid orphaned timers.src/main/cue/cue-engine.ts-826-836 (1)
826-836:⚠️ Potential issue | 🟠 MajorRisk of infinite recursion in completion chain.
After a Cue run completes,
notifyAgentCompletedis called (line 828), which could trigger another subscription listening for that session's completion. If a subscription'ssource_sessionmatches its own owning session, this creates an infinite loop.While this requires misconfiguration, it's easy to accidentally create (e.g., a "retry on failure" pattern that inadvertently triggers itself). Consider adding a guard to break the cycle.
🛡️ Proposed fix to prevent self-triggering loops
+ // Emit completion event for agent completion chains + // This allows downstream subscriptions to react to this Cue run's completion + // Guard: don't notify if this run was itself triggered by an agent.completed subscription + // to prevent infinite loops from self-referential configurations + const wasTriggeredByCompletion = event.type === 'agent.completed'; + if (!wasTriggeredByCompletion) { this.notifyAgentCompleted(sessionId, { sessionName: result.sessionName, status: result.status, exitCode: result.exitCode, durationMs: result.durationMs, stdout: result.stdout, triggeredBy: subscriptionName, }); + }Alternatively, track a "chain depth" counter and break at a reasonable limit (e.g., 5 levels).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-engine.ts` around lines 826 - 836, Add a guard to prevent self-triggering completion loops when notifyAgentCompleted is called from Cue runs: modify notifyAgentCompleted (and the call site in cue-engine.ts that passes triggeredBy/subscriptionName) to either 1) ignore/return early when triggeredBy === sessionId (or when the subscription’s source_session equals its owning session) or 2) accept and propagate an optional chainDepth counter that is incremented on each notifyAgentCompleted call and stops processing when it exceeds a safe limit (e.g., 5); update the call in cue-engine.ts that invokes notifyAgentCompleted(sessionId, {..., triggeredBy: subscriptionName}) to pass/increment chainDepth and ensure the notifyAgentCompleted implementation enforces the break condition.
🟡 Minor comments (6)
src/renderer/components/SettingsModal.tsx-3751-3757 (1)
3751-3757:⚠️ Potential issue | 🟡 MinorUse platform-aware shortcut text in the Cue hint.
Line 3756 hardcodes a macOS key combo. On Windows/Linux this will show the wrong shortcut.
🛠️ Suggested fix
<kbd className="px-1.5 py-0.5 rounded text-[10px] font-mono" style={{ backgroundColor: theme.colors.border }} > - ⌘⇧U + {formatMetaKey()}+Shift+U </kbd>{' '}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SettingsModal.tsx` around lines 3751 - 3757, The hint in SettingsModal currently hardcodes the macOS shortcut "⌘⇧U" inside the <kbd> element; change it to render a platform-aware string (e.g. "⌘⇧U" for macOS and "Ctrl+Shift+U" for Windows/Linux). Locate the JSX in SettingsModal (the "Open the Cue dashboard with" text and the <kbd> element) and replace the hardcoded text with a small platform check (window.navigator.platform or an existing isMac/isDarwin helper) that returns the correct display string, then use that variable inside the <kbd> element so the UI shows the right shortcut per OS.src/renderer/components/SessionItem.tsx-161-168 (1)
161-168:⚠️ Potential issue | 🟡 MinorAdd an accessible label for the Cue badge.
Line 164 relies on
titleonly. That won’t reliably expose the status to screen readers. Add an explicitaria-label(and keep the icon decorative).♿ Suggested fix
{cueSubscriptionCount != null && cueSubscriptionCount > 0 && ( <span className="shrink-0 flex items-center" title={`Maestro Cue active (${cueSubscriptionCount} subscription${cueSubscriptionCount === 1 ? '' : 's'})`} + aria-label={`Maestro Cue active (${cueSubscriptionCount} subscription${cueSubscriptionCount === 1 ? '' : 's'})`} > - <Zap className="w-3 h-3" style={{ color: '#2dd4bf' }} fill="#2dd4bf" /> + <Zap + className="w-3 h-3" + style={{ color: '#2dd4bf' }} + fill="#2dd4bf" + aria-hidden="true" + /> </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionItem.tsx` around lines 161 - 168, The Cue badge uses only a title attribute which is not reliably exposed to screen readers; update the span that renders when cueSubscriptionCount is present (the element containing the Zap icon and using the title string) to include an explicit aria-label with the same descriptive text (e.g. "Maestro Cue active (X subscriptions)"), mark the Zap icon component as decorative (aria-hidden="true" or equivalent) so assistive tech reads the span label only, and keep the existing title for hover tooltips; changes should be made around the JSX that references cueSubscriptionCount and the Zap component in SessionItem.src/__tests__/main/cue/cue-executor.test.ts-501-510 (1)
501-510:⚠️ Potential issue | 🟡 MinorRemove the unused first spy on
mockChild.kill.Line 503 creates a spy (
killSpy) that is immediately replaced by the second spy at line 509 and never asserted. The first spy is redundant and should be removed.🛠️ Suggested fix
it('should send SIGTERM when timeout expires', async () => { const config = createExecutionConfig({ timeoutMs: 5000 }); - const killSpy = vi.spyOn(mockChild, 'kill'); const resultPromise = executeCuePrompt(config); await vi.advanceTimersByTimeAsync(0); // Wait: re-spy after child is created const childKill = vi.spyOn(mockChild, 'kill');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-executor.test.ts` around lines 501 - 510, Remove the redundant spy creation "const killSpy = vi.spyOn(mockChild, 'kill');" in the test "should send SIGTERM when timeout expires" and rely on the later spy "const childKill = vi.spyOn(mockChild, 'kill');" for assertions; update any assertions/refers to use childKill (or remove references to killSpy) so only the single spy on mockChild.kill created after the child is instantiated is used in executeCuePrompt test.src/__tests__/main/cue/cue-reconciler.test.ts-276-279 (1)
276-279:⚠️ Potential issue | 🟡 MinorMake the sleep-duration assertion deterministic.
Line 276 and Line 278 call
Date.now()separately, sogapMscan exceedsleepDurationby a few ms and make Line 284 flaky.Suggested change
const sleepDuration = 60 * 60 * 1000; // 1 hour + const now = Date.now(); const config = makeConfig({ - sleepStartMs: Date.now() - sleepDuration, - wakeTimeMs: Date.now(), + sleepStartMs: now - sleepDuration, + wakeTimeMs: now, sessions, });Also applies to: 284-284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-reconciler.test.ts` around lines 276 - 279, The test is using Date.now() twice causing a race that makes the sleep-duration assertion flaky; capture a single timestamp (e.g. const now = Date.now()) and use it for both sleepStartMs and wakeTimeMs when calling makeConfig so gapMs is exactly computed from the same reference, then update any related assertions (sleepDuration, gapMs) to derive from that single now value to make the assertion deterministic.src/__tests__/main/cue/cue-ipc-handlers.test.ts-69-80 (1)
69-80:⚠️ Potential issue | 🟡 MinorAdd
cue:getQueueStatusto the IPC contract test.The registration coverage misses one live channel, so a regression there won’t fail this suite.
Suggested patch
function createMockEngine() { return { getStatus: vi.fn().mockReturnValue([]), getActiveRuns: vi.fn().mockReturnValue([]), getActivityLog: vi.fn().mockReturnValue([]), start: vi.fn(), stop: vi.fn(), stopRun: vi.fn().mockReturnValue(true), stopAll: vi.fn(), + getQueueStatus: vi.fn().mockReturnValue(new Map<string, number>()), refreshSession: vi.fn(), isEnabled: vi.fn().mockReturnValue(false), }; } @@ const expectedChannels = [ 'cue:getStatus', 'cue:getActiveRuns', 'cue:getActivityLog', 'cue:enable', 'cue:disable', 'cue:stopRun', 'cue:stopAll', + 'cue:getQueueStatus', 'cue:refreshSession', 'cue:readYaml', 'cue:writeYaml', 'cue:validateYaml', ];Also applies to: 113-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-ipc-handlers.test.ts` around lines 69 - 80, The IPC contract test is missing the live channel "cue:getQueueStatus" so add it to the registration coverage where channels are asserted/registered; update the mock engine or channel registration list used in the test (referencing createMockEngine and the test that collects registered IPC channels) to include "cue:getQueueStatus" alongside the other cue channels so the suite fails on regressions for that channel as well.src/main/preload/cue.ts-14-55 (1)
14-55:⚠️ Potential issue | 🟡 MinorPreload Cue types are drifting from the shared core types.
CueSessionStatushere is missingprojectRoot, and maintaining duplicate interfaces in this file will keep causing drift.♻️ Proposed refactor
import { ipcRenderer } from 'electron'; +import type { + CueEventType, + CueRunStatus, + CueEvent, + CueRunResult, + CueSessionStatus, +} from '../cue/cue-types'; -/** Event types that can trigger a Cue subscription */ -export type CueEventType = 'time.interval' | 'file.changed' | 'agent.completed'; - -/** Status of a Cue run */ -export type CueRunStatus = 'running' | 'completed' | 'failed' | 'timeout' | 'stopped'; - -/** An event instance produced by a trigger */ -export interface CueEvent { - id: string; - type: CueEventType; - timestamp: string; - triggerName: string; - payload: Record<string, unknown>; -} - -/** Result of a completed (or failed/timed-out) Cue run */ -export interface CueRunResult { - runId: string; - sessionId: string; - sessionName: string; - subscriptionName: string; - event: CueEvent; - status: CueRunStatus; - stdout: string; - stderr: string; - exitCode: number | null; - durationMs: number; - startedAt: string; - endedAt: string; -} - -/** Status summary for a Cue-enabled session */ -export interface CueSessionStatus { - sessionId: string; - sessionName: string; - toolType: string; - enabled: boolean; - subscriptionCount: number; - activeRuns: number; - lastTriggered?: string; - nextTrigger?: string; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/preload/cue.ts` around lines 14 - 55, The local CueSessionStatus interface in this file is missing the projectRoot field and duplicates the shared core type; replace the local declaration by importing and re-exporting the shared core CueSessionStatus type (or extend it) so the preload types stay in sync—update any references to use the imported CueSessionStatus symbol and remove the duplicate interface to prevent drift.
🧹 Nitpick comments (10)
src/renderer/components/History/HistoryEntryItem.tsx (1)
23-28: Consider extracting the CUE teal color to theme or constants.The CUE color
#06b6d4is hardcoded here and duplicated inHistoryFilterToggle.tsx. Consider defining a shared constant (e.g.,CUE_TEALas seen inCueModal.tsx) or adding it to the theme system for consistency and easier maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/HistoryEntryItem.tsx` around lines 23 - 28, The CUE teal color is hardcoded in HistoryEntryItem (and duplicated in HistoryFilterToggle); extract that hex into a shared constant (e.g., export const CUE_TEAL = '#06b6d4') or add it to the theme and update HistoryEntryItem's case 'CUE' to reference CUE_TEAL for bg/text/border computations; also update HistoryFilterToggle (and any other files like CueModal) to import and use the new CUE_TEAL constant so the color is defined in one place for consistency and easier maintenance.src/__tests__/renderer/components/CueModal.test.tsx (1)
328-339: Consider using a more robust selector for the close button.The test relies on finding a button with
.lucide-xclass, which is an implementation detail of the Lucide icon library. If the library changes its class naming, this test would break silently. Consider adding adata-testidoraria-labelto the close button in the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CueModal.test.tsx` around lines 328 - 339, Test uses an implementation-specific selector (.lucide-x) to find the close button in CueModal; update the CueModal close button element (in the CueModal component) to include a stable identifier (e.g., aria-label="Close" or data-testid="cue-modal-close"), then update this test (CueModal.test.tsx) to query the button with a robust selector such as screen.getByRole('button', { name: /close/i }) or screen.getByTestId('cue-modal-close') and assert mockOnClose is called after clicking that element.src/renderer/components/History/HistoryFilterToggle.tsx (1)
26-31: Duplicate hardcoded CUE color.Same hardcoded color as in
HistoryEntryItem.tsx. Consider extracting to a shared constant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/HistoryFilterToggle.tsx` around lines 26 - 31, The CUE color is duplicated in HistoryFilterToggle ('case "CUE"') and HistoryEntryItem; extract a shared constant (e.g., export const CUE_COLOR = '#06b6d4') into a common constants module (or the shared colors file) and replace the hardcoded string in both HistoryFilterToggle and HistoryEntryItem to import CUE_COLOR, then derive bg ('CUE_COLOR' + '20') and border ('CUE_COLOR' + '40') where needed so both components use the single source of truth.src/renderer/components/HistoryDetailModal.tsx (1)
177-183: Deduplicate CUE color tokens to keep styling consistent.The same hard-coded CUE colors are repeated in multiple places; extracting a single constant avoids drift and simplifies future theme updates.
Suggested change
+const CUE_PILL_COLORS = { + bg: '#06b6d420', + text: '#06b6d4', + border: '#06b6d440', +} as const; + const getPillColor = () => { if (entry.type === 'AUTO') { return { bg: theme.colors.warning + '20', text: theme.colors.warning, border: theme.colors.warning + '40', }; } if (entry.type === 'CUE') { - return { - bg: '#06b6d420', - text: '#06b6d4', - border: '#06b6d440', - }; + return CUE_PILL_COLORS; } return { bg: theme.colors.accent + '20', text: theme.colors.accent, border: theme.colors.accent + '40', }; };style={{ - backgroundColor: '#06b6d420', - color: '#06b6d4', - border: '1px solid `#06b6d440`', + backgroundColor: CUE_PILL_COLORS.bg, + color: CUE_PILL_COLORS.text, + border: `1px solid ${CUE_PILL_COLORS.border}`, }}Also applies to: 374-382
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/HistoryDetailModal.tsx` around lines 177 - 183, Extract the repeated hard-coded CUE color tokens used in HistoryDetailModal into a single exported constant (e.g., CUE_COLOR = { bg: '#06b6d420', text: '#06b6d4', border: '#06b6d440' }) and replace the inline object in the branch that checks entry.type === 'CUE' and any other occurrences (including the block around lines 374-382) to reference that constant; update imports/exports as needed so other components can reuse the same CUE_COLOR instead of duplicating the literal values.src/__tests__/renderer/components/SessionList.test.tsx (1)
3192-3195: Replace fixed-time sleeps withwaitFor()to avoid flaky tests in CI environments.Lines 3194 and 3221 use
setTimeout(50)to "settle" effects. This is timing-sensitive and can intermittently fail in slower CI environments. Replace withwaitFor()using the DOM condition being checked immediately after.Suggested change
- // Give async effects time to settle - await act(async () => { - await new Promise((r) => setTimeout(r, 50)); - }); - - expect(screen.queryByTestId('icon-zap')).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByTestId('icon-zap')).not.toBeInTheDocument(); + });- await act(async () => { - await new Promise((r) => setTimeout(r, 50)); - }); - - expect(screen.queryByTestId('icon-zap')).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByTestId('icon-zap')).not.toBeInTheDocument(); + });Also applies to: 3221–3223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/SessionList.test.tsx` around lines 3192 - 3195, Replace the fixed 50ms setTimeout-based waits in the test’s async act blocks (the occurrences of "await act(async () => { await new Promise((r) => setTimeout(r, 50)); });") with testing-library's waitFor and an explicit DOM/state condition — e.g., wrap the assertion or the condition you expect after the effect inside waitFor so it polls until satisfied; also ensure waitFor is imported from "@testing-library/react" at the top of the test file and remove the setTimeout-based act blocks (apply the same replacement for both occurrences around lines 3194 and 3221).src/renderer/components/LogViewer.tsx (1)
19-25: Consider consolidating log-level literals into one typed source-of-truth.
'debug' | ... | 'cue'and the level array are now duplicated in multiple places, making future level changes easy to miss.♻️ Suggested refactor
interface SystemLogEntry { timestamp: number; level: 'debug' | 'info' | 'warn' | 'error' | 'toast' | 'autorun' | 'cue'; message: string; context?: string; data?: unknown; } +type LogLevel = SystemLogEntry['level']; +const LOG_LEVELS: readonly LogLevel[] = [ + 'debug', + 'info', + 'warn', + 'error', + 'toast', + 'autorun', + 'cue', +]; -const enabledLevels = new Set<'debug' | 'info' | 'warn' | 'error' | 'toast' | 'autorun' | 'cue'>( +const enabledLevels = new Set<LogLevel>( (['debug', 'info', 'warn', 'error'] as const).filter( (level) => LOG_LEVEL_PRIORITY[level] >= LOG_LEVEL_PRIORITY[logLevel] ) ); -const [selectedLevels, setSelectedLevelsState] = useState< - Set<'debug' | 'info' | 'warn' | 'error' | 'toast' | 'autorun' | 'cue'> ->(() => { +const [selectedLevels, setSelectedLevelsState] = useState<Set<LogLevel>>(() => { ... }); -{(['debug', 'info', 'warn', 'error', 'toast', 'autorun', 'cue'] as const).map((level) => { +{LOG_LEVELS.map((level) => {Also applies to: 70-101, 490-530
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/LogViewer.tsx` around lines 19 - 25, Consolidate the duplicated log-level literals by introducing a single source-of-truth: create a const tuple (e.g. const LOG_LEVELS = ['debug','info','warn','error','toast','autorun','cue'] as const) and derive a type alias (type LogLevel = typeof LOG_LEVELS[number]); then update SystemLogEntry.level to use LogLevel and replace any separate level arrays or literals elsewhere in this file (and other usages around the LogViewer component) to reference LOG_LEVELS and LogLevel instead of repeating the string union or separate arrays so future changes are made in one place.src/main/cue/cue-file-watcher.ts (2)
75-81: Consider awaiting watcher.close() for cleaner shutdown.The
watcher.close()method in chokidar v3 returns a Promise. While the current synchronous cleanup works (the watcher will close asynchronously), making the cleanup function async would ensure resources are fully released before the caller continues.However, since this is a cleanup function typically called during shutdown, the current approach is acceptable and won't cause issues in practice.
♻️ Optional: async cleanup for completeness
-export function createCueFileWatcher(config: CueFileWatcherConfig): () => void { +export function createCueFileWatcher(config: CueFileWatcherConfig): () => Promise<void> { // ... existing code ... - return () => { + return async () => { for (const timer of debounceTimers.values()) { clearTimeout(timer); } debounceTimers.clear(); - watcher.close(); + await watcher.close(); }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-file-watcher.ts` around lines 75 - 81, The cleanup returned currently calls watcher.close() synchronously; change the returned function to be async and await watcher.close() so the chokidar watcher is fully closed before shutdown; keep clearing debounceTimers (clearTimeout on each timer and debounceTimers.clear()) before or after awaiting close as preferred, and ensure the function signature becomes async () => { ... await watcher.close(); } so callers can await the cleanup.
70-73: Consider using the cue-db logger instead of console.error.The module comment mentions "the parent engine will handle logging", but using
console.errordirectly bypasses any structured logging. If a logger utility is available (as suggested by the imports list mentioningsrc/main/utils/logger.ts), consider using it for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-file-watcher.ts` around lines 70 - 73, The file watcher currently calls console.error inside watcher.on('error', ...) which bypasses the project's logger; change this to use the shared logger (e.g. logger.error) imported from the project's logger module, preserving the existing message and including the triggerName and the error object as structured metadata (so use logger.error with the message and the error/metadata) in the watcher.on('error', ...) handler to ensure consistent, structured logging.src/__tests__/renderer/components/CueHelpModal.test.tsx (1)
44-64: Consider adding missing theme properties to mockTheme.The
mockThemeobject is missing theaccentForegroundproperty that may be used by the component (as seen in other modal components). While tests pass, having a complete mock ensures tests don't break if the component starts using this property.♻️ Add missing theme property
const mockTheme: Theme = { id: 'test-dark', name: 'Test Dark', mode: 'dark', colors: { bgMain: '#1a1a1a', bgSidebar: '#252525', bgActivity: '#2d2d2d', border: '#444444', textMain: '#ffffff', textDim: '#888888', accent: '#007acc', + accentForeground: '#ffffff', error: '#ff4444', success: '#44ff44', warning: '#ffaa00', cursor: '#ffffff', selection: '#264f78', terminalBackground: '#000000', }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CueHelpModal.test.tsx` around lines 44 - 64, The mockTheme object used in CueHelpModal.test.tsx is missing the accentForeground property defined on Theme; update the mockTheme constant to include an accentForeground string (e.g., matching accent or a contrasting color) inside the colors object so components referencing colors.accentForeground do not break—locate the mockTheme definition in the test and add the accentForeground key to the colors map.src/main/cue/cue-db.ts (1)
20-30: Use Cue union types fortypeandstatusfields.These are currently plain strings; switching to
CueEventType/CueRunStatuswill catch invalid values at compile time.♻️ Proposed refactor
import Database from 'better-sqlite3'; import * as path from 'path'; import * as fs from 'fs'; import { app } from 'electron'; +import type { CueEventType, CueRunStatus } from './cue-types'; @@ export interface CueEventRecord { id: string; - type: string; + type: CueEventType; @@ - status: string; + status: CueRunStatus; @@ export function recordCueEvent(event: { id: string; - type: string; + type: CueEventType; @@ - status: string; + status: CueRunStatus; payload?: string; }): void { @@ -export function updateCueEventStatus(id: string, status: string): void { +export function updateCueEventStatus(id: string, status: CueRunStatus): void {Also applies to: 144-152, 173-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-db.ts` around lines 20 - 30, Change the plain string types in the CueEventRecord interface to the appropriate Cue union types: replace the type of the "type" field with CueEventType and the "status" field with CueRunStatus; update any other occurrences of those fields (the other interfaces/usages around the other occurrences noted) to use the same union types so compilation will catch invalid values and adjust any imports to bring in CueEventType and CueRunStatus from their module. Ensure referenced symbols are CueEventRecord, CueEventType, and CueRunStatus and update related definitions/usages accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (78)
CLAUDE-IPC.mdCLAUDE-PATTERNS.mdCLAUDE.mdpackage.jsonsrc/__tests__/main/cue/cue-completion-chains.test.tssrc/__tests__/main/cue/cue-concurrency.test.tssrc/__tests__/main/cue/cue-db.test.tssrc/__tests__/main/cue/cue-engine.test.tssrc/__tests__/main/cue/cue-executor.test.tssrc/__tests__/main/cue/cue-file-watcher.test.tssrc/__tests__/main/cue/cue-filter.test.tssrc/__tests__/main/cue/cue-ipc-handlers.test.tssrc/__tests__/main/cue/cue-reconciler.test.tssrc/__tests__/main/cue/cue-sleep-wake.test.tssrc/__tests__/main/cue/cue-yaml-loader.test.tssrc/__tests__/main/process-listeners/exit-listener.test.tssrc/__tests__/renderer/components/CueHelpModal.test.tsxsrc/__tests__/renderer/components/CueModal.test.tsxsrc/__tests__/renderer/components/CueYamlEditor.test.tsxsrc/__tests__/renderer/components/History/HistoryEntryItem.test.tsxsrc/__tests__/renderer/components/History/HistoryFilterToggle.test.tsxsrc/__tests__/renderer/components/HistoryDetailModal.test.tsxsrc/__tests__/renderer/components/HistoryPanel.test.tsxsrc/__tests__/renderer/components/LogViewer.test.tsxsrc/__tests__/renderer/components/SessionItemCue.test.tsxsrc/__tests__/renderer/components/SessionList.test.tsxsrc/__tests__/renderer/hooks/useCue.test.tssrc/__tests__/renderer/hooks/useCueAutoDiscovery.test.tssrc/__tests__/setup.tssrc/__tests__/web/mobile/MobileHistoryPanel.test.tsxsrc/main/cue/cue-db.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-executor.tssrc/main/cue/cue-file-watcher.tssrc/main/cue/cue-filter.tssrc/main/cue/cue-reconciler.tssrc/main/cue/cue-types.tssrc/main/cue/cue-yaml-loader.tssrc/main/index.tssrc/main/ipc/handlers/cue.tssrc/main/ipc/handlers/director-notes.tssrc/main/ipc/handlers/index.tssrc/main/preload/cue.tssrc/main/preload/directorNotes.tssrc/main/preload/files.tssrc/main/preload/index.tssrc/main/process-listeners/exit-listener.tssrc/main/process-listeners/types.tssrc/main/utils/logger.tssrc/renderer/App.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/CueHelpModal.tsxsrc/renderer/components/CueModal.tsxsrc/renderer/components/CueYamlEditor.tsxsrc/renderer/components/DirectorNotes/UnifiedHistoryTab.tsxsrc/renderer/components/History/HistoryEntryItem.tsxsrc/renderer/components/History/HistoryFilterToggle.tsxsrc/renderer/components/HistoryDetailModal.tsxsrc/renderer/components/HistoryPanel.tsxsrc/renderer/components/LogViewer.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionItem.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/constants/cuePatterns.tssrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/global.d.tssrc/renderer/hooks/agent/useAgentSessionManagement.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/useCue.tssrc/renderer/hooks/useCueAutoDiscovery.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.tssrc/shared/logger-types.tssrc/shared/templateVariables.tssrc/shared/types.ts
Comment on lines
+252
to
+266
| /** Stops a specific running execution */ | ||
| stopRun(runId: string): boolean { | ||
| const run = this.activeRuns.get(runId); | ||
| if (!run) return false; | ||
|
|
||
| run.abortController?.abort(); | ||
| run.result.status = 'stopped'; | ||
| run.result.endedAt = new Date().toISOString(); | ||
| run.result.durationMs = Date.now() - new Date(run.result.startedAt).getTime(); | ||
|
|
||
| this.activeRuns.delete(runId); | ||
| this.pushActivityLog(run.result); | ||
| this.deps.onLog('cue', `[CUE] Run stopped: ${runId}`); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Concurrency tracking not updated when run is stopped.
When stopRun is called, activeRunCount is not decremented and drainQueue is not called. This causes the concurrency slot to be "leaked"—after stopping a run, the engine will think it has one fewer available slot than it actually does. Eventually, if enough runs are stopped, no new runs will be dispatched even though no runs are actually active.
Compare with the completion path in doExecuteCueRun (lines 821-824) which properly decrements and drains.
🐛 Proposed fix to maintain concurrency consistency
stopRun(runId: string): boolean {
const run = this.activeRuns.get(runId);
if (!run) return false;
run.abortController?.abort();
run.result.status = 'stopped';
run.result.endedAt = new Date().toISOString();
run.result.durationMs = Date.now() - new Date(run.result.startedAt).getTime();
+ const sessionId = run.result.sessionId;
this.activeRuns.delete(runId);
this.pushActivityLog(run.result);
+
+ // Decrement active run count and drain queue (same as normal completion)
+ const count = this.activeRunCount.get(sessionId) ?? 1;
+ this.activeRunCount.set(sessionId, Math.max(0, count - 1));
+ this.drainQueue(sessionId);
+
this.deps.onLog('cue', `[CUE] Run stopped: ${runId}`);
return true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/cue/cue-engine.ts` around lines 252 - 266, stopRun currently aborts
and cleans up a run but doesn't update concurrency tracking; update stopRun (the
method named stopRun in cue-engine.ts) to decrement this.activeRunCount
(mirroring the completion path in doExecuteCueRun) and then call
this.drainQueue() after removing the run from this.activeRuns and before
returning true; ensure you guard the decrement so activeRunCount never becomes
negative (e.g., only decrement if > 0) to maintain consistent concurrency
accounting.
Comment on lines
+188
to
+206
| if (sshRemoteConfig?.enabled && sshStore) { | ||
| const sshWrapConfig: SshSpawnWrapConfig = { | ||
| command, | ||
| args: finalArgs, | ||
| cwd: projectRoot, | ||
| prompt: substitutedPrompt, | ||
| customEnvVars: effectiveEnvVars, | ||
| agentBinaryName: agentDef.binaryName, | ||
| promptArgs: agentDef.promptArgs, | ||
| noPromptSeparator: agentDef.noPromptSeparator, | ||
| }; | ||
|
|
||
| const sshResult = await wrapSpawnWithSsh(sshWrapConfig, sshRemoteConfig, sshStore); | ||
| command = sshResult.command; | ||
| spawnArgs = sshResult.args; | ||
| spawnCwd = sshResult.cwd; | ||
| spawnEnvVars = sshResult.customEnvVars; | ||
| prompt = sshResult.prompt; | ||
|
|
There was a problem hiding this comment.
Don’t silently execute locally when SSH is requested but unavailable.
When sshRemoteConfig.enabled is true and sshStore is missing, the run currently continues locally. Also, stdin prompt routing is keyed off “SSH enabled” instead of “SSH wrapping actually applied”.
🛠️ Proposed fix
let spawnArgs = finalArgs;
let spawnCwd = projectRoot;
let spawnEnvVars = effectiveEnvVars;
let prompt: string | undefined = substitutedPrompt;
+ let sendPromptViaStdin = false;
- if (sshRemoteConfig?.enabled && sshStore) {
+ if (sshRemoteConfig?.enabled) {
+ if (!sshStore) {
+ const message = `SSH is enabled for session "${session.name}" but SSH settings store is unavailable`;
+ onLog('error', message);
+ return {
+ runId,
+ sessionId: session.id,
+ sessionName: session.name,
+ subscriptionName: subscription.name,
+ event,
+ status: 'failed',
+ stdout: '',
+ stderr: message,
+ exitCode: null,
+ durationMs: Date.now() - startTime,
+ startedAt,
+ endedAt: new Date().toISOString(),
+ };
+ }
const sshWrapConfig: SshSpawnWrapConfig = {
command,
@@
command = sshResult.command;
spawnArgs = sshResult.args;
spawnCwd = sshResult.cwd;
spawnEnvVars = sshResult.customEnvVars;
prompt = sshResult.prompt;
+ sendPromptViaStdin = Boolean(sshResult.prompt);
@@
- if (prompt && sshRemoteConfig?.enabled) {
+ if (prompt && sendPromptViaStdin) {
// SSH large prompt mode — send via stdin
child.stdin?.write(prompt);
child.stdin?.end();Based on learnings: When implementing features that spawn agent processes, support SSH remote execution by checking for sshRemoteConfig, using wrapSpawnWithSsh() from ssh-spawn-wrapper.ts, and passing the SSH store adapter.
Also applies to: 290-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/cue/cue-executor.ts` around lines 188 - 206, When
sshRemoteConfig.enabled is true but sshStore is undefined we must fail fast
instead of falling back to local execution: if sshRemoteConfig.enabled and
!sshStore throw or return an error. Only call wrapSpawnWithSsh(sshWrapConfig,
sshRemoteConfig, sshStore) when sshStore is present and then assign command,
spawnArgs, spawnCwd, spawnEnvVars and prompt from its result; update any logic
that routes stdin/prompt to use whether SSH wrapping was actually applied (e.g.,
a boolean like sshWrapped or presence of sshResult.prompt) rather than
sshRemoteConfig.enabled. Apply the same change pattern to the similar block
referenced around lines 290-297 so both places validate sshStore before
proceeding.
Comment on lines
+336
to
+339
| setTimeout(() => { | ||
| if (!child.killed) { | ||
| child.kill('SIGKILL'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js ChildProcess killed property behavior when child.kill() is called
💡 Result:
In Node.js, child.killed (aka subprocess.killed) is not “the process is dead”.
- It is set to
trueafterchild.kill()successfully sends/delivers a signal to the child process. [1] - It does not mean the child has terminated (the process may ignore the signal, take time to exit, be stuck, etc.). [1][2]
- To know the child actually ended, rely on the
'exit'/'close'events and/orchild.exitCode/child.signalCode, notchild.killed. [1][2]
Also note: kill() sends a signal (default SIGTERM) and may not actually terminate the process; behavior differs by platform (e.g., Windows treats it more like a forceful kill). [2]
🏁 Script executed:
fd cue-executor.ts --type fRepository: RunMaestro/Maestro
Length of output: 90
🏁 Script executed:
sed -n '325,350p' src/main/cue/cue-executor.tsRepository: RunMaestro/Maestro
Length of output: 662
SIGKILL escalation is broken due to incorrect process-state check.
child.killed is set to true immediately after child.kill('SIGTERM') is called, regardless of whether the process actually exits. Since the setTimeout fires 5 seconds later, the condition if (!child.killed) will be false, and the SIGKILL escalation will never execute. If the process ignores SIGTERM, it will never be forcefully terminated.
Use child.exitCode === null && child.signalCode === null to check if the process is still running:
Fix
// Escalate to SIGKILL after delay
setTimeout(() => {
- if (!child.killed) {
+ if (child.exitCode === null && child.signalCode === null) {
child.kill('SIGKILL');
}
}, SIGKILL_DELAY_MS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setTimeout(() => { | |
| if (!child.killed) { | |
| child.kill('SIGKILL'); | |
| } | |
| setTimeout(() => { | |
| if (child.exitCode === null && child.signalCode === null) { | |
| child.kill('SIGKILL'); | |
| } | |
| }, SIGKILL_DELAY_MS); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/cue/cue-executor.ts` around lines 336 - 339, The SIGKILL escalation
currently checks child.killed which is set immediately and prevents escalation;
change the timeout handler that sends SIGKILL to instead test the child's
runtime state using child.exitCode === null && child.signalCode === null, and
only call child.kill('SIGKILL') if both are null; update the setTimeout callback
that currently references child.killed and child.kill('SIGKILL') (the block
around child.kill('SIGTERM') and the subsequent setTimeout) to perform this new
check before escalating.
Comment on lines
+45
to
+56
| if (filterValue.startsWith('>=')) { | ||
| const threshold = Number(filterValue.slice(2)); | ||
| if (Number(payloadValue) < threshold) return false; | ||
| } else if (filterValue.startsWith('<=')) { | ||
| const threshold = Number(filterValue.slice(2)); | ||
| if (Number(payloadValue) > threshold) return false; | ||
| } else if (filterValue.startsWith('>')) { | ||
| const threshold = Number(filterValue.slice(1)); | ||
| if (Number(payloadValue) <= threshold) return false; | ||
| } else if (filterValue.startsWith('<')) { | ||
| const threshold = Number(filterValue.slice(1)); | ||
| if (Number(payloadValue) >= threshold) return false; |
There was a problem hiding this comment.
Guard numeric comparisons against NaN values.
At Line 45–Line 56, malformed thresholds (or non-numeric payload values) can yield NaN and incorrectly pass the filter.
Suggested fix
+function toFiniteNumber(value: unknown): number | null {
+ const n = typeof value === 'number' ? value : Number(value);
+ return Number.isFinite(n) ? n : null;
+}
+
export function matchesFilter(
payload: Record<string, unknown>,
filter: Record<string, string | number | boolean>
): boolean {
@@
if (filterValue.startsWith('>=')) {
- const threshold = Number(filterValue.slice(2));
- if (Number(payloadValue) < threshold) return false;
+ const threshold = toFiniteNumber(filterValue.slice(2));
+ const current = toFiniteNumber(payloadValue);
+ if (threshold === null || current === null || current < threshold) return false;
} else if (filterValue.startsWith('<=')) {
- const threshold = Number(filterValue.slice(2));
- if (Number(payloadValue) > threshold) return false;
+ const threshold = toFiniteNumber(filterValue.slice(2));
+ const current = toFiniteNumber(payloadValue);
+ if (threshold === null || current === null || current > threshold) return false;
} else if (filterValue.startsWith('>')) {
- const threshold = Number(filterValue.slice(1));
- if (Number(payloadValue) <= threshold) return false;
+ const threshold = toFiniteNumber(filterValue.slice(1));
+ const current = toFiniteNumber(payloadValue);
+ if (threshold === null || current === null || current <= threshold) return false;
} else if (filterValue.startsWith('<')) {
- const threshold = Number(filterValue.slice(1));
- if (Number(payloadValue) >= threshold) return false;
+ const threshold = toFiniteNumber(filterValue.slice(1));
+ const current = toFiniteNumber(payloadValue);
+ if (threshold === null || current === null || current >= threshold) return false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (filterValue.startsWith('>=')) { | |
| const threshold = Number(filterValue.slice(2)); | |
| if (Number(payloadValue) < threshold) return false; | |
| } else if (filterValue.startsWith('<=')) { | |
| const threshold = Number(filterValue.slice(2)); | |
| if (Number(payloadValue) > threshold) return false; | |
| } else if (filterValue.startsWith('>')) { | |
| const threshold = Number(filterValue.slice(1)); | |
| if (Number(payloadValue) <= threshold) return false; | |
| } else if (filterValue.startsWith('<')) { | |
| const threshold = Number(filterValue.slice(1)); | |
| if (Number(payloadValue) >= threshold) return false; | |
| function toFiniteNumber(value: unknown): number | null { | |
| const n = typeof value === 'number' ? value : Number(value); | |
| return Number.isFinite(n) ? n : null; | |
| } | |
| if (filterValue.startsWith('>=')) { | |
| const threshold = toFiniteNumber(filterValue.slice(2)); | |
| const current = toFiniteNumber(payloadValue); | |
| if (threshold === null || current === null || current < threshold) return false; | |
| } else if (filterValue.startsWith('<=')) { | |
| const threshold = toFiniteNumber(filterValue.slice(2)); | |
| const current = toFiniteNumber(payloadValue); | |
| if (threshold === null || current === null || current > threshold) return false; | |
| } else if (filterValue.startsWith('>')) { | |
| const threshold = toFiniteNumber(filterValue.slice(1)); | |
| const current = toFiniteNumber(payloadValue); | |
| if (threshold === null || current === null || current <= threshold) return false; | |
| } else if (filterValue.startsWith('<')) { | |
| const threshold = toFiniteNumber(filterValue.slice(1)); | |
| const current = toFiniteNumber(payloadValue); | |
| if (threshold === null || current === null || current >= threshold) return false; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/cue/cue-filter.ts` around lines 45 - 56, Guard numeric comparisons
against NaN by parsing both the threshold and the payload before comparing: in
cue-filter.ts where filterValue, payloadValue and threshold are used, call
Number(...) once for payloadValue and for each threshold (extracted from
filterValue.slice(...)), check with isNaN(parsedThreshold) or
isNaN(parsedPayload) and return false (fail the filter) when either is NaN, then
perform the numeric comparisons as before; ensure you replace repeated
Number(...) calls with the parsed variables to avoid inconsistent conversions.
Comment on lines
+344
to
+362
| onCueRun: async (sessionId, _prompt, event) => { | ||
| // Stub for Phase 03 executor integration — returns a placeholder result. | ||
| // The actual executor (cue-executor.ts) is wired in a future phase. | ||
| logger.info(`[CUE] Run triggered for session ${sessionId}: ${event.triggerName}`, 'Cue'); | ||
| return { | ||
| runId: event.id, | ||
| sessionId, | ||
| sessionName: '', | ||
| subscriptionName: event.triggerName, | ||
| event, | ||
| status: 'completed' as const, | ||
| stdout: '', | ||
| stderr: '', | ||
| exitCode: 0, | ||
| durationMs: 0, | ||
| startedAt: new Date().toISOString(), | ||
| endedAt: new Date().toISOString(), | ||
| }; | ||
| }, |
There was a problem hiding this comment.
onCueRun is still a stub and reports fake successful runs.
At Line 345–Line 362, Cue executions are marked completed without launching any real agent/process work. This makes the feature appear healthy while doing nothing.
If helpful, I can draft the wiring patch to connect this callback to the existing cue-executor path and open a follow-up issue with concrete acceptance checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/index.ts` around lines 344 - 362, The onCueRun stub returns a fake
successful result; replace it by calling the real executor in cue-executor
(import the executor entrypoint, e.g. executeCue or runCue from
cue-executor.ts), await its run for the given sessionId and event, map the
executor's returned fields (runId, status, stdout, stderr, exitCode, durationMs,
startedAt, endedAt, subscriptionName) into the object currently returned by
onCueRun, log start/finish via logger.info/logger.error, and on exceptions set
status to 'failed' and include error info; ensure onCueRun remains async and
preserves sessionId/sessionName/event in the returned payload.
Comment on lines
+81
to
+85
| maestroCue: { | ||
| id: 'maestroCue', | ||
| label: 'Maestro Cue', | ||
| keys: ['Meta', 'Shift', 'u'], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and get line counts
wc -l src/renderer/constants/shortcuts.ts
# Check the maestroCue shortcut at lines 81-85
echo "=== maestroCue shortcut (lines 81-85) ==="
sed -n '81,85p' src/renderer/constants/shortcuts.ts
# Check around line 167 for toggleTabUnread
echo -e "\n=== toggleTabUnread shortcut (lines 165-175) ==="
sed -n '165,175p' src/renderer/constants/shortcuts.ts
# Search for all occurrences of Meta+Shift+u pattern
echo -e "\n=== All shortcuts using Meta and Shift combinations ==="
rg -n "Meta.*Shift|Shift.*Meta" src/renderer/constants/shortcuts.tsRepository: RunMaestro/Maestro
Length of output: 2978
🏁 Script executed:
#!/bin/bash
# Check useMainKeyboardHandler.ts to understand the handler logic
if [ -f "src/renderer/hooks/useMainKeyboardHandler.ts" ]; then
echo "=== File found, checking handler logic around line 423-426 and 484-489 ==="
wc -l src/renderer/hooks/useMainKeyboardHandler.ts
echo -e "\n--- Handler section for maestroCue (around line 423-426) ---"
sed -n '420,430p' src/renderer/hooks/useMainKeyboardHandler.ts
echo -e "\n--- Handler section for TAB_SHORTCUTS (around line 484-489) ---"
sed -n '480,495p' src/renderer/hooks/useMainKeyboardHandler.ts
else
echo "File not found. Let's search for useMainKeyboardHandler"
find . -name "*useMainKeyboardHandler*" -type f
fiRepository: RunMaestro/Maestro
Length of output: 235
🏁 Script executed:
#!/bin/bash
# Verify the exact structure of shortcuts
echo "=== Check GENERAL_SHORTCUTS and TAB_SHORTCUTS structure ==="
rg -n "GENERAL_SHORTCUTS|TAB_SHORTCUTS|maestroCue|toggleTabUnread" src/renderer/constants/shortcuts.ts | head -20Repository: RunMaestro/Maestro
Length of output: 276
🏁 Script executed:
#!/bin/bash
# Check useMainKeyboardHandler.ts to understand the handler logic
file_path="src/renderer/hooks/keyboard/useMainKeyboardHandler.ts"
echo "=== File info ==="
wc -l "$file_path"
echo -e "\n=== Search for maestroCue handler ==="
rg -n "maestroCue" "$file_path" -A 3 -B 1
echo -e "\n=== Search for toggleTabUnread handler ==="
rg -n "toggleTabUnread" "$file_path" -A 3 -B 1
echo -e "\n=== Check TAB_SHORTCUTS usage ==="
rg -n "TAB_SHORTCUTS" "$file_path" -A 5 -B 2Repository: RunMaestro/Maestro
Length of output: 966
🏁 Script executed:
#!/bin/bash
# Get more context around how shortcuts are handled overall
file_path="src/renderer/hooks/keyboard/useMainKeyboardHandler.ts"
# Look for keyboard event handling and how GENERAL_SHORTCUTS vs TAB_SHORTCUTS are processed
echo "=== Search for GENERAL_SHORTCUTS or shortcut matching logic ==="
rg -n "GENERAL_SHORTCUTS|Object\.entries.*SHORTCUTS|matchesKeys" "$file_path" -A 2 -B 1 | head -50Repository: RunMaestro/Maestro
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Get the full context of how shortcuts are processed - look at the structure around line 423 and 677
file_path="src/renderer/hooks/keyboard/useMainKeyboardHandler.ts"
echo "=== maestroCue handler context (lines 410-450) ==="
sed -n '410,450p' "$file_path"
echo -e "\n=== toggleTabUnread handler context (lines 660-690) ==="
sed -n '660,690p' "$file_path"
echo -e "\n=== Search for where TAB_SHORTCUTS is checked ==="
rg -n "isTabShortcut" "$file_path" -B 5 | head -30Repository: RunMaestro/Maestro
Length of output: 4660
Keyboard shortcut conflict: maestroCue and toggleTabUnread both use ['Meta', 'Shift', 'u'].
maestroCue (line 81-85) is evaluated in the general keyboard handler (useMainKeyboardHandler.ts line 423) and will prevent toggleTabUnread (line 168-171) from ever executing. When the maestroCue feature flag is enabled, pressing Cmd+Shift+U will always open the Cue modal, blocking the "Toggle Tab Unread" functionality. Consider using a different key combination for maestroCue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/constants/shortcuts.ts` around lines 81 - 85, The maestroCue
shortcut (constant maestroCue) conflicts with toggleTabUnread because both use
['Meta','Shift','u'], so update the maestroCue keys to a different,
non-conflicting combination (e.g., choose an unused Meta+Shift+<key>) in the
shortcuts constants and ensure any UI/help text or tests referencing maestroCue
are updated accordingly; verify in the main keyboard handler
(useMainKeyboardHandler) that toggleTabUnread still responds to Meta+Shift+U
when the maestroCue feature flag is enabled.
Greptile SummaryThis PR implements Maestro Cue, a comprehensive event-driven automation system that enables agents to trigger prompts based on time intervals, file changes, and agent completions. The implementation spans 79 files with 13,244 insertions and includes: Core Architecture:
Infrastructure:
UI Components:
Integration:
Critical Issue: Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[App Startup] --> Init[Initialize CueEngine]
Init --> CheckFlag{Encore Feature<br/>maestroCue enabled?}
CheckFlag -->|No| Skip[Skip engine start]
CheckFlag -->|Yes| StartEngine[engine.start]
StartEngine --> ScanSessions[Scan all sessions]
ScanSessions --> LoadConfigs[Load maestro-cue.yaml<br/>from project roots]
LoadConfigs --> SetupTimers[Setup interval timers]
SetupTimers --> SetupWatchers[Setup file watchers]
SetupWatchers --> SetupYAMLWatch[Setup YAML hot reload]
subgraph "Event Triggers"
Timer[time.interval<br/>fires every N minutes]
FileChange[file.changed<br/>chokidar detects changes]
AgentDone[agent.completed<br/>exit listener notifies]
end
Timer --> CheckFilter{Payload<br/>filter match?}
FileChange --> CheckFilter
AgentDone --> CheckFilter
CheckFilter -->|No| Drop[Drop event]
CheckFilter -->|Yes| CheckConcurrency{Active runs <<br/>max_concurrent?}
CheckConcurrency -->|No| Queue[Add to event queue]
CheckConcurrency -->|Yes| CheckFanOut{fan_out<br/>targets?}
CheckFanOut -->|No| Execute[executeCueRun<br/>on owner session]
CheckFanOut -->|Yes| FanOut[Execute on each<br/>target session]
Execute --> CallStub[Call onCueRun callback]
FanOut --> CallStub
CallStub --> Stub{Stub or real<br/>implementation?}
Stub -->|Stub| FakeResult[Return fake 'completed'<br/>NO ACTUAL EXECUTION]
Stub -->|Real| CallExecutor[executeCuePrompt]
CallExecutor --> ReadPrompt[Read prompt file]
ReadPrompt --> SubstituteVars[Substitute template vars]
SubstituteVars --> BuildArgs[Build agent spawn args]
BuildArgs --> CheckSSH{SSH remote<br/>enabled?}
CheckSSH -->|Yes| WrapSSH[Wrap with SSH]
CheckSSH -->|No| SpawnLocal[Spawn agent process]
WrapSSH --> SpawnLocal
SpawnLocal --> CaptureOutput[Capture stdout/stderr]
CaptureOutput --> Timeout{Timeout?}
Timeout -->|Yes| SendSIGTERM[SIGTERM then SIGKILL]
Timeout -->|No| WaitExit[Wait for exit]
SendSIGTERM --> RecordResult[Record CueRunResult]
WaitExit --> RecordResult
FakeResult --> RecordResult
RecordResult --> NotifyCompletion[Notify agent.completed<br/>for chains]
NotifyCompletion --> CheckFanIn{Fan-in<br/>subscription?}
CheckFanIn -->|Yes| TrackCompletion[Track completion]
CheckFanIn -->|No| Done[Done]
TrackCompletion --> AllDone{All sources<br/>completed?}
AllDone -->|No| WaitMore[Wait for more]
AllDone -->|Yes| FireFanIn[Fire downstream sub<br/>with combined output]
FireFanIn --> Done
Queue --> DrainQueue[Drain queue when<br/>slot available]
DrainQueue --> CheckConcurrency
style Stub fill:#ff6b6b
style FakeResult fill:#ff6b6b
style CallExecutor fill:#51cf66
Last reviewed commit: 6c2c252 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+344
to
+362
| onCueRun: async (sessionId, _prompt, event) => { | ||
| // Stub for Phase 03 executor integration — returns a placeholder result. | ||
| // The actual executor (cue-executor.ts) is wired in a future phase. | ||
| logger.info(`[CUE] Run triggered for session ${sessionId}: ${event.triggerName}`, 'Cue'); | ||
| return { | ||
| runId: event.id, | ||
| sessionId, | ||
| sessionName: '', | ||
| subscriptionName: event.triggerName, | ||
| event, | ||
| status: 'completed' as const, | ||
| stdout: '', | ||
| stderr: '', | ||
| exitCode: 0, | ||
| durationMs: 0, | ||
| startedAt: new Date().toISOString(), | ||
| endedAt: new Date().toISOString(), | ||
| }; | ||
| }, |
There was a problem hiding this comment.
stub implementation doesn't execute prompts — entire feature non-functional
The onCueRun callback is still using a placeholder that immediately returns fake "completed" results without spawning any agent processes. Import and call executeCuePrompt from cue-executor.ts:
Suggested change
| onCueRun: async (sessionId, _prompt, event) => { | |
| // Stub for Phase 03 executor integration — returns a placeholder result. | |
| // The actual executor (cue-executor.ts) is wired in a future phase. | |
| logger.info(`[CUE] Run triggered for session ${sessionId}: ${event.triggerName}`, 'Cue'); | |
| return { | |
| runId: event.id, | |
| sessionId, | |
| sessionName: '', | |
| subscriptionName: event.triggerName, | |
| event, | |
| status: 'completed' as const, | |
| stdout: '', | |
| stderr: '', | |
| exitCode: 0, | |
| durationMs: 0, | |
| startedAt: new Date().toISOString(), | |
| endedAt: new Date().toISOString(), | |
| }; | |
| }, | |
| onCueRun: async (sessionId, prompt, event) => { | |
| const session = sessionsStore.get('sessions', []).find((s: any) => s.id === sessionId); | |
| if (!session) { | |
| logger.error(`[CUE] Session not found: ${sessionId}`, 'Cue'); | |
| return { | |
| runId: event.id, | |
| sessionId, | |
| sessionName: '', | |
| subscriptionName: event.triggerName, | |
| event, | |
| status: 'failed' as const, | |
| stdout: '', | |
| stderr: 'Session not found', | |
| exitCode: null, | |
| durationMs: 0, | |
| startedAt: new Date().toISOString(), | |
| endedAt: new Date().toISOString(), | |
| }; | |
| } | |
| const { executeCuePrompt } = await import('./cue/cue-executor'); | |
| const agentConfigs = agentConfigsStore.get('configs', {}) as Record<string, any>; | |
| const sessionConfig = agentConfigs[session.toolType] || {}; | |
| return await executeCuePrompt({ | |
| runId: event.id, | |
| session: { | |
| id: session.id, | |
| name: session.name, | |
| toolType: session.toolType, | |
| cwd: session.cwd || session.fullPath || os.homedir(), | |
| projectRoot: session.cwd || session.fullPath || os.homedir(), | |
| }, | |
| subscription: { name: event.triggerName, event: event.type, enabled: true, prompt }, | |
| event, | |
| promptPath: prompt, | |
| toolType: session.toolType, | |
| projectRoot: session.cwd || session.fullPath || os.homedir(), | |
| templateContext: { | |
| session: { | |
| id: session.id, | |
| name: session.name, | |
| toolType: session.toolType, | |
| cwd: session.cwd || session.fullPath || os.homedir(), | |
| projectRoot: session.cwd || session.fullPath || os.homedir(), | |
| }, | |
| }, | |
| timeoutMs: 30 * 60 * 1000, | |
| sshRemoteConfig: session.sshRemoteConfig, | |
| customPath: sessionConfig.customPath, | |
| customArgs: sessionConfig.customArgs, | |
| customEnvVars: sessionConfig.customEnvVars, | |
| customModel: session.customModel, | |
| onLog: (level, message) => logger.cue(message, 'Cue'), | |
| sshStore: createSshRemoteStoreAdapter(store), | |
| agentConfigValues: sessionConfig, | |
| }); | |
| }, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto Run Summary
Documents processed:
Total tasks completed: 48
Changes
CHANGES
This PR was automatically created by Maestro Auto Run.
Summary by CodeRabbit
Release Notes