feat: add cli.trigger event type and maestro-cli cue trigger command#786
Merged
pedramamini merged 12 commits intoRunMaestro:rcfrom Apr 12, 2026
Merged
feat: add cli.trigger event type and maestro-cli cue trigger command#786pedramamini merged 12 commits intoRunMaestro:rcfrom
pedramamini merged 12 commits intoRunMaestro:rcfrom
Conversation
Adds a new first-class Cue event type `cli.trigger` for subscriptions
designed to be invoked from the CLI, plus the `maestro-cli cue trigger`
command with optional `--prompt` override.
- New `cli.trigger` event type across contracts, validation, UI, and pipeline editor
- `maestro-cli cue trigger <name> [-p <prompt>] [--json]` command
- Prompt override flows through engine → dispatch → executor
- `{{CUE_CLI_PROMPT}}` template variable for cli.trigger event payloads
- WebSocket `trigger_cue_subscription` message type for CLI→desktop communication
- Renderer bridge for remote trigger via useRemoteIntegration hook
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a CLI "cue trigger" feature with optional prompt override and JSON output; wires a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Preload as Preload/IPC
participant WS as WebServer/WS
participant Engine as CueEngine
participant Dispatch as DispatchService
participant Executor as Executor
CLI->>Preload: triggerSubscription(name, prompt)
activate Preload
Preload->>WS: send WS message: trigger_cue_subscription(name, prompt)
activate WS
WS->>Engine: triggerSubscription(name, promptOverride)
activate Engine
Engine->>Dispatch: dispatchSubscription(..., promptOverride)
activate Dispatch
Dispatch->>Executor: executeRun(prompt: promptOverride ?? sub.prompt)
activate Executor
Executor-->>Dispatch: execution result
deactivate Executor
Dispatch-->>Engine: void
deactivate Dispatch
Engine-->>WS: boolean (success)
deactivate Engine
WS-->>Preload: trigger_cue_subscription_result(success)
deactivate WS
Preload-->>CLI: Promise<boolean>
deactivate Preload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 Confidence Score: 4/5Safe to merge after fixing the missing process.exit(1) in the --json failure path; all other changes are well-structured. One P1 correctness issue: the --json failure path exits 0 instead of 1, breaking exit-code-based scripting. Everything else — the full CLI→WebSocket→IPC→engine→executor chain, the new template variable, validator, and UI constants — is implemented correctly and consistently with existing patterns. src/cli/commands/cue-trigger.ts — exit code on --json failure path needs fixing. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as maestro-cli cue trigger
participant WS as WebSocket (WebServer)
participant MH as MessageHandler
participant Renderer as Renderer (useRemoteIntegration)
participant IPC as IPC cue:triggerSubscription
participant Engine as CueEngine
participant Dispatch as CueDispatchService
participant Executor as CueExecutor
CLI->>WS: trigger_cue_subscription {name, prompt}
WS->>MH: handleTriggerCueSubscription
MH->>Renderer: remote:triggerCueSubscription (IPC)
Renderer->>IPC: cue:triggerSubscription {name, prompt}
IPC->>Engine: triggerSubscription(name, promptOverride)
Engine->>Dispatch: dispatchSubscription(sessionId, sub, event, manual, promptOverride)
Dispatch->>Executor: executeRun(sessionId, prompt, event, name)
Executor-->>Engine: CueRunResult
Engine-->>IPC: true
IPC-->>Renderer: boolean
Renderer-->>WS: sendRemoteTriggerCueSubscriptionResponse
WS-->>CLI: trigger_cue_subscription_result {success}
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/cli/commands/cue-trigger.ts`:
- Around line 31-47: The JSON output branch currently doesn't exit non-zero on
failure, drops backend error details, and omits empty-string prompts; update the
JSON branch (where options.json is checked) to: include result.error in the
serialized object (e.g., "error": result.error), include the prompt when
options.prompt is defined (check for typeof options.prompt !== 'undefined'
rather than truthiness), and call process.exit(1) when result.success is false
so the CLI returns a non-zero code on failure; also ensure the non-JSON error
branch logs result.error along with the existing message referencing
subscriptionName and options.prompt handling remains consistent.
In `@src/main/preload/process.ts`:
- Around line 1089-1104: The onRemoteTriggerCueSubscription handler must always
send a reply on responseChannel even if the provided callback throws or rejects;
update the handler inside onRemoteTriggerCueSubscription to invoke the callback
and catch both synchronous exceptions and Promise rejections (e.g., use
Promise.resolve(callback(...)).then(...).catch(...)) and in both success and
error paths call ipcRenderer.send(responseChannel, { success: true, result }) or
ipcRenderer.send(responseChannel, { success: false, error: errorMessage });
ensure the handler variable name (handler), the callback parameter (callback),
and ipcRenderer.send with responseChannel are used so callers always receive a
response.
In `@src/main/web-server/handlers/messageHandlers.ts`:
- Around line 2324-2351: The handler handleTriggerCueSubscription must validate
incoming payloads and report exceptions: ensure subscriptionName is a non-empty
string and prompt is either undefined or a string (reject and call sendError if
not), then call callbacks.triggerCueSubscription inside a try/catch (or handle
Promise rejection) and on any failure call sendError with a safe message and
also call captureException/captureMessage from src/utils/sentry.ts with context
(include subscriptionName, prompt, and requestId) so failures are explicitly
reported; keep existing success/send response logic but do not swallow errors
silently.
In `@src/main/web-server/web-server-factory.ts`:
- Around line 1717-1751: The handleResponse inside triggerCueSubscription uses
clearTimeout(timeoutId) before timeoutId is declared, causing a TDZ
ReferenceError if the renderer replies quickly; move the creation of timeoutId
(the setTimeout(...) assigned to timeoutId) to before
ipcMain.once(responseChannel, handleResponse) or alternatively declare timeoutId
(let timeoutId: NodeJS.Timeout | null = null) above handleResponse and only call
clearTimeout(timeoutId!) after checking it's set, ensuring variables
responseChannel, handleResponse, timeoutId, mainWindow and ipcMain are
referenced in the same scope so early responses don't access an uninitialized
timeoutId.
In `@src/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsx`:
- Around line 261-271: In the CLI example inside the TriggerConfig.tsx case
'cli.trigger', wrap the interpolated subscription name in quotes so names with
spaces are handled; change the template from <code>maestro-cli cue trigger
{data.customLabel || data.label || 'name'}</code> to include quotes around the
expression (e.g. <code>maestro-cli cue trigger "{data.customLabel || data.label
|| 'name'}"</code>) while keeping the same fallback logic.
In `@src/renderer/hooks/remote/useRemoteIntegration.ts`:
- Around line 883-896: The catch block in the onRemoteTriggerCueSubscription
handler currently swallows errors; change it to catch the error object (e) and
report it using the Sentry utilities (imported captureException and/or
captureMessage from src/utils/sentry.ts) with contextual info (subscriptionName,
responseChannel, and prompt) before calling
window.maestro.process.sendRemoteTriggerCueSubscriptionResponse(responseChannel,
false); locate the async handler registered via
window.maestro.process.onRemoteTriggerCueSubscription and update its catch
clause to captureException(caughtError, { extra: { subscriptionName,
responseChannel, prompt } }) or a captureMessage call so failures are reported.
In `@src/shared/templateVariables.ts`:
- Around line 94-95: The CUE event-type docs for the template variable
CUE_EVENT_TYPE need to mention the new cli.trigger event: update the description
string for CUE_EVENT_TYPE in templateVariables (where CUE_CLI_PROMPT was added)
to include "cli.trigger" as a valid event type and optionally note that
CUE_CLI_PROMPT contains the prompt text passed via the --prompt flag for
cli.trigger events; edit the CUE_EVENT_TYPE doc/comment to list cli.trigger
alongside the other event types so the docs are consistent with the
CUE_CLI_PROMPT variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0be9cbd2-45c6-4ad8-a4eb-a29c4f05f42c
📒 Files selected for processing (26)
src/cli/commands/cue-trigger.tssrc/cli/index.tssrc/main/cue/config/cue-config-validator.tssrc/main/cue/cue-dispatch-service.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-executor.tssrc/main/ipc/handlers/cue.tssrc/main/preload/cue.tssrc/main/preload/process.tssrc/main/web-server/WebServer.tssrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/managers/CallbackRegistry.tssrc/main/web-server/types.tssrc/main/web-server/web-server-factory.tssrc/renderer/components/CuePipelineEditor/cueEventConstants.tssrc/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.tsxsrc/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsxsrc/renderer/components/CuePipelineEditor/utils/pipelineGraph.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tssrc/renderer/constants/cuePatterns.tssrc/renderer/constants/cueYamlDefaults.tssrc/renderer/global.d.tssrc/renderer/hooks/cue/usePipelineState.tssrc/renderer/hooks/remote/useRemoteIntegration.tssrc/shared/cue/contracts.tssrc/shared/templateVariables.ts
src/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsx
Show resolved
Hide resolved
- CLI: exit non-zero on JSON failure, include error field, use !== undefined for prompt check - Preload: wrap callback in try/catch to always send IPC response on error - WebServerFactory: fix TDZ race by declaring timeoutId with let before use - MessageHandlers: validate subscriptionName/prompt types, log errors via logger - TriggerConfig: quote subscription name in CLI example for names with spaces - useRemoteIntegration: log errors instead of silently swallowing in catch - templateVariables: add cli.trigger to CUE_EVENT_TYPE description Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/web-server/handlers/messageHandlers.ts (1)
2353-2357:⚠️ Potential issue | 🟠 MajorReport trigger failures to Sentry in addition to logging.
This catch path logs and responds to the client, but it does not emit explicit Sentry telemetry for production diagnostics.
Proposed patch
import { logger } from '../../utils/logger'; +import { captureException } from '../../utils/sentry'; @@ .catch((error) => { const err = error instanceof Error ? error : new Error(String(error)); + captureException(err, { + extra: { + operation: 'web.triggerCueSubscription', + subscriptionName, + prompt: prompt as string | undefined, + requestId: message.requestId, + }, + }); logger.error(`Failed to trigger Cue subscription: ${err.message}`, 'WebSocket'); this.sendError(client, `Failed to trigger Cue subscription: ${err.message}`); });As per coding guidelines: "Use Sentry utilities (
captureException,captureMessage) fromsrc/utils/sentry.tsfor explicit error reporting with context."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-server/handlers/messageHandlers.ts` around lines 2353 - 2357, The catch block that currently logs and responds to the client (logger.error(...) and this.sendError(...)) should also report the failure to Sentry by calling the sentry utilities (import captureException and/or captureMessage from src/utils/sentry.ts) with the Error object and relevant context (e.g., tag "WebSocket" and any subscription/client identifiers). Update the file to import captureException (and captureMessage if desired), then inside the .catch((error) => { ... }) invoke captureException(err, { tags: { source: 'WebSocket', action: 'triggerCueSubscription' }, extra: { clientId: <if available> } }) before or after logging and sending the client error so the failure is recorded in Sentry as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/web-server/handlers/messageHandlers.ts`:
- Around line 2353-2357: The catch block that currently logs and responds to the
client (logger.error(...) and this.sendError(...)) should also report the
failure to Sentry by calling the sentry utilities (import captureException
and/or captureMessage from src/utils/sentry.ts) with the Error object and
relevant context (e.g., tag "WebSocket" and any subscription/client
identifiers). Update the file to import captureException (and captureMessage if
desired), then inside the .catch((error) => { ... }) invoke
captureException(err, { tags: { source: 'WebSocket', action:
'triggerCueSubscription' }, extra: { clientId: <if available> } }) before or
after logging and sending the client error so the failure is recorded in Sentry
as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ca5075c-f2a4-4f7a-adb3-12a1daf7f612
📒 Files selected for processing (7)
src/cli/commands/cue-trigger.tssrc/main/preload/process.tssrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/web-server-factory.tssrc/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsxsrc/renderer/hooks/remote/useRemoteIntegration.tssrc/shared/templateVariables.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/renderer/hooks/remote/useRemoteIntegration.ts
- src/renderer/components/CuePipelineEditor/panels/triggers/TriggerConfig.tsx
- src/shared/templateVariables.ts
- src/cli/commands/cue-trigger.ts
- src/main/preload/process.ts
- Accept upstream changes for contextUsage, WebServer, InputArea, SessionList - Remove AutoRun.tsx and MainPanel.tsx (refactored into directories on this branch) - Port upstream rename "Exchange" → "PlayBooks" into AutoRunToolbar.tsx - Port upstream z-index removal into MainPanelHeader.tsx Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/tunnel-manager.ts (1)
47-48:⚠️ Potential issue | 🟠 MajorGuard
exitstate updates to the currently active child process.At Line 97-Line 104, the
exitcallback updates shared state without verifying that the exiting process is still the active one. A late exit from a previouscloudflaredprocess can null outthis.processand setthis.errorwhile a newer tunnel is already running.Proposed fix
- this.process = spawn(cloudflaredBinary, ['tunnel', '--url', `http://localhost:${port}`]); + const spawnedProcess = spawn(cloudflaredBinary, ['tunnel', '--url', `http://localhost:${port}`]); + this.process = spawnedProcess; @@ - this.process.stderr?.on('data', (data: Buffer) => { + spawnedProcess.stderr?.on('data', (data: Buffer) => { @@ - this.process.on('error', (err) => { + spawnedProcess.on('error', (err) => { @@ - this.process.on('exit', (code) => { + spawnedProcess.on('exit', (code) => { + if (this.process !== spawnedProcess) { + return; + } logger.info(`cloudflared exited with code ${code}`, 'TunnelManager'); if (!resolved) {Also applies to: 90-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/tunnel-manager.ts` around lines 47 - 48, The exit handler updates shared state unconditionally which allows a stale child process to clear this.process/this.error; fix by capturing the spawned child in a local const (e.g., const child = spawn(...)) and in the 'exit'/'close' callback verify that this.process === child (or compare child.pid) before mutating this.process or this.error (apply same guard in any other exit/error handlers referenced around startTunnel/spawn and the existing event listeners).src/renderer/components/InputArea.tsx (1)
504-532:⚠️ Potential issue | 🔴 CriticalRemove unused model and effort props passed to InputArea.
MainPanelContent.tsx passes
currentModel,currentEffort,availableModels,availableEfforts,onModelChange, andonEffortChangeto InputArea (lines 650–655), but these props are not defined in InputAreaProps. Either add them to InputAreaProps if they're needed, or remove the unused prop assignments from MainPanelContent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/InputArea.tsx` around lines 504 - 532, MainPanelContent is passing props currentModel, currentEffort, availableModels, availableEfforts, onModelChange, and onEffortChange to InputArea but InputAreaProps doesn't declare them; either remove those unused prop assignments from MainPanelContent or add/declare them in InputAreaProps and the InputArea component (prefer making them optional in InputAreaProps and wiring them into the component UI if you actually need model/effort controls). Locate the InputAreaProps type and the InputArea function/component and either (A) delete the six prop assignments from the JSX caller in MainPanelContent, or (B) add optional props currentModel, currentEffort, availableModels, availableEfforts, onModelChange, onEffortChange to InputAreaProps and update InputArea to consume/use them (or explicitly ignore with underscore names) to eliminate the unused-prop mismatch.src/renderer/components/SessionList/SessionList.tsx (2)
694-716:⚠️ Potential issue | 🔴 Critical
LiveOverlayPanelstill requiresrestartTunnelprop but it's not being passed.The
restartTunnelprop remains inLiveOverlayPanelPropsinterface as a required field (line 31 of LiveOverlayPanel.tsx) and is actively used within the component (lines 78-79). However, it is not being provided when instantiatingLiveOverlayPanelin SessionList.tsx. Either pass therestartTunnelhandler fromuseLiveOverlay, or remove it from the component's interface and implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 694 - 716, LiveOverlayPanel is missing the required restartTunnel prop declared in LiveOverlayPanelProps and used inside the component; fix by passing the restartTunnel handler from the parent (grab the restartTunnel returned from useLiveOverlay in SessionList and add restartTunnel={restartTunnel} to the LiveOverlayPanel props) or alternatively remove restartTunnel from LiveOverlayPanelProps and its internal usage (lines referencing restartTunnel inside LiveOverlayPanel) if it’s no longer needed; update either SessionList.tsx to provide restartTunnel or LiveOverlayPanel.tsx to drop the prop and associated logic.
43-100:⚠️ Potential issue | 🟠 MajorRemove unused props from
useSessionListPropsreturn object.The hook
useSessionListPropsreturns three props that are not declared in theSessionListPropsinterface and are not used anywhere in theSessionListcomponent:
navIndexMaponConfigureCueopenFeedbackUpdate
src/renderer/hooks/props/useSessionListProps.tsto remove these three properties from the return object and update the dependency array accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 43 - 100, The hook useSessionListProps is returning unused properties (navIndexMap, onConfigureCue, openFeedback) that are not declared in SessionListProps and not used in the SessionList component; open src/renderer/hooks/props/useSessionListProps.ts and remove navIndexMap, onConfigureCue and openFeedback from the return object and from any internal exports, then update the hook's dependency array and any destructuring callers to stop expecting those keys so only the actual props remain (ensure references to useSessionListProps, navIndexMap, onConfigureCue, openFeedback, and SessionListProps are cleaned up).
🧹 Nitpick comments (2)
src/__tests__/renderer/utils/contextUsage.test.ts (1)
414-427: Consider adding a non-finite fallback regression test.Since runtime now checks
Number.isFinite, add a case withfallbackPercentage: NaN(orInfinity) to explicitly pin the invalid-fallback branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/utils/contextUsage.test.ts` around lines 414 - 427, Add a regression test to exercise the non-finite fallback branch in calculateContextDisplay by calling it with a fallbackPercentage of NaN (or Infinity) and asserting the behavior (e.g., that tokens are clamped and percentage becomes 100 or other expected outcome); locate the test near the existing "should clamp fallback percentages above 100 before deriving tokens" case in src/__tests__/renderer/utils/contextUsage.test.ts and add a similar it() that passes fallbackPercentage: NaN (and/or Infinity) to calculateContextDisplay and asserts the expected tokens and percentage to pin the Number.isFinite check.src/renderer/components/SessionList/SessionList.tsx (1)
217-225:minWidthreduced from 280 to 256.The resizable panel's minimum width has been reduced. Ensure this doesn't cause layout issues with the sidebar content at the new minimum width, particularly the LIVE/OFFLINE button text which has width thresholds at 256px and 295px (line 687-689).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 217 - 225, The minimum width for the resizable left sidebar was lowered to 256 in the useResizablePanel call (props: width leftSidebarWidthState, minWidth 256, maxWidth 600, setWidth setLeftSidebarWidthState, externalRef sidebarContainerRef), which may break the LIVE/OFFLINE button layout that has text width thresholds at 256px and 295px; either raise minWidth back to 280 or adjust the button rendering logic to handle 256px (update the LIVE/OFFLINE button width checks/conditional rendering to account for the new 256px breakpoint or ensure text truncation/spacing works at that width) so the sidebar UI remains stable at the new minimum.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup-git-hooks.mjs`:
- Around line 4-10: The runGit function currently throws on any spawnSync error
which makes npm prepare fail when git is not installed; change runGit to treat a
missing git executable (result.error with code 'ENOENT') as a recoverable case
by returning null or an empty string (and not throwing), and only throw for
other unexpected errors; also update the caller (prepare) to check the runGit
return value and log a non-fatal warning when git is unavailable instead of
aborting hook setup so the script continues when .git exists but git is not
installed. Ensure you only alter runGit and the prepare invocation handling,
keep other error behavior unchanged, and reference runGit and prepare in your
changes.
In `@src/main/web-server/WebServer.ts`:
- Around line 478-479: The selectSession wrapper in WebServer.ts currently drops
the optional focus flag; update the function signature and call to forward the
third optional boolean parameter to CallbackRegistry.selectSession so the focus
signal is preserved (i.e., change selectSession: async (sessionId: string,
tabId?: string) => this.callbackRegistry.selectSession(sessionId, tabId) to
accept and pass focus?: boolean through to CallbackRegistry.selectSession).
Ensure the optional focus parameter name matches
CallbackRegistry.selectSession(focus?: boolean) so callers sending focus: false
are honored.
- Around line 195-205: The isServableWebAssetsPath function currently only
checks for index.html and dev entrypoint references; update it to also verify
the presence of the bundled assets directory (e.g., path.join(candidatePath,
'assets')) before returning true. Specifically, in isServableWebAssetsPath add
an existsSync check for the assets directory and return false if the assets
folder is missing so that setupMiddleware's conditional assets mount and the UI
bootstrap remain consistent.
---
Outside diff comments:
In `@src/main/tunnel-manager.ts`:
- Around line 47-48: The exit handler updates shared state unconditionally which
allows a stale child process to clear this.process/this.error; fix by capturing
the spawned child in a local const (e.g., const child = spawn(...)) and in the
'exit'/'close' callback verify that this.process === child (or compare
child.pid) before mutating this.process or this.error (apply same guard in any
other exit/error handlers referenced around startTunnel/spawn and the existing
event listeners).
In `@src/renderer/components/InputArea.tsx`:
- Around line 504-532: MainPanelContent is passing props currentModel,
currentEffort, availableModels, availableEfforts, onModelChange, and
onEffortChange to InputArea but InputAreaProps doesn't declare them; either
remove those unused prop assignments from MainPanelContent or add/declare them
in InputAreaProps and the InputArea component (prefer making them optional in
InputAreaProps and wiring them into the component UI if you actually need
model/effort controls). Locate the InputAreaProps type and the InputArea
function/component and either (A) delete the six prop assignments from the JSX
caller in MainPanelContent, or (B) add optional props currentModel,
currentEffort, availableModels, availableEfforts, onModelChange, onEffortChange
to InputAreaProps and update InputArea to consume/use them (or explicitly ignore
with underscore names) to eliminate the unused-prop mismatch.
In `@src/renderer/components/SessionList/SessionList.tsx`:
- Around line 694-716: LiveOverlayPanel is missing the required restartTunnel
prop declared in LiveOverlayPanelProps and used inside the component; fix by
passing the restartTunnel handler from the parent (grab the restartTunnel
returned from useLiveOverlay in SessionList and add
restartTunnel={restartTunnel} to the LiveOverlayPanel props) or alternatively
remove restartTunnel from LiveOverlayPanelProps and its internal usage (lines
referencing restartTunnel inside LiveOverlayPanel) if it’s no longer needed;
update either SessionList.tsx to provide restartTunnel or LiveOverlayPanel.tsx
to drop the prop and associated logic.
- Around line 43-100: The hook useSessionListProps is returning unused
properties (navIndexMap, onConfigureCue, openFeedback) that are not declared in
SessionListProps and not used in the SessionList component; open
src/renderer/hooks/props/useSessionListProps.ts and remove navIndexMap,
onConfigureCue and openFeedback from the return object and from any internal
exports, then update the hook's dependency array and any destructuring callers
to stop expecting those keys so only the actual props remain (ensure references
to useSessionListProps, navIndexMap, onConfigureCue, openFeedback, and
SessionListProps are cleaned up).
---
Nitpick comments:
In `@src/__tests__/renderer/utils/contextUsage.test.ts`:
- Around line 414-427: Add a regression test to exercise the non-finite fallback
branch in calculateContextDisplay by calling it with a fallbackPercentage of NaN
(or Infinity) and asserting the behavior (e.g., that tokens are clamped and
percentage becomes 100 or other expected outcome); locate the test near the
existing "should clamp fallback percentages above 100 before deriving tokens"
case in src/__tests__/renderer/utils/contextUsage.test.ts and add a similar it()
that passes fallbackPercentage: NaN (and/or Infinity) to calculateContextDisplay
and asserts the expected tokens and percentage to pin the Number.isFinite check.
In `@src/renderer/components/SessionList/SessionList.tsx`:
- Around line 217-225: The minimum width for the resizable left sidebar was
lowered to 256 in the useResizablePanel call (props: width
leftSidebarWidthState, minWidth 256, maxWidth 600, setWidth
setLeftSidebarWidthState, externalRef sidebarContainerRef), which may break the
LIVE/OFFLINE button layout that has text width thresholds at 256px and 295px;
either raise minWidth back to 280 or adjust the button rendering logic to handle
256px (update the LIVE/OFFLINE button width checks/conditional rendering to
account for the new 256px breakpoint or ensure text truncation/spacing works at
that width) so the sidebar UI remains stable at the new minimum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 597b1ec8-9721-488a-8d49-715186c2b4d1
📒 Files selected for processing (33)
.gitignore.prettierignoreREADME.mdpackage.jsonscripts/setup-git-hooks.mjssrc/__tests__/main/stats/integration.test.tssrc/__tests__/main/tunnel-manager.test.tssrc/__tests__/main/web-server/WebServer.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser.test.tsxsrc/__tests__/renderer/components/GroupChatModals.test.tsxsrc/__tests__/renderer/components/InputArea.test.tsxsrc/__tests__/renderer/components/SessionList.test.tsxsrc/__tests__/renderer/components/SessionList/LiveOverlayPanel.test.tsxsrc/__tests__/renderer/components/Settings/tabs/EncoreTab.test.tsxsrc/__tests__/renderer/utils/contextUsage.test.tssrc/__tests__/shared/agentMetadata.test.tssrc/main/tunnel-manager.tssrc/main/web-server/WebServer.tssrc/renderer/components/AgentSessionsBrowser.tsxsrc/renderer/components/AutoRun/AutoRunExpandedModal.tsxsrc/renderer/components/AutoRun/AutoRunToolbar.tsxsrc/renderer/components/InputArea.tsxsrc/renderer/components/MainPanel/MainPanelHeader.tsxsrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/hooks/remote/useLiveOverlay.tssrc/renderer/hooks/session/useSessionCrud.tssrc/renderer/hooks/symphony/useSymphonyContribution.tssrc/renderer/hooks/wizard/useWizardHandlers.tssrc/renderer/types/index.tssrc/renderer/utils/contextUsage.tssrc/renderer/utils/tabHelpers.tssrc/renderer/utils/worktreeSession.tssrc/shared/agentMetadata.ts
✅ Files skipped from review due to trivial changes (6)
- .gitignore
- .prettierignore
- README.md
- src/renderer/components/AutoRun/AutoRunToolbar.tsx
- src/renderer/components/AutoRun/AutoRunExpandedModal.tsx
- src/tests/renderer/components/Settings/tabs/EncoreTab.test.tsx
- Handle missing git gracefully in setup-git-hooks.mjs (non-fatal exit) - Reject incomplete web bundles missing assets/ directory - Forward optional focus flag in WebServer selectSession wrapper - Update WebServer test to create assets/ directory fixture Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/web-server/WebServer.ts (2)
501-559:⚠️ Potential issue | 🔴 CriticalKeep the existing
WebServerbroadcast API until callers are migrated.
src/main/web-server/web-server-factory.ts:857-871still callsserver.broadcastSettingsChanged(settings), andsrc/main/process-listeners/forwarding-listeners.ts:40-50still callswebServer.broadcastToolEvent(...). Removing those public wrappers here turns this into a compile break unless the call sites are updated in the same changeset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-server/WebServer.ts` around lines 501 - 559, The WebServer class removed public wrapper APIs still relied on elsewhere; restore the public methods with the original signatures so existing call sites compile: add back broadcastSettingsChanged(settings: SettingsType) that delegates to this.broadcastService.broadcastSettingsChanged(settings) (or appropriate service), and add back broadcastToolEvent(event: ToolEventType) that delegates to this.broadcastService.broadcastToolEvent(event) (or the correct target), ensuring the method names and parameter types match the callers (references: WebServer class, broadcastSettingsChanged, broadcastToolEvent) so callers in web-server-factory.ts and forwarding-listeners.ts continue to compile until they are migrated.
474-479:⚠️ Potential issue | 🔴 CriticalRegister the new cue-trigger WebSocket handler here.
setupMessageHandlerCallbacks()still only forwards the existing session/tab actions. Withmaestro-cli cue triggergoing through the WebSocket path in this PR, the request dead-ends here instead of reachingCallbackRegistry/engine.Also applies to: 482-498
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-server/WebServer.ts` around lines 474 - 479, The WebSocket callbacks registered in setupMessageHandlerCallbacks currently only forward session/tab actions and are missing the new cue-trigger handler, so add a callback entry in this.messageHandler.setCallbacks that maps the incoming cue trigger message (e.g., "cueTrigger" or the exact message key used by the client) to a forwarder that calls the CallbackRegistry method (e.g., this.callbackRegistry.triggerCue or this.callbackRegistry.handleCueTrigger) with the same parameters (sessionId, cue identifier, any payload/options) and make it async/return the registry result; also apply the same addition to the other callback registration block around the 482-498 region so all WebSocket paths route cue-trigger requests to CallbackRegistry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/main/web-server/WebServer.ts`:
- Around line 501-559: The WebServer class removed public wrapper APIs still
relied on elsewhere; restore the public methods with the original signatures so
existing call sites compile: add back broadcastSettingsChanged(settings:
SettingsType) that delegates to
this.broadcastService.broadcastSettingsChanged(settings) (or appropriate
service), and add back broadcastToolEvent(event: ToolEventType) that delegates
to this.broadcastService.broadcastToolEvent(event) (or the correct target),
ensuring the method names and parameter types match the callers (references:
WebServer class, broadcastSettingsChanged, broadcastToolEvent) so callers in
web-server-factory.ts and forwarding-listeners.ts continue to compile until they
are migrated.
- Around line 474-479: The WebSocket callbacks registered in
setupMessageHandlerCallbacks currently only forward session/tab actions and are
missing the new cue-trigger handler, so add a callback entry in
this.messageHandler.setCallbacks that maps the incoming cue trigger message
(e.g., "cueTrigger" or the exact message key used by the client) to a forwarder
that calls the CallbackRegistry method (e.g., this.callbackRegistry.triggerCue
or this.callbackRegistry.handleCueTrigger) with the same parameters (sessionId,
cue identifier, any payload/options) and make it async/return the registry
result; also apply the same addition to the other callback registration block
around the 482-498 region so all WebSocket paths route cue-trigger requests to
CallbackRegistry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd4f73ff-e7e5-49e0-bba4-c837158c0f17
📒 Files selected for processing (3)
scripts/setup-git-hooks.mjssrc/__tests__/main/web-server/WebServer.test.tssrc/main/web-server/WebServer.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/main/web-server/WebServer.test.ts
- scripts/setup-git-hooks.mjs
- Add highlightSlashCommand import to InputArea.tsx (lost during merge) - Add cli.trigger case to trigger source registry exhaustive switch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/renderer/components/SessionList/SessionList.tsx (2)
1184-1192:⚠️ Potential issue | 🔴 CriticalFix
SidebarActionsprop API drift at the call site.Line 1190 passes
openWizard, butsrc/renderer/components/SessionList/SidebarActions.tsxprops currently exposeopenFeedbackand unread-related props instead. This mismatch will fail type-checking unlessSidebarActionswas updated in lockstep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 1184 - 1192, The call site in SessionList.tsx passes a prop named openWizard to the SidebarActions component, but SidebarActions.tsx expects openFeedback and unread-related props; update the call site to match SidebarActions' API by replacing openWizard with the correct prop(s) (e.g., openFeedback) and include any required unread props (or alternately update SidebarActions to accept openWizard if that was intended); locate the usage of SidebarActions in SessionList.tsx and align the prop names with the SidebarActions component's prop definitions to restore type-safety.
231-245:⚠️ Potential issue | 🔴 CriticalRestore
restartTunnelcontract betweenuseLiveOverlayandLiveOverlayPanel.Line 231-245 drops
restartTunnelfrom the hook result, and Line 694-716 no longer passes it toLiveOverlayPanel. Insrc/renderer/components/SessionList/LiveOverlayPanel.tsx(Line 10-32),restartTunnelis still required, so this is a type/API break and removes restart capability from the panel flow.Also applies to: 694-716
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 231 - 245, The hook useLiveOverlay removed restartTunnel from its returned object, breaking the LiveOverlayPanel API which still expects restartTunnel; restore restartTunnel in useLiveOverlay's return value and ensure the caller in SessionList passes restartTunnel through to LiveOverlayPanel (re-add restartTunnel to the destructuring at the top of SessionList and to the props passed into LiveOverlayPanel), keeping the symbol name restartTunnel consistent between useLiveOverlay and LiveOverlayPanel.src/main/web-server/WebServer.ts (2)
278-346:⚠️ Potential issue | 🔴 CriticalRe-add the cue-trigger bridge through
WebServer.This PR adds
trigger_cue_subscription, andCallbackRegistryalready exposestriggerCueSubscription, butWebServerno longer provides a setter for it andsetupMessageHandlerCallbacks()never forwards it toWebSocketMessageHandler. As written, the new CLI/WebSocket trigger flow has no path from the socket layer into the cue dispatcher/executor.Also applies to: 473-499
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-server/WebServer.ts` around lines 278 - 346, Add a delegated setter on WebServer for the cue-trigger bridge and wire it into the socket message handler: implement setTriggerCueSubscription(callback: TriggerCueSubscriptionCallback) to call this.callbackRegistry.setTriggerCueSubscription(callback), and update setupMessageHandlerCallbacks() to forward the registry's triggerCueSubscription to the WebSocketMessageHandler (e.g., call messageHandler.setTriggerCueSubscription(...) or pass the registry callback when registering handlers) so socket messages can reach CallbackRegistry.triggerCueSubscription.
278-346:⚠️ Potential issue | 🔴 CriticalRestore the terminal callback setters or migrate the factory in the same PR.
src/main/web-server/web-server-factory.ts:278-350still callssetWriteToTerminalCallback,setResizeTerminalCallback,setSpawnTerminalForWebCallback, andsetKillTerminalForWebCallback. After removing those methods fromWebServer, this class no longer matches its current caller and the web terminal path breaks at compile time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-server/WebServer.ts` around lines 278 - 346, The WebServer class no longer exposes terminal-related setters but web-server-factory.ts still calls setWriteToTerminalCallback, setResizeTerminalCallback, setSpawnTerminalForWebCallback, and setKillTerminalForWebCallback; restore compatibility by either re-adding those four methods to WebServer (implementations should delegate to this.callbackRegistry the same way other setters do) or update the factory to call the new session/terminal APIs (e.g., setWriteToSessionCallback / setExecuteCommandCallback or the appropriate new callback methods) so the caller and WebServer's public API match.
🧹 Nitpick comments (1)
src/renderer/components/SessionList/SessionList.tsx (1)
1018-1032: Extract a sharedNewGroupButtoncomponent to remove duplicated JSX.The same button markup/style appears in three branches, which increases maintenance risk for future style/behavior edits.
Also applies to: 1055-1070, 1122-1134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 1018 - 1032, Extract the repeated button JSX into a new presentational component (e.g., NewGroupButton) and replace the three duplicated blocks with <NewGroupButton onClick={createNewGroup} />; the component should accept props for onClick and optionally className/title, import and render the <Plus /> icon and use the shared theme (theme.colors.accent) for the inline styles (backgroundColor, color, border) and the same className and children ("New Group"); update usages in the branches where createNewGroup is currently passed so behavior remains identical.
🤖 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/renderer/components/InputArea.tsx`:
- Around line 323-330: The filter couples command matching to the literal input
(including a leading '/'), causing commands whose stored form omits the slash to
be excluded; update the filtering in the function that uses slashCommands,
isTerminalMode and inputValueLower to normalize both sides before
comparison—strip a single leading '/' from inputValueLower (or derive a new
normalizedInput) and also normalize cmd.command (strip leading '/' if present),
then use startsWith on those normalized strings so highlighting (which strips
'/') and filtering remain consistent.
---
Outside diff comments:
In `@src/main/web-server/WebServer.ts`:
- Around line 278-346: Add a delegated setter on WebServer for the cue-trigger
bridge and wire it into the socket message handler: implement
setTriggerCueSubscription(callback: TriggerCueSubscriptionCallback) to call
this.callbackRegistry.setTriggerCueSubscription(callback), and update
setupMessageHandlerCallbacks() to forward the registry's triggerCueSubscription
to the WebSocketMessageHandler (e.g., call
messageHandler.setTriggerCueSubscription(...) or pass the registry callback when
registering handlers) so socket messages can reach
CallbackRegistry.triggerCueSubscription.
- Around line 278-346: The WebServer class no longer exposes terminal-related
setters but web-server-factory.ts still calls setWriteToTerminalCallback,
setResizeTerminalCallback, setSpawnTerminalForWebCallback, and
setKillTerminalForWebCallback; restore compatibility by either re-adding those
four methods to WebServer (implementations should delegate to
this.callbackRegistry the same way other setters do) or update the factory to
call the new session/terminal APIs (e.g., setWriteToSessionCallback /
setExecuteCommandCallback or the appropriate new callback methods) so the caller
and WebServer's public API match.
In `@src/renderer/components/SessionList/SessionList.tsx`:
- Around line 1184-1192: The call site in SessionList.tsx passes a prop named
openWizard to the SidebarActions component, but SidebarActions.tsx expects
openFeedback and unread-related props; update the call site to match
SidebarActions' API by replacing openWizard with the correct prop(s) (e.g.,
openFeedback) and include any required unread props (or alternately update
SidebarActions to accept openWizard if that was intended); locate the usage of
SidebarActions in SessionList.tsx and align the prop names with the
SidebarActions component's prop definitions to restore type-safety.
- Around line 231-245: The hook useLiveOverlay removed restartTunnel from its
returned object, breaking the LiveOverlayPanel API which still expects
restartTunnel; restore restartTunnel in useLiveOverlay's return value and ensure
the caller in SessionList passes restartTunnel through to LiveOverlayPanel
(re-add restartTunnel to the destructuring at the top of SessionList and to the
props passed into LiveOverlayPanel), keeping the symbol name restartTunnel
consistent between useLiveOverlay and LiveOverlayPanel.
---
Nitpick comments:
In `@src/renderer/components/SessionList/SessionList.tsx`:
- Around line 1018-1032: Extract the repeated button JSX into a new
presentational component (e.g., NewGroupButton) and replace the three duplicated
blocks with <NewGroupButton onClick={createNewGroup} />; the component should
accept props for onClick and optionally className/title, import and render the
<Plus /> icon and use the shared theme (theme.colors.accent) for the inline
styles (backgroundColor, color, border) and the same className and children
("New Group"); update usages in the branches where createNewGroup is currently
passed so behavior remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a15a374-e9e8-45bf-898f-992bd6949e1a
📒 Files selected for processing (12)
src/main/cue/cue-dispatch-service.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-template-context-builder.tssrc/main/cue/triggers/cue-trigger-source-registry.tssrc/main/ipc/handlers/cue.tssrc/main/web-server/WebServer.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tssrc/renderer/components/InputArea.tsxsrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/global.d.tssrc/renderer/hooks/cue/usePipelineState.tssrc/shared/cue/contracts.ts
✅ Files skipped from review due to trivial changes (2)
- src/renderer/hooks/cue/usePipelineState.ts
- src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/ipc/handlers/cue.ts
- src/shared/cue/contracts.ts
- src/main/cue/cue-dispatch-service.ts
- src/renderer/global.d.ts
- src/main/cue/cue-engine.ts
InputArea.tsx, SessionList.tsx, and MainPanelContent.tsx had code lost during merge resolution. These files have no cli.trigger feature changes, so restoring upstream/rc versions is the correct fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous merge resolution dropped many methods from WebServer.ts. Restoring upstream/rc version and re-applying only the assets directory check from the CodeRabbit review fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The CLI trigger feature requires this callback delegation method on WebServer for the web-server-factory to wire up cue trigger handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test expected 8 event types but cli.trigger was added, making it 9. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add onRemoteTriggerCueSubscription, sendRemoteTriggerCueSubscriptionResponse, and cue.triggerSubscription mocks to support the cli.trigger feature's remote integration hooks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/renderer/hooks/useRemoteIntegration.test.ts (1)
191-194: Capture the cue-trigger callback in the mock for testability.This registration mock currently ignores the handler argument, so this suite can’t directly invoke/assert the new remote cue-trigger path like it does for other remote handlers.
Proposed refactor
+let onRemoteTriggerCueSubscriptionHandler: ((...args: unknown[]) => void) | undefined; + onRemoteTriggerCueSubscription: vi.fn().mockImplementation((handler) => { + onRemoteTriggerCueSubscriptionHandler = handler; return () => {}; }),beforeEach(() => { vi.clearAllMocks(); + onRemoteTriggerCueSubscriptionHandler = undefined; // ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/hooks/useRemoteIntegration.test.ts` around lines 191 - 194, The mock for onRemoteTriggerCueSubscription ignores the handler argument which prevents tests from invoking the registered callback; update the mock implementation of onRemoteTriggerCueSubscription to capture and store the passed handler (e.g., in a local variable or test-scoped reference) and return an unsubscribe function as currently done, so tests can call the stored handler and assert behavior (also ensure sendRemoteTriggerCueSubscriptionResponse remains available for assertions). Use the existing mock name onRemoteTriggerCueSubscription and the response mock sendRemoteTriggerCueSubscriptionResponse to wire up invocation and verification.
🤖 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/main/web-server/WebServer.ts`:
- Around line 543-545: The new setter setTriggerCueSubscriptionCallback
registers a TriggerCueSubscriptionCallback in callbackRegistry but
setupMessageHandlerCallbacks never wires a 'trigger_cue_subscription' WebSocket
message to it, so incoming trigger_cue_subscription messages are ignored; update
setupMessageHandlerCallbacks to register a handler for the
'trigger_cue_subscription' message that retrieves and calls the callback from
this.callbackRegistry (use the same callback type installed by
setTriggerCueSubscriptionCallback) and ensure proper args and error handling
when invoking the callback so the CLI trigger flow executes.
---
Nitpick comments:
In `@src/__tests__/renderer/hooks/useRemoteIntegration.test.ts`:
- Around line 191-194: The mock for onRemoteTriggerCueSubscription ignores the
handler argument which prevents tests from invoking the registered callback;
update the mock implementation of onRemoteTriggerCueSubscription to capture and
store the passed handler (e.g., in a local variable or test-scoped reference)
and return an unsubscribe function as currently done, so tests can call the
stored handler and assert behavior (also ensure
sendRemoteTriggerCueSubscriptionResponse remains available for assertions). Use
the existing mock name onRemoteTriggerCueSubscription and the response mock
sendRemoteTriggerCueSubscriptionResponse to wire up invocation and verification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7720fb6e-c897-4d5b-bda8-ed1f64bfe547
📒 Files selected for processing (12)
src/__tests__/renderer/hooks/cue/usePipelineState.test.tssrc/__tests__/renderer/hooks/useRemoteIntegration.test.tssrc/main/cue/cue-dispatch-service.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-template-context-builder.tssrc/main/cue/triggers/cue-trigger-source-registry.tssrc/main/ipc/handlers/cue.tssrc/main/web-server/WebServer.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tssrc/renderer/global.d.tssrc/renderer/hooks/cue/usePipelineState.tssrc/shared/cue/contracts.ts
✅ Files skipped from review due to trivial changes (4)
- src/main/cue/cue-template-context-builder.ts
- src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts
- src/tests/renderer/hooks/cue/usePipelineState.test.ts
- src/renderer/hooks/cue/usePipelineState.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/shared/cue/contracts.ts
- src/main/cue/triggers/cue-trigger-source-registry.ts
- src/main/cue/cue-engine.ts
- src/main/cue/cue-dispatch-service.ts
- Add setTriggerCueSubscriptionCallback mock to web-server-factory test - Update TriggerDrawer test to expect 8 trigger types (includes cli.trigger) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsx (1)
116-124: Make this test assertCLI Triggerexplicitly, not only draggable count.Counting
[draggable="true"]is a bit brittle and can pass even if the new trigger is missing but some other draggable node appears. Assert expected trigger labels (or at leastCLI Trigger) in addition to count.Proposed test hardening
it('should render exactly 8 trigger types (no agent.completed)', () => { - const { container } = render( - <TriggerDrawer isOpen={true} onClose={() => {}} theme={mockTheme} /> - ); - - // Each trigger item is a draggable div; count them - const draggableItems = container.querySelectorAll('[draggable="true"]'); - expect(draggableItems.length).toBe(8); + render(<TriggerDrawer isOpen={true} onClose={() => {}} theme={mockTheme} />); + + const expectedTriggers = [ + 'Startup', + 'Heartbeat', + 'Scheduled', + 'File Change', + 'Pull Request', + 'Issue', + 'Pending Task', + 'CLI Trigger', + ]; + expectedTriggers.forEach((label) => { + expect(screen.getByText(label)).toBeInTheDocument(); + }); + expect(screen.queryByText('Agent Done')).not.toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsx` around lines 116 - 124, The test in TriggerDrawer.test.tsx currently only counts elements with draggable="true", which is brittle; update the test for TriggerDrawer to also assert that the expected trigger label(s) exist (at minimum assert presence of the "CLI Trigger" text) in addition to asserting draggableItems.length === 8 — locate the test that queries container.querySelectorAll('[draggable="true"]') and add an assertion using the rendered output to verify getByText('CLI Trigger') (or equivalent text query) is present so the specific trigger is guaranteed to be rendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsx`:
- Around line 116-124: The test in TriggerDrawer.test.tsx currently only counts
elements with draggable="true", which is brittle; update the test for
TriggerDrawer to also assert that the expected trigger label(s) exist (at
minimum assert presence of the "CLI Trigger" text) in addition to asserting
draggableItems.length === 8 — locate the test that queries
container.querySelectorAll('[draggable="true"]') and add an assertion using the
rendered output to verify getByText('CLI Trigger') (or equivalent text query) is
present so the specific trigger is guaranteed to be rendered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5cd347d2-2494-4eae-9c9b-2d966468f2af
📒 Files selected for processing (2)
src/__tests__/main/web-server/web-server-factory.test.tssrc/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsx
✅ Files skipped from review due to trivial changes (1)
- src/tests/main/web-server/web-server-factory.test.ts
Contributor
Author
|
@pedramamini @reachrazamair good to go for your review |
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
2 participants
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.
Summary
cli.triggeras a first-class Cue event type — subscriptions withon: cli.triggerare designed to be invoked from the command linemaestro-cli cue trigger <name> [-p <prompt>] [--json]command that fires a named subscription via WebSocket{{CUE_CLI_PROMPT}}template variable available incli.triggersubscription promptsExample usage
maestro-cli cue trigger deploy maestro-cli cue trigger deploy --prompt "Deploy to staging only"Files changed (26)
Core: contracts, engine, dispatch service, executor, IPC handler, preload, template variables
WebSocket layer: message handler, callback registry, WebServer, factory, renderer bridge
CLI: new
cue-trigger.tscommand, registered undercueparent in indexUI: event constants, TriggerDrawer, TriggerConfig, pipeline graph/YAML utils, patterns, defaults, usePipelineState
Test plan
maestro-cli cue trigger <name>fires acli.triggersubscription--promptoverride replaces the configured prompt{{CUE_CLI_PROMPT}}substitutes in prompt templatescli.triggerappears in pipeline editor trigger drawercli.triggersubscriptions🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
maestro-cli cue trigger <name>with optional--promptand--json; supports remote/renderer-triggering and UI/editor support for a CLI Trigger typeTests
Chores