-
Notifications
You must be signed in to change notification settings - Fork 246
fix: argument denylist + Windows shell escaping#536
fix: argument denylist + Windows shell escaping#536openasocket wants to merge 195 commits intoRunMaestro:0.16.0-RCfrom
Conversation
Summary
- Add DENIED_CUSTOM_ARGS denylist in
applyAgentConfigOverrides()to prevent--no-sandbox,--dangerous-auto-approve,-y, and other security-sensitive flags from being injected viasessionCustomArgs(TASK-S01) - Harden Windows
escapeArgsForShell()to cover;, backtick,$, and%metacharacters; addcanRunWithoutShell()optimization for resolved .exe paths (TASK-S03)
Cherry-picked from feat/gemini-cli audit fixes -- shared security hardening that applies to all agents.
Test plan
- 6 new agent-args denylist tests pass
- 65 shellEscape tests pass (30+ per-metacharacter)
- 117 total tests pass for changed files
- Full test suite: 513/515 test files pass (1 pre-existing flaky failure in AgentSessionsModal unrelated to this PR)
- Existing agent spawn behavior unchanged for clean custom args
Generated with Claude Code
Summary by CodeRabbit
-
New Features
- Windows agents can now execute certain commands directly without invoking the shell, improving performance and reducing security exposure for already-resolved executables.
- Added security controls to deny specific custom arguments from being passed through agent configurations, with warning logs for denied arguments.
-
Tests
- Expanded test coverage for shell escaping across Windows batch and PowerShell contexts, including metacharacter handling and edge cases.
- Added tests for custom argument denylist behavior and validation scenarios.
|
Important Review skippedToo many files! This PR contains 231 files, which is 81 over the limit of 150. Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: Files ignored due to path filters (1)
Files selected for processing (231)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughWalkthroughThis pull request introduces Windows shell decision logic optimization and custom argument validation. It adds a Changes
Estimated code review effort3 (Moderate) | ~25 minutes Possibly related PRs
Pre-merge checks | 3Passed checks (3 passed)
Tip: You can configure your own custom pre-merge checks in the settings. Finishing TouchesGenerate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds two security hardening features: a Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%%
Loading
flowchart TD A[Windows agent spawn request] --> B{canRunWithoutShell\ncommandToSpawn?} B -- "true (.exe with path)" --> C[useShell = false\nspawn directly\nno shell metachar risk] B -- "false (.cmd/.bat/bare name)" --> D[buildExpandedEnv\ncustomEnvVarsToPass] D --> E[getWindowsShellForAgentExecution] E --> F{Shell priority} F -- "1 * customShellPath" --> G[useShell = true\nshell = custom] F -- "2 * currentShell cmd.exe" --> H[useShell = true\nshell = current] F -- "3 * default" --> I[useShell = true\nshell = PowerShell] C --> J[spawn process] G & H & I --> K[escapeArgsForShell\nargs] K --> J subgraph "agent-args.ts -- before spawn" L[parseCustomArgs\nsessionCustomArgs] --> M{DENIED_CUSTOM_ARGS\n.has arg?} M -- "exact match " --> N[warn + strip] M -- "flag=value bypass " --> O[passes through\n security gap] M -- "clean arg" --> P[keep] end Last reviewed commit: b0cbd15 |
| const filteredCustomArgs = parsedCustomArgs.filter((arg) => { | ||
| if (DENIED_CUSTOM_ARGS.has(arg)) { | ||
| logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg }); | ||
| return false; | ||
| } | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Denylist bypass via flag=value syntax
The filter uses DENIED_CUSTOM_ARGS.has(arg), which performs an exact string match on each parsed token. A user can bypass every entry on the denylist by attaching a value with =:
--approval-mode=bypassPermissions- token is"--approval-mode=bypassPermissions"- not in the Set - passes through--dangerously-skip-permissions=true- same bypass--dangerous-auto-approve=1- same bypass
--approval-mode in particular is designed to accept a value, making this an exploitable path.
The fix is to also block any token that starts with a denied flag followed by =:
const isDenied =
DENIED_CUSTOM_ARGS.has(arg) ||
[...DENIED_CUSTOM_ARGS].some((denied) => arg.startsWith(`${denied}=`));
if (isDenied) {
logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg });
return false;
}
return true;
});
| if (canRunWithoutShell(commandToSpawn)) { | ||
| // Command is a fully-resolved .exe path -- spawn directly without shell. | ||
| // This is the safest option: no shell metacharacter interpretation at all. | ||
| // Note: .cmd/.bat files (e.g., npm-installed CLIs like Gemini CLI) | ||
| // cannot use this path -- they require shell interpretation. | ||
| useShell = false; | ||
| logger.info(`Using shell:false for resolved .exe on Windows`, LOG_CONTEXT, { | ||
| agentId: agent?.id, | ||
| command: commandToSpawn, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildExpandedEnv skipped for .exe spawns -- env vars not pre-expanded
In the original code, buildExpandedEnv(customEnvVarsToPass) was always called on Windows before spawning. This PR moves it into the else branch, so it is now skipped whenever canRunWithoutShell returns true.
When spawning with shell: false, Node.js passes env vars directly to the OS. Any custom env var whose value contains references like %SOME_VAR% or $SOME_VAR will not be expanded because there is no shell to do it. buildExpandedEnv exists precisely to pre-expand those references, so skipping it for .exe spawns means user-configured env vars that reference other variables will be passed in raw, unexpanded form.
Consider calling buildExpandedEnv before the canRunWithoutShell branch to keep the pre-expansion behaviour consistent:
const expandedEnv = buildExpandedEnv(customEnvVarsToPass);
customEnvVarsToPass = Object.fromEntries(
Object.entries(expandedEnv).filter(([_, value]) => value !== undefined)
) as Record<string, string>;
if (canRunWithoutShell(commandToSpawn)) {
useShell = false;
// ...
} else {
// shell selection ...
}
| export function canRunWithoutShell(command: string): boolean { | ||
| const path = require('path'); | ||
| const ext = path.extname(command).toLowerCase(); | ||
| const hasPath = /\\|\//.test(command); | ||
|
|
||
| // Must have a directory path (not just a basename) and be a native .exe | ||
| return hasPath && ext === '.exe'; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require('path') inside function body
canRunWithoutShell calls require('path') at the call site instead of importing it at the module top level. The same pattern exists for require('fs') in getWindowsShellForAgentExecution. While Node.js caches require so there is no runtime cost, this is inconsistent with the ES-module import style used throughout the rest of the codebase and makes it harder to statically analyze or tree-shake the module.
Consider adding a top-level import:
and removing the inline require('path') call.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/main/process-manager/utils/shellEscape.test.ts`:
- Around line 115-118: Update the test for empty-string handling: change the
expectation for escapeCmdArg('') to expect '""' (an explicitly quoted empty
string) because cmd.exe drops unquoted empty args, and add a new test asserting
escapePowerShellArg('') returns '' (an empty string) to cover PowerShell
behavior; locate and modify the tests referencing escapeCmdArg and add one for
escapePowerShellArg in the existing shellEscape.test.ts so they reflect these
correct platform-specific outputs.
In `@src/main/ipc/handlers/process.ts`:
- Around line 330-331: The Windows-shell skip currently checks
config.sessionSshRemoteConfig?.enabled before resolving SSH, causing shell/PATH
setup to be skipped even when getSshRemoteConfig() returns no usable config;
change the logic in the process spawn path (around isWindows and
canRunWithoutShell) to call getSshRemoteConfig() first and validate the returned
config (e.g., non-null and has a valid remoteId or matches the terminal request)
before trusting config.sessionSshRemoteConfig?.enabled; if getSshRemoteConfig()
yields no usable remote, fall back to the Windows shell/PATH handling (i.e.,
only skip shell when a resolved, usable SSH config exists), and apply the same
adjustment to the other spawn block referenced (the 389-403 region) so both
branches consistently resolve SSH before deciding to skip the Windows shell.
In `@src/main/utils/agent-args.ts`:
- Around line 10-18: The denylist currently only checks exact tokens against
DENIED_CUSTOM_ARGS so forms like "--approval-mode=full-auto" slip through and
"full-auto" can become an orphaned positional; update the token-filtering logic
that processes agent args to (1) normalize tokens by splitting at the first '='
for long options before checking against DENIED_CUSTOM_ARGS, and (2) when a
denied flag that accepts a value is encountered (either as "--flag=value" or
"--flag" followed by a separate token), ensure you consume/remove the value
token as well so no orphan remains; keep DENIED_CUSTOM_ARGS as the canonical set
and apply these checks in the same function that iterates/filters args so both
"--flag=value" and "--flag value" are blocked.
i Review info
Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a28b537d-456a-4067-ad21-df920296bac8
Files selected for processing (6)
src/__tests__/main/process-manager/utils/shellEscape.test.tssrc/__tests__/main/utils/agent-args.test.tssrc/main/ipc/handlers/process.tssrc/main/process-manager/utils/index.tssrc/main/process-manager/utils/shellEscape.tssrc/main/utils/agent-args.ts
| it('should handle empty string', () => { | ||
| // Empty string has no special chars and is short, returned as-is | ||
| expect(escapeCmdArg('')).toBe(''); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analysis chain
Script executed:
fd -t f "shellEscape" --extension ts --extension js --extension tsx --extension jsx
Repository: RunMaestro/Maestro
Length of output: 168
Script executed:
cat -n src/__tests__/main/process-manager/utils/shellEscape.test.ts | head -150
Repository: RunMaestro/Maestro
Length of output: 6276
Script executed:
rg "escapeCmdArg|escapePowerShellArg" -A 15 --type ts --type tsx | head -100
Repository: RunMaestro/Maestro
Length of output: 89
Script executed:
cat -n src/main/process-manager/utils/shellEscape.ts
Repository: RunMaestro/Maestro
Length of output: 12971
Script executed:
rg "escapePowerShellArg.*''" src/__tests__/main/process-manager/utils/shellEscape.test.ts -A 5
Repository: RunMaestro/Maestro
Length of output: 322
Script executed:
rg "escapeCmdArg|escapePowerShellArg" --type ts -B 3 -A 3 | head -200
Repository: RunMaestro/Maestro
Length of output: 18692
Script executed:
cat -n src/__tests__/main/process-manager/utils/shellEscape.test.ts | sed -n '132,300p'
Repository: RunMaestro/Maestro
Length of output: 7317
Script executed:
rg "escapePowerShellArg.*empty|empty.*escapePowerShellArg" -i src/__tests__/main/process-manager/utils/shellEscape.test.ts -A 5
Repository: RunMaestro/Maestro
Length of output: 44
Script executed:
tail -n +132 src/__tests__/main/process-manager/utils/shellEscape.test.ts | head -70
Repository: RunMaestro/Maestro
Length of output: 2408
Empty strings must be quoted to survive shell argument parsing.
The expectation escapeCmdArg('') === '' is incorrect. In cmd.exe, an empty unquoted argument is parsed away--it must be "" to be preserved as an actual argument token. Similarly, escapePowerShellArg('') should return '', but there's no test for this case.
Suggested fix
- // Empty string has no special chars and is short, returned as-is
- expect(escapeCmdArg('')).toBe('');
- });
+ it('should preserve empty string arguments', () => {
+ expect(escapeCmdArg('')).toBe('""');
+ expect(escapePowerShellArg('')).toBe("''");
+ });
Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/process-manager/utils/shellEscape.test. ts` around lines
115 - 118, Update the test for empty-string handling: change the expectation for
escapeCmdArg('') to expect '""' (an explicitly quoted empty string) because
cmd.exe drops unquoted empty args, and add a new test asserting
escapePowerShellArg('') returns '' (an empty string) to cover PowerShell
behavior; locate and modify the tests referencing escapeCmdArg and add one for
escapePowerShellArg in the existing shellEscape.test.ts so they reflect these
correct platform-specific outputs.
src/main/ipc/handlers/process.ts
Outdated
| if (isWindows && !config.sessionSshRemoteConfig?.enabled) { | ||
| // Use expanded environment with custom env vars to ensure PATH includes all binary locations | ||
| const expandedEnv = buildExpandedEnv(customEnvVarsToPass); | ||
|
// Filter out undefined values to match Record |
||
| customEnvVarsToPass = Object.fromEntries( | ||
| Object.entries(expandedEnv).filter(([_, value]) => value !== undefined) | ||
|
) as Record |
||
|
|
||
| // Get the preferred shell for Windows (custom -> current -> PowerShell) | ||
| // PowerShell is preferred over cmd.exe to avoid command line length limits | ||
| const customShellPath = settingsStore.get('customShellPath', '') as string; | ||
| const shellConfig = getWindowsShellForAgentExecution({ | ||
| customShellPath, | ||
| currentShell: shellToUse, | ||
| }); | ||
| shellToUse = shellConfig.shell; | ||
| useShell = shellConfig.useShell; | ||
|
|
||
| logger.info(`Forcing shell execution for agent on Windows for PATH access`, LOG_CONTEXT, { | ||
| agentId: agent?.id, | ||
| command: commandToSpawn, | ||
| args: argsToSpawn, | ||
| shell: shellToUse, | ||
| shellSource: shellConfig.source, | ||
| }); | ||
| if (canRunWithoutShell(commandToSpawn)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve SSH before deciding to skip the Windows shell path.
This guard keys off sessionSshRemoteConfig.enabled, not on a successful getSshRemoteConfig() result. If enabled is true but remoteId is null/stale--or if a terminal request carries this field--we skip the Windows shell/PATH setup and then fall through to a local shell:false spawn. On Windows that breaks .cmd wrappers and other PATH-dependent agents; please move the Windows fallback after SSH resolution, or rerun it when no usable remote config is found.
Also applies to: 389-403
Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/handlers/process.ts` around lines 330 - 331, The Windows-shell
skip currently checks config.sessionSshRemoteConfig?.enabled before resolving
SSH, causing shell/PATH setup to be skipped even when getSshRemoteConfig()
returns no usable config; change the logic in the process spawn path (around
isWindows and canRunWithoutShell) to call getSshRemoteConfig() first and
validate the returned config (e.g., non-null and has a valid remoteId or matches
the terminal request) before trusting config.sessionSshRemoteConfig?.enabled; if
getSshRemoteConfig() yields no usable remote, fall back to the Windows
shell/PATH handling (i.e., only skip shell when a resolved, usable SSH config
exists), and apply the same adjustment to the other spawn block referenced (the
389-403 region) so both branches consistently resolve SSH before deciding to
skip the Windows shell.
| const DENIED_CUSTOM_ARGS = new Set([ | ||
| '--no-sandbox', | ||
| '--include-directories', | ||
| '--dangerous-auto-approve', | ||
| '--dangerously-skip-permissions', | ||
| '--dangerously-bypass-approvals-and-sandbox', | ||
| '--approval-mode', | ||
| '-y', | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analysis chain
Script executed:
Repository: RunMaestro/Maestro
Length of output: 1745
Script executed:
Repository: RunMaestro/Maestro
Length of output: 882
Script executed:
rg -n "parseCustomArgs" src/main/utils/agent-args.ts -A 15
Repository: RunMaestro/Maestro
Length of output: 1233
The denylist still misses --flag=value forms and leaves orphan values.
The filter at lines 167-173 only rejects exact token matches. --approval-mode=full-auto passes through unchanged, and --approval-mode full-auto strips only the flag, leaving full-auto as a positional argument. The filtering needs to normalize long options on = and consume the following token for denied flags that take a value.
Possible shape of the fix
'--no-sandbox',
'--include-directories',
'--dangerous-auto-approve',
'--dangerously-skip-permissions',
'--dangerously-bypass-approvals-and-sandbox',
'--approval-mode',
'-y',
]);
+const DENIED_CUSTOM_ARGS_WITH_VALUE = new Set([
+ '--include-directories',
+ '--approval-mode',
+]);
const parsedCustomArgs = parseCustomArgs(effectiveCustomArgs);
-const filteredCustomArgs = parsedCustomArgs.filter((arg) => {
- if (DENIED_CUSTOM_ARGS.has(arg)) {
- logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg });
- return false;
- }
- return true;
-});
+const filteredCustomArgs: string[] = [];
+for (let i = 0; i < parsedCustomArgs.length; i++) {
+ const arg = parsedCustomArgs[i];
+ const normalizedArg = arg.split('=')[0];
+ if (DENIED_CUSTOM_ARGS.has(normalizedArg)) {
+ logger.warn('Stripped denied custom arg', LOG_CONTEXT, { arg: normalizedArg });
+ if (DENIED_CUSTOM_ARGS_WITH_VALUE.has(normalizedArg) && !arg.includes('=') && i + 1 < parsedCustomArgs.length) {
+ i += 1;
+ }
+ continue;
+ }
+ filteredCustomArgs.push(arg);
+}
Also applies to: 167-176
Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/utils/agent-args.ts` around lines 10 - 18, The denylist currently
only checks exact tokens against DENIED_CUSTOM_ARGS so forms like
"--approval-mode=full-auto" slip through and "full-auto" can become an orphaned
positional; update the token-filtering logic that processes agent args to (1)
normalize tokens by splitting at the first '=' for long options before checking
against DENIED_CUSTOM_ARGS, and (2) when a denied flag that accepts a value is
encountered (either as "--flag=value" or "--flag" followed by a separate token),
ensure you consume/remove the value token as well so no orphan remains; keep
DENIED_CUSTOM_ARGS as the canonical set and apply these checks in the same
function that iterates/filters args so both "--flag=value" and "--flag value"
are blocked.
Maintainer ReviewGood direction on both the denylist and Must Fix1. Denylist bypass via The Fix: split on 2.
3. Merge conflict This PR currently has merge conflicts with Nice to Have
SummaryPlease rebase onto |
and groups from external apps and OS notification clicks.
- Add deep-links module with URL parsing, protocol registration,
single-instance locking, and cross-platform event handling
- Wire notification click handlers to navigate to the originating
agent/tab via deep link dispatch
- Thread sessionId/tabId context through notification preload bridge
- Add onDeepLink listener in renderer with routing to existing
navigation handlers
- Register maestro:// protocol in electron-builder config
- Add 18 tests covering URL parsing and notification click wiring
notification click handler to prevent malformed URLs with special chars
- Add process.exit(0) after app.quit() so secondary instances exit
immediately without running further module-level setup
- Use useRef for sessions in deep link effect to avoid tearing down
and re-registering the IPC listener on every sessions change
- Guard against navigating to non-existent session IDs in deep link
handler to prevent invalid UI state
- Add cross-reference comment in global.d.ts linking to canonical
ParsedDeepLink type (can't import in ambient declaration file)
- Add test for URI-encoding round-trip in notification click handler
buildGroupDeepLink(), and buildFocusDeepLink() utilities
- Add {{AGENT_DEEP_LINK}}, {{TAB_DEEP_LINK}}, {{GROUP_DEEP_LINK}}
template variables available in system prompts, custom AI commands,
and Auto Run documents
- Wire activeTabId and groupId into TemplateContext at all call sites
(agentStore, useInputProcessing, useRemoteHandlers,
useDocumentProcessor, useMergeTransferHandlers, batch-processor)
- Refactor notifications.ts to use shared buildSessionDeepLink()
- Add sessionId/tabId to notifyToast callers where context is available
(merge, transfer, summarize, PR creation)
- Add docs/deep-links.md documentation page with URL format, usage
examples, template variables, and platform behavior
- Add 8 tests for URL builders, 6 tests for template variable
substitution including URI encoding
Broadcast new history entries via IPC when they are added, subscribe in
the UnifiedHistoryTab with RAF batching and deduplication, and extend
the HistoryStatsBar with spinning Active agent count and Queued message
count indicators derived from the Zustand session store.
- Replace unstable sessionNameMap Zustand selector (new Map per render) with
a stable ref + subscribe pattern to avoid streaming effect re-subscription
- Dedupe within batch before merging; compute setTotalEntries and
setHistoryStats from deduplicated entries only (not raw batch)
- Clear pendingEntriesRef on cleanup to prevent stale replay after resubscribe
- Use HistoryEntry (not UnifiedHistoryEntry) in preload callback type since
the wire payload lacks sourceSessionId
- Use canonical UsageStats interface in global.d.ts (fixes pre-existing
cacheReadTokens/cacheWriteTokens field name mismatch)
- Add `symphony: boolean` (default true) to EncoreFeatureFlags
- Gate Symphony modal, menu item, keyboard shortcut (|Y), and command palette entry
- Add `symphonyRegistryUrls` setting for user-configured additional registry URLs
- Replace single `fetchRegistry()` with `fetchRegistries()` that fetches default + custom URLs in parallel
- Merge repositories by slug (default registry wins on conflicts), isolated per-URL error handling
- Add Symphony toggle + Registry Sources UI in Settings > Encore tab
- Update tests for new symphony flag across all encore feature assertions
- Skip registry cache when custom source URLs are configured (stale cache fix)
- Runtime-validate symphonyRegistryUrls from settings store
- Reset modal-open flags when Encore Feature toggles are disabled
- Normalize registry URLs before duplicate/default checks
- Add aria-label to icon-only registry URL remove button
- Expose setSymphonyRegistryUrls in getSettingsActions()
- Validate persisted symphonyRegistryUrls with Array.isArray guard
- 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
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.
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.
- 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)
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
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.
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).
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
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
- 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
- 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)
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).
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.
Add repo and poll_minutes fields to CueSubscription interface.
Add cue_github_seen SQLite table with 5 CRUD functions for tracking
seen GitHub items (isGitHubItemSeen, markGitHubItemSeen, hasAnyGitHubSeen,
pruneGitHubSeen, clearGitHubSeenForSubscription).
Create cue-github-poller.ts module that polls GitHub CLI for new PRs/issues,
seeds existing items on first run, and fires CueEvents for new items.
Comprehensive test suite with 17 test cases covering all polling behaviors.
All 264 Cue tests pass, lint clean.
- Session bookmarks hide automatically when filtering unread agents only
- Unread tab filter now keeps busy tabs visible for better awareness
- Tab keyboard shortcuts now work across AI, terminal, and file modes
- Unread-only navigation now treats busy tabs as navigable targets
visits unread or busy agents instead of all visible agents. The
currently active agent is always included so you don't get stuck.
Claude ID: 21220442-55d4-4a7c-ba6c-34d278946cf4
Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
- OS notifications are kept alive to prevent lost click events
- Notification lifecycle cleanup now runs on close and click reliably
- Agent drawer auto-focuses the search box when opened for faster picking
- Tab Bar tooltips now reflect user-configured shortcut bindings dynamically
- Unread-only session filtering preserves your active session visibility
- Unread filter now considers unread/busy worktree children, not just parents
- Worktrees auto-expand while filtering unread so nothing important is hidden
- Right panel focus ring clears correctly when focus leaves the panel
- Focus ring styling no longer bumps z-index, reducing visual layering glitches
- Tab naming: resolve early from partial output (2s interval) instead of
waiting for full process exit; bump timeout 30s - 45s for cold starts
- Cue pipeline: prefer subscription name match over stale agent_id when
resolving target sessions; fall back to name-based lookup when agent_id
is absent
- Auto Run: return empty content with notFound flag for missing files
instead of throwing, avoiding errors on deleted/renamed documents
- LogViewer: add per-entry copy button with clipboard support
- AutoRunDocumentSelector: use file icons from theme, move task percentage
badge to right-aligned position
- Web server: avoid returning stale logs from wrong tab during new tab
creation race
visible rather than conditional on hasUnreadAgents. Removed the
now-unused hasUnreadAgents prop and memo.
- Edge-based prompts: each trigger-agent edge can carry its own prompt,
enabling multiple triggers to feed the same agent with different instructions
- Custom trigger labels: user-defined names (e.g. "Morning Check") displayed
on trigger nodes and serialized as subscription labels in YAML
- Run Now button: manually trigger any subscription from the Cue dashboard,
bypassing event conditions via synthetic event dispatch
- Visual polish: pulsing animation on active nodes, larger arrowheads on
selected edges, disabled drawer buttons when viewing all pipelines
- YAML round-trip: yamlToPipeline deserializes edge prompts and trigger labels;
pipelineToYaml generates unique prompt file paths for multi-trigger agents
- Tests: 4 new test cases covering edge prompts, custom labels, multi-trigger
serialization, and unique prompt file path generation
Claude ID: 3ce19e17-50c0-47ff-bc36-f979900ec700
Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
preventing the unresponsive gap after the loading screen clears.
Progress stages now follow an orchestra warm-up sequence:
tuning instruments - reading the score - seating the musicians -
warming up the ensemble - the concertmaster rises - maestro
takes the podium - curtain up.
Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3
Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
configuration panel from 200px to 80% viewport height. When expanded,
textareas switch from fixed rows to flex-fill layout, responsively
splitting available space among input/output prompts and per-edge
trigger prompts. Includes smooth height transition animation.
Claude ID: fc0c8654-1a8f-4a20-a734-fd60d0e73aa3
Maestro ID: b9bc0d08-5be2-4fdf-93cd-5618a8d53b35
launch terminal tabs on demand. Removes unnecessary createTerminalTab
calls and imports from worktreeSession.ts and useWizardHandlers.ts.
Claude ID: c57f388f-2e03-48f6-b055-bfb0510fd774
Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
session when it had no unread tabs and wasn't busy. Added activeSessionId
exemption so the user's current worktree always appears in the left bar.
- Fix DEFAULT_SHORTCUTS.maestroCue - openCue in CueHelpModal (runtime crash)
- Remove orphaned auto-scroll toggle JSX and state refs in TerminalOutput
- Remove orphaned CueYamlEditor import/state in CueModal (replaced by pipeline editor)
- Remove orphaned stats/WakaTime settings and effects in EncoreTab
- Remove duplicate filterUnreadAgents branch in useMainKeyboardHandler
- Add missing imports in DocumentGenerationView (MermaidRenderer, useClickOutside, etc.)
- Prefix unused readOnlyMode param in agent-spawner
- Remove unnecessary eslint-disable comment in MainPanel
- Update CueModal test: YAML editor assertion - stub no-throw check
- Update keyboard handler test: Cmd+T in terminal mode correctly blocked (AI mode only)
large prompts were embedded in bash -c CLI args via buildSshCommand(),
where ProcessManager couldn't detect --input-format stream-json flags
to enable stdin delivery. Switched to buildSshCommandWithStdin() for
large prompts, matching the proven pattern in process:spawn IPC handler.
Also added missing SSH wrapping to spawnModeratorSynthesis().
When switching from terminal back to AI mode (via tab click, Cmd+J, or any
other path), the input textarea is automatically focused so the user can
immediately start typing.
Claude ID: 3ce1f013-9673-40ec-ac0c-5cf1eaca0198
Maestro ID: 373c50f0-14a3-45b7-833f-ccc086211379
- Terminal tabs get proper per-tab process IDs via `getTerminalSessionId`
- Closing sessions now iterates all `terminalTabs` to terminate each PTY
- Deleting sessions also shuts down every terminal tab process cleanly
- Improved test coverage for multi-tab PTY termination on close
- More resilient error handling around terminal-tab process kills
- Sentry captures tab-kill failures with session and tab context
- Clearer lifecycle comments and intent: "kill all processes" upfront
shortcut. Now Cmd+0 goes to the last tab (consistent with Cmd+1-9 tab
navigation) and Cmd+Shift+0 resets font size. Font size reset is now
customizable in Settings - Shortcuts (moved from FIXED_SHORTCUTS to
DEFAULT_SHORTCUTS).
- Added `initialFileTreeReady` gate to session store initialization flow
- New splash progress stage highlights file indexing at 80%
- UI rendering stage bumped to 90% once file tree finishes loading
- File tree manager signals readiness exactly once on success or error
- Session restoration unblocks splash immediately when no sessions exist
- Session-load failures now mark file tree ready to prevent startup hangs
- App initialization reacts to settings, sessions, and file tree readiness
- Improved startup messaging: "Indexing the score..." during file discovery
- Updated tests to cover new three-gate splash behavior and progress
subtract the widths of the sticky left (search/filter) and right (+)
elements, causing the active tab's close button to be hidden behind
them when jumping to a tab or after auto-rename.
button that navigates to the agent (session) or specific tab and
closes the modal. Group chat processes get a similar button that
navigates to the group chat.
staged images. Applies to single tab close (Cmd+W, click X), and bulk
operations (close all, close other, close left, close right).
Security: filter dangerous flags (--no-sandbox, -y, --dangerously-skip-permissions,
etc.) from user-supplied custom args before passing to agent spawn. Logs a warning
per stripped arg for auditability. Includes 6 unit tests covering all scenarios.
Co-Authored-By: Claude Opus 4.6
- Add ;, backtick, $ to CMD_SPECIAL_CHARS detection regex
- Escape % by doubling to %% in escapeCmdArg (prevents env var expansion in cmd /c)
- Add canRunWithoutShell() helper: uses shell:false for resolved .exe paths, avoiding
all shell metacharacter risks; documents .cmd limitation for npm-installed CLIs
- Apply shell:false optimization in process.ts Windows spawn path
- Add 30+ new test cases covering each metacharacter individually for both cmd.exe
and PowerShell escaping, plus combined/edge-case scenarios
- Add tests for canRunWithoutShell (.exe, .cmd, .bat, basenames, extensionless)
Co-Authored-By: Claude Opus 4.6
b0cbd15 to
e3795d9
Compare
|
Retargeted from |
|
@openasocket please run the following on your machine for this PR so it can move toward acceptance. Here is the prompt you should run: You are responsible for getting this PR in RunMaestro/Maestro to an accept-ready state. |