Conversation
📝 WalkthroughWalkthroughThe changes extend CLI agent capability handling to support two new tool types: opencode and factory-droid. This includes detection logic across command files, spawning implementations, output parsing, and updated type definitions in the agent spawner service. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 support for OpenCode and Factory Droid agent types to the Maestro CLI, following the established Claude/Codex integration pattern. The implementation includes binary detection ( The core architecture is sound and closely mirrors Codex's approach. Minor issues identified:
Confidence Score: 4/5
Last reviewed commit: f8477a9 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const usage = parser.extractUsage(event as any); | ||
| if (usage) { | ||
| usageStats = mergeUsageStats(usageStats, { | ||
| inputTokens: usage.inputTokens || 0, | ||
| outputTokens: usage.outputTokens || 0, | ||
| cacheReadTokens: usage.cacheReadTokens || 0, | ||
| cacheCreationTokens: usage.cacheCreationTokens || 0, | ||
| costUsd: usage.costUsd || 0, | ||
| contextWindow: usage.contextWindow || 0, | ||
| reasoningTokens: usage.reasoningTokens || 0, | ||
| }); |
There was a problem hiding this comment.
Unnecessary as any casts bypass type safety. Both spawnOpenCodeAgent (line 676) and spawnDroidAgent (line 780) cast the event to any before calling parser.extractUsage(), but the equivalent spawnCodexAgent at line 566 calls parser.extractUsage(event) without a cast. Since OpenCodeOutputParser.extractUsage() and FactoryDroidOutputParser.extractUsage() both accept the same ParsedEvent type returned by parseJsonLine(), the casts are unnecessary and hide type mismatches.
| const usage = parser.extractUsage(event as any); | |
| if (usage) { | |
| usageStats = mergeUsageStats(usageStats, { | |
| inputTokens: usage.inputTokens || 0, | |
| outputTokens: usage.outputTokens || 0, | |
| cacheReadTokens: usage.cacheReadTokens || 0, | |
| cacheCreationTokens: usage.cacheCreationTokens || 0, | |
| costUsd: usage.costUsd || 0, | |
| contextWindow: usage.contextWindow || 0, | |
| reasoningTokens: usage.reasoningTokens || 0, | |
| }); | |
| const usage = parser.extractUsage(event); |
Apply the same fix at line 780 in spawnDroidAgent.
| if (cachedOpenCodePath) { | ||
| return { available: true, path: cachedOpenCodePath, source: 'settings' }; | ||
| } |
There was a problem hiding this comment.
Cached detection results always report source: 'settings' regardless of how the path was originally resolved. When cachedOpenCodePath is first populated via PATH detection (returning source: 'path'), subsequent cache hits still report source: 'settings'. The same issue exists in detectDroid() (line ~289) and mirrors a pre-existing pattern in detectClaude() and detectCodex().
To fix, cache the source alongside the path:
| if (cachedOpenCodePath) { | |
| return { available: true, path: cachedOpenCodePath, source: 'settings' }; | |
| } | |
| let cachedOpenCodePath: string | null = null; | |
| let cachedOpenCodeSource: 'settings' | 'path' | null = null; |
Then update cache returns to use the cached source. Apply the same pattern to detectDroid().
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/cli/services/agent-spawner.ts (2)
148-175: Good addition of generic command lookup helper.
findCommandInPathnicely consolidates the pattern used byfindClaudeInPathandfindCodexInPath. Consider refactoring those two functions to use this helper in a future cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/services/agent-spawner.ts` around lines 148 - 175, Add a one-line summary: the new helper findCommandInPath centralizes command lookup and should be reused by existing findClaudeInPath and findCodexInPath; refactor those two functions to call findCommandInPath(commandName) instead of duplicating spawn logic, passing the appropriate command names (e.g., "claude" and "codex" or whatever they currently use), and preserve any special environment handling by forwarding getExpandedPath via the helper; ensure error/close handling remains the same and remove the duplicated spawn implementations from findClaudeInPath and findCodexInPath.
676-687: Type castas anysuggests type mismatch with parser.The
event as anycast indicates the parsed event type doesn't match whatextractUsageexpects. Consider aligning the types inOpenCodeOutputParserto avoid the cast, or add a type guard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/services/agent-spawner.ts` around lines 676 - 687, The call to parser.extractUsage(event as any) masks a type mismatch between the event variable and the parser's expected input; update the OpenCodeOutputParser type signature (or the parser instance type) so extractUsage accepts the actual event type, or add a narrow type guard before calling extractUsage that validates/casts event to the parser-expected interface; locate the parser instance and the extractUsage method on OpenCodeOutputParser and either align their parameter types or implement a function like isOpenCodeOutputEvent(event): event is OpenCodeOutputEvent and use it before calling parser.extractUsage, then pass the correctly typed value into mergeUsageStats (usageStats) without using any casts.
🤖 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/cli/services/agent-spawner.ts`:
- Around line 148-175: Add a one-line summary: the new helper findCommandInPath
centralizes command lookup and should be reused by existing findClaudeInPath and
findCodexInPath; refactor those two functions to call
findCommandInPath(commandName) instead of duplicating spawn logic, passing the
appropriate command names (e.g., "claude" and "codex" or whatever they currently
use), and preserve any special environment handling by forwarding
getExpandedPath via the helper; ensure error/close handling remains the same and
remove the duplicated spawn implementations from findClaudeInPath and
findCodexInPath.
- Around line 676-687: The call to parser.extractUsage(event as any) masks a
type mismatch between the event variable and the parser's expected input; update
the OpenCodeOutputParser type signature (or the parser instance type) so
extractUsage accepts the actual event type, or add a narrow type guard before
calling extractUsage that validates/casts event to the parser-expected
interface; locate the parser instance and the extractUsage method on
OpenCodeOutputParser and either align their parameter types or implement a
function like isOpenCodeOutputEvent(event): event is OpenCodeOutputEvent and use
it before calling parser.extractUsage, then pass the correctly typed value into
mergeUsageStats (usageStats) without using any casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54d7a12c-fd01-4468-987e-c6aa080e3501
📒 Files selected for processing (3)
src/cli/commands/run-playbook.tssrc/cli/commands/send.tssrc/cli/services/agent-spawner.ts
1 participant
This pull request adds support for two new agent types, OpenCode and Factory Droid, to the CLI. The changes include detection of their CLI binaries, validation logic, and agent spawning and output parsing. The implementation closely follows the existing structure for Claude and Codex agents, ensuring consistent error handling and session management.
Agent support and detection enhancements:
sendandrun-playbookcommands, including clear error messages if the binaries are not found. [1] [2] [3] [4] [5]findCommandInPath) to support flexible binary lookup for new agent types.Agent spawning and output parsing:
spawnOpenCodeAgentandspawnDroidAgentfunctions to handle agent process management, output parsing, error handling, and usage statistics aggregation for OpenCode and Factory Droid agents.OpenCodeOutputParser,FactoryDroidOutputParser) for structured JSON event parsing from agent responses.CLI integration and agent selection:
spawnAgentfunction to route requests to the appropriate agent-spawning logic based on the agent type, supporting OpenCode and Factory Droid alongside existing Claude and Codex agents.replacement for #520 which escalated through review, this adds minimal support for opencode/droid support for CLI for use with Maestro-Discord
Summary by CodeRabbit