Add Paseo schedule commands to Maestro CLI#1030
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughHidden review stack artifactWalkthroughAdds a Paseo CLI adapter that resolves and runs a local paseo executable, command handlers for run and schedule operations (create/ls/logs) with standardized success/error printing, CLI registration for the new commands, and unit tests for service and command layers. ChangesPaseo CLI Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 wires a
Confidence Score: 3/5The new Paseo adapter is otherwise clean, but the --no-run-now safety flag is silently dropped before it reaches Paseo, so the bounding control highlighted in the PR description does not actually work. Commander.js maps both --run-now and --no-run-now onto the same src/cli/services/paseo.ts — the runNow/noRunNow interface and the conditional that emits --no-run-now need to be aligned with how Commander.js actually delivers the option. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant MaestroCLI as maestro-cli (index.ts)
participant CmdHandler as paseo command (commands/paseo.ts)
participant Service as paseo service (services/paseo.ts)
participant PaseoCLI as paseo binary
User->>MaestroCLI: maestro-cli paseo schedule create [opts] "prompt"
MaestroCLI->>CmdHandler: paseoScheduleCreate(prompt, options)
CmdHandler->>Service: createPaseoSchedule(prompt, options)
Service->>Service: resolvePaseoCliPath(options.cliPath)
Note over Service: checks --cli-path → PASEO_CLI_PATH → macOS bundle → PATH
Service->>PaseoCLI: spawn(cliPath, ['schedule','create', ...flags, prompt])
PaseoCLI-->>Service: stdout / stderr / exit code
alt "exit code == 0"
Service-->>CmdHandler: "{ stdout, stderr }"
CmdHandler-->>User: prints stdout
else "exit code != 0"
Service-->>CmdHandler: throws Error
CmdHandler-->>User: prints error, exits 1
end
Reviews (1): Last reviewed commit: "Add Paseo schedule commands to CLI" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| export function createPaseoSchedule( | ||
| prompt: string, | ||
| options: PaseoScheduleCreateOptions | ||
| ): Promise<PaseoCommandResult> { |
There was a problem hiding this comment.
--no-run-now is silently dropped before reaching Paseo
Commander.js maps both --run-now and --no-run-now onto the same runNow property — true for the affirmative flag and false for the negation. It never sets a noRunNow property, so options.noRunNow is always undefined when the command arrives from the CLI, meaning the --no-run-now argument is never forwarded to Paseo. Because the PR describes this flag as a key safety-bounding control, passing --no-run-now currently has no effect.
| if (options.runNow) { | ||
| args.push('--run-now'); | ||
| } | ||
| if (options.noRunNow) { | ||
| args.push('--no-run-now'); | ||
| } |
There was a problem hiding this comment.
runNow?: boolean field and use strict equality checks to emit the correct flag only when the option was explicitly provided.
| if (options.runNow) { | |
| args.push('--run-now'); | |
| } | |
| if (options.noRunNow) { | |
| args.push('--no-run-now'); | |
| } | |
| if (options.runNow === true) { | |
| args.push('--run-now'); | |
| } else if (options.runNow === false) { | |
| args.push('--no-run-now'); | |
| } |
| export interface PaseoScheduleCreateOptions extends PaseoExecOptions { | ||
| every?: string; | ||
| cron?: string; | ||
| name?: string; | ||
| target?: string; | ||
| provider?: string; | ||
| mode?: string; | ||
| cwd?: string; | ||
| maxRuns?: string; | ||
| expiresIn?: string; | ||
| runNow?: boolean; | ||
| noRunNow?: boolean; | ||
| host?: string; | ||
| json?: boolean; | ||
| } |
There was a problem hiding this comment.
noRunNow field from the interface; runNow?: boolean already carries all three states (true / false / undefined).
| export interface PaseoScheduleCreateOptions extends PaseoExecOptions { | |
| every?: string; | |
| cron?: string; | |
| name?: string; | |
| target?: string; | |
| provider?: string; | |
| mode?: string; | |
| cwd?: string; | |
| maxRuns?: string; | |
| expiresIn?: string; | |
| runNow?: boolean; | |
| noRunNow?: boolean; | |
| host?: string; | |
| json?: boolean; | |
| } | |
| export interface PaseoScheduleCreateOptions extends PaseoExecOptions { | |
| every?: string; | |
| cron?: string; | |
| name?: string; | |
| target?: string; | |
| provider?: string; | |
| mode?: string; | |
| cwd?: string; | |
| maxRuns?: string; | |
| expiresIn?: string; | |
| runNow?: boolean; | |
| host?: string; | |
| json?: boolean; | |
| } |
| return; | ||
| } | ||
|
|
||
| const details = [stderr.trim(), stdout.trim(), `Paseo exited with code ${code}`] |
There was a problem hiding this comment.
code is null and the error message reads "Paseo exited with code null", which is confusing. Guarding with a fallback string makes the message accurate.
| const details = [stderr.trim(), stdout.trim(), `Paseo exited with code ${code}`] | |
| const details = [stderr.trim(), stdout.trim(), `Paseo exited with code ${code ?? '(signal)'}`] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/__tests__/cli/services/paseo.test.ts (1)
98-102: ⚡ Quick winAdd test coverage for spawn failure.
The test suite covers non-zero exit codes but not spawn errors (when
childProcess.spawnemits an 'error' event, handled at line 89-91 in paseo.ts). This occurs when the executable cannot be spawned (e.g., command not found, permission denied).Consider adding a test case for this scenario to ensure the error handling at line 89-91 in paseo.ts works correctly.
📋 Suggested test case
it('rejects when spawn fails', async () => { const child = new EventEmitter() as EventEmitter & { stdout: Readable; stderr: Readable; }; child.stdout = new Readable({ read() {} }); child.stderr = new Readable({ read() {} }); vi.mocked(spawn).mockReturnValue(child as any); setImmediate(() => { child.emit('error', new Error('ENOENT: command not found')); }); await expect(runPaseoCommand(['schedule', 'ls'], { cliPath: '/missing/paseo' })) .rejects.toThrow('Failed to run Paseo CLI'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/cli/services/paseo.test.ts` around lines 98 - 102, Add a new test that simulates a spawn failure by mocking childProcess.spawn (used by runPaseoCommand) to return an EventEmitter-like child that has stdout and stderr Readable streams, then emit an 'error' event (e.g., new Error('ENOENT: command not found')) via setImmediate; assert that runPaseoCommand rejects with the expected message (the same message produced in paseo.ts's spawn 'error' handler, e.g., 'Failed to run Paseo CLI'). This will exercise the spawn error handler in paseo.ts and ensure runPaseoCommand correctly surfaces spawn failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/commands/paseo.ts`:
- Around line 54-59: The catch blocks in the CLI command handlers (around
createPaseoSchedule, the other two handlers on the same file) currently only
call printError; import captureException from src/utils/sentry.ts and report
unexpected errors to Sentry before printing: in each catch, detect expected
operational errors (e.g., known custom error types/messages such as "daemon
unavailable" or "invalid schedule ID") and skip Sentry, otherwise call
captureException(error) supplying any available context (e.g., the command name
and options) and then call printError(error, options.json); ensure the import
for captureException is added at the top of the file and that the catch blocks
around createPaseoSchedule, the handler at lines ~63–68, and the handler at
~75–80 follow this pattern.
In `@src/cli/services/paseo.ts`:
- Around line 136-141: The code currently appends both '--run-now' and
'--no-run-now' when options.runNow and options.noRunNow are both true; modify
the block around the args push (the options.runNow / options.noRunNow handling
in paseo.ts) to enforce mutual exclusivity: detect if both flags are set and
either throw a clear error (or log and exit) or decide a single precedence
(e.g., prefer noRunNow) and only push one flag. Ensure the check runs before
pushing to args so only one of '--run-now' or '--no-run-now' is ever added.
---
Nitpick comments:
In `@src/__tests__/cli/services/paseo.test.ts`:
- Around line 98-102: Add a new test that simulates a spawn failure by mocking
childProcess.spawn (used by runPaseoCommand) to return an EventEmitter-like
child that has stdout and stderr Readable streams, then emit an 'error' event
(e.g., new Error('ENOENT: command not found')) via setImmediate; assert that
runPaseoCommand rejects with the expected message (the same message produced in
paseo.ts's spawn 'error' handler, e.g., 'Failed to run Paseo CLI'). This will
exercise the spawn error handler in paseo.ts and ensure runPaseoCommand
correctly surfaces spawn failures.
🪄 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: ab9d4201-5e2c-4807-b419-2bbecaa768b7
📒 Files selected for processing (5)
src/__tests__/cli/commands/paseo.test.tssrc/__tests__/cli/services/paseo.test.tssrc/cli/commands/paseo.tssrc/cli/index.tssrc/cli/services/paseo.ts
| try { | ||
| const result = await createPaseoSchedule(prompt, options); | ||
| printResult(result); | ||
| } catch (error) { | ||
| printError(error, options.json); | ||
| } |
There was a problem hiding this comment.
Integrate Sentry for error tracking as required by coding guidelines.
All three command handlers catch errors without using Sentry utilities for production error tracking. As per coding guidelines, unexpected errors should be reported to Sentry for observability, even in CLI contexts.
Consider:
- Import
captureExceptionfromsrc/utils/sentry.ts - Distinguish expected operational errors (daemon unavailable, invalid schedule ID) from unexpected errors (bugs, crashes)
- For unexpected errors, call
captureException(error)with appropriate context beforeprintError - For expected errors, proceed with current handling
Alternatively, if all CLI errors should be tracked in production, add captureException(error) at the start of each catch block.
As per coding guidelines: "Use Sentry utilities (captureException, captureMessage) from src/utils/sentry.ts for explicit error reporting with context."
Also applies to: 63-68, 75-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/commands/paseo.ts` around lines 54 - 59, The catch blocks in the CLI
command handlers (around createPaseoSchedule, the other two handlers on the same
file) currently only call printError; import captureException from
src/utils/sentry.ts and report unexpected errors to Sentry before printing: in
each catch, detect expected operational errors (e.g., known custom error
types/messages such as "daemon unavailable" or "invalid schedule ID") and skip
Sentry, otherwise call captureException(error) supplying any available context
(e.g., the command name and options) and then call printError(error,
options.json); ensure the import for captureException is added at the top of the
file and that the catch blocks around createPaseoSchedule, the handler at lines
~63–68, and the handler at ~75–80 follow this pattern.
|
@IAKGnaHz thanks for the contribution — clean integration slice and the safety bounding story is the right one. A few items from the AI reviews look worth addressing before this merges: P1 — Suggested shape: collapse if (options.runNow === true) {
args.push('--run-now');
} else if (options.runNow === false) {
args.push('--no-run-now');
}That also resolves CodeRabbit's mutual-exclusivity concern — both flags can't be set at once anymore. Sentry reporting in CLI catch blocks (CodeRabbit) Nit — null exit code message (Greptile P2) Nit — spawn-failure test (CodeRabbit) Once those are in (especially the P1) I'm happy to take another pass. Thanks again! |
|
Thanks for the review. I pushed a follow-up commit addressing the concrete feedback:
Validation run locally and in the push hook:
On the Sentry suggestion: I checked the existing Sentry utilities and they currently live under |
|
I pushed one more small follow-up commit: Reason: during local UI testing, schedule-created work is visible as schedule activity but the spawned one-off agents do not get the schedule name as their agent title. For the delegation workflow, one-off tasks should be easy to inspect in Paseo while we iterate on the orchestration layer. This adds maestro-cli paseo run \
--title "Repo Health Check" \
--provider codex \
--cwd /path/to/repo \
--detach \
"Check the repo health and summarize the result."Supported options include I also ran a real smoke test through the built Maestro CLI against the local Paseo daemon: node dist/cli/maestro-cli.js paseo run \
--title "Maestro Run Title Smoke" \
--provider codex \
--cwd /Users/admin/Documents/AMH/ALL-AI \
--detach \
--json \
"..."Paseo returned an agent with Validation:
This keeps |
2 participants
Summary
This PR adds a small, CLI-first integration point between Maestro and Paseo by introducing a
maestro-cli paseo schedule ...command group.The goal is to let Maestro act as the user-facing orchestration entrypoint while delegating agent lifecycle, recurring execution, timelines, and visibility to a running Paseo daemon.
Background
Maestro already provides a useful agent-oriented CLI surface (
send, playbooks, settings, session inspection) and is well positioned to act as an orchestration layer for higher-level workflows. Paseo, meanwhile, provides a daemon-backed execution model with a native UI for inspecting agent timelines, schedule runs, logs, and multi-agent progress.In local testing, the most reliable integration path was not to ask an intermediate agent to run shell commands, but to let Maestro call the Paseo CLI directly and return the structured result to the user or automation layer.
That gives a clean division of responsibilities:
Research / Findings
A short local spike validated the following:
paseo schedule createcan create bounded scheduled agent runs with--max-runsand--expires-in, which is important for safe automation.paseo schedule lsandpaseo schedule logs <id>.codexandclaudeare discovered by Paseo and can be selected per scheduled task.paseo run --titlesupport would be a natural follow-up because it provides better UI titles for ad hoc delegated tasks.Changes
This PR adds:
A Paseo CLI adapter in
src/cli/services/paseo.ts--cli-pathPASEO_CLI_PATH/Applications/Paseo.app/Contents/Resources/bin/paseo)paseofromPATHchild_process.spawnA new command module in
src/cli/commands/paseo.tspaseo schedule create <prompt>paseo schedule lspaseo schedule logs <schedule-id>--json,--host, and--cli-pathCLI registration in
src/cli/index.tsUnit coverage for both service and command layers
Example Usage
Safety Notes
The schedule creation path exposes Paseo's built-in bounding controls:
--max-runs--expires-in--no-run-now--cwdThese are useful defaults for higher-level orchestration because Maestro can create visible, bounded Paseo work without creating unbounded recurring agents.
Validation
Targeted checks:
Full push validation also passed:
The push hook also ran:
npm run build:promptsnpm run format:check:allnpm run lintnpm run lint:eslintnpm run testlint:eslintreported one existing warning insrc/main/web-server/web-server-factory.ts, but no errors.Follow-up Plan
This is deliberately a small integration slice. Follow-up improvements that would build on it:
Add
maestro-cli paseo runsupportpaseo run --title --detach --provider --cwdAdd a higher-level
delegatecommandPersist Paseo IDs in Maestro-side task records
Add richer output parsing for
--jsonExpose this from playbooks or future workflow templates
Summary by CodeRabbit