Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a Factory Droid agent plugin (config, CLI arg builder, executor), a streaming Droid JSONL output parser with cost accumulation and Claude-compatible conversion, integrates Droid-aware parsing into engine and TUI, adds an agentCommand shorthand to config, documentation, navigation, and a new todo-app example. Changes
Sequence Diagram(s)sequenceDiagram
participant TUI as TUI (RunApp / RightPanel)
participant Engine as Engine
participant Agent as DroidAgentPlugin
participant CLI as Droid CLI
participant Parser as Droid JSONL Parser
participant Display as TUI Display
TUI->>Engine: execute(prompt, agentId='droid')
Engine->>Agent: buildArgs(prompt, options)
Agent-->>Engine: CLI args (may include --output-format stream-json)
Engine->>CLI: spawn process with args
CLI-->>Engine: JSONL output stream (lines)
loop streaming lines
Engine->>Parser: push(line)
Parser->>Parser: parse line, accumulate costs
Parser-->>Engine: Claude-compatible JSONL messages / events
Engine-->>Display: emit formatted output (including cost events)
end
Engine->>Parser: flush()
Parser-->>Display: final output + cost summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/plugins/agents/droid/config.ts`:
- Around line 6-8: The DROID_NON_INTERACTIVE_FLAGS constant currently includes
both '--print' and its short form '-p'; remove the redundant '-p' entry so the
array contains only '--print' (leave DROID_DEFAULT_COMMAND untouched), and
run/update any code or tests that construct CLI invocations from
DROID_NON_INTERACTIVE_FLAGS to ensure they no longer expect the short flag.
In `@src/plugins/agents/droid/outputParser.ts`:
- Around line 178-184: The code allows null to pass the object check, so update
the conditional and assignment around args to prevent assigning null to
DroidToolCall.arguments: change the check to ensure args is an object and not
null (e.g., typeof args === 'object' && args !== null) and when constructing the
return value for id/name/arguments ensure arguments is either the object cast to
Record<string, unknown> or a safe default (e.g., {}) so DroidToolCall.arguments
can never be null; modify the block that builds the return value (referencing
args, id, name, and DroidToolCall.arguments in outputParser.ts) accordingly.
- Around line 448-454: The recalculation block that sets
this.summary.totalTokens from component sums overwrites previously accumulated
totalTokens and loses earlier events' totals; remove that block so totalTokens
continues to be accumulated from incoming cost.totalTokens (the addition at line
where cost.totalTokens is added to this.summary.totalTokens) rather than
recomputed, or if you prefer incremental calculation, compute the sum of the
current event's components (cost.inputTokens, cost.outputTokens,
cost.cacheReadTokens, cost.cacheWriteTokens) and add that value to
this.summary.totalTokens instead of assigning it directly; update references to
cost.totalTokens and this.summary.totalTokens accordingly.
In `@src/tui/components/RightPanel.tsx`:
- Around line 570-571: The useMemo hook that calls
parseAgentOutput(iterationOutput, agentName) is missing agentName in its
dependency array, causing stale memoized results when agentName changes; update
the dependency array for the useMemo surrounding parseAgentOutput to include
agentName alongside iterationOutput and iterationTiming?.isRunning so the memo
recomputes when the agent changes.
🧹 Nitpick comments (5)
src/plugins/agents/droid/index.ts (2)
47-49: Consider logging a warning when config parsing fails.When
safeParsefails, the function silently returns without applying any configuration. This could hide misconfiguration from users (e.g., invalidreasoningEffortvalue). WhilevalidateConfiginsrc/config/index.tsalso validates Droid options, it only runs at startup - this initialize method may be called in other contexts.💡 Optional: Add warning for parse failures
if (!parsed.success) { + console.warn('[droid] Invalid agent config:', parsed.error.issues.map(i => i.message).join(', ')); return; }
64-67: Mutating a readonly property's nested value.The
metafield is declared asreadonly, but itssupportsSubagentTracingproperty is being mutated at runtime. While this works in TypeScript (readonly only prevents reassignment of the reference), it's an unexpected pattern that could cause confusion.Consider either:
- Removing
readonlyfrom the declaration if mutation is intentional- Or handling this differently (e.g., a getter that checks
enableTracing)src/tui/output-parser.ts (1)
54-56: Loose string matching could cause false positives.The
isDroidAgentcheck usesincludes('droid')which could match unintended agent names like'android-agent'or'mydroid'. Consider using a more precise match.💡 Suggested fix for precise matching
function isDroidAgent(agentPlugin?: string): boolean { - return agentPlugin?.toLowerCase().includes('droid') ?? false; + const normalised = agentPlugin?.toLowerCase(); + return normalised === 'droid' || normalised === 'factory droid'; }src/plugins/agents/droid/outputParser.ts (2)
375-388: Consider adding array check for type consistency.
JSON.parsecould return an array (e.g., for input"[1,2,3]"), which would pass thetypeof parsed === 'object'check but doesn't match the return typeRecord<string, unknown>. This is unlikely to cause runtime issues but is technically a type inconsistency.Suggested fix
if (typeof argumentsValue === 'string') { try { const parsed = JSON.parse(argumentsValue) as Record<string, unknown>; - if (parsed && typeof parsed === 'object') { + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { return parsed; } } catch { return { arguments: argumentsValue }; } return { arguments: argumentsValue }; }
652-658: Internal state exposed directly; consider returning copies.
getState()returns direct references to the internalmessagesandfallbackarrays. If callers mutate these arrays, it would corrupt the parser's internal state. Consider returning shallow copies for defensive programming.Suggested fix
getState(): { messages: DroidJsonlMessage[]; fallback: string[]; costSummary: DroidCostSummary } { return { - messages, - fallback, + messages: [...messages], + fallback: [...fallback], costSummary: costAccumulator.getSummary(), }; },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.beads/issues.jsonlsrc/config/index.tssrc/config/schema.tssrc/config/types.tssrc/engine/index.tssrc/plugins/agents/builtin/index.tssrc/plugins/agents/droid/commandBuilder.tssrc/plugins/agents/droid/config.tssrc/plugins/agents/droid/index.tssrc/plugins/agents/droid/outputParser.tssrc/plugins/agents/droid/schema.tssrc/tui/components/RightPanel.tsxsrc/tui/components/RunApp.tsxsrc/tui/output-parser.tswebsite/content/docs/plugins/agents/droid.mdxwebsite/lib/navigation.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with 'ABOUTME: '
Files:
src/config/schema.tssrc/tui/components/RunApp.tsxsrc/plugins/agents/droid/commandBuilder.tssrc/plugins/agents/builtin/index.tswebsite/lib/navigation.tssrc/config/types.tssrc/plugins/agents/droid/config.tssrc/tui/components/RightPanel.tsxsrc/config/index.tssrc/plugins/agents/droid/schema.tssrc/engine/index.tssrc/plugins/agents/droid/index.tssrc/tui/output-parser.tssrc/plugins/agents/droid/outputParser.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with "ABOUTME: "
Files:
src/config/schema.tssrc/tui/components/RunApp.tsxsrc/plugins/agents/droid/commandBuilder.tssrc/plugins/agents/builtin/index.tswebsite/lib/navigation.tssrc/config/types.tssrc/plugins/agents/droid/config.tssrc/tui/components/RightPanel.tsxsrc/config/index.tssrc/plugins/agents/droid/schema.tssrc/engine/index.tssrc/plugins/agents/droid/index.tssrc/tui/output-parser.tssrc/plugins/agents/droid/outputParser.ts
🧬 Code graph analysis (7)
src/tui/components/RunApp.tsx (1)
src/tui/output-parser.ts (1)
StreamingOutputParser(224-420)
src/plugins/agents/droid/commandBuilder.ts (1)
src/plugins/agents/droid/config.ts (1)
DROID_NON_INTERACTIVE_FLAGS(8-8)
src/tui/components/RightPanel.tsx (1)
src/tui/output-parser.ts (1)
parseAgentOutput(116-182)
src/config/index.ts (1)
src/plugins/agents/droid/schema.ts (1)
DroidAgentConfigSchema(10-17)
src/engine/index.ts (1)
src/plugins/agents/droid/outputParser.ts (3)
createDroidStreamingJsonlParser(595-660)isDroidJsonlMessage(662-664)toClaudeJsonlMessages(666-731)
src/plugins/agents/droid/index.ts (3)
src/plugins/agents/droid/config.ts (1)
DROID_DEFAULT_COMMAND(6-6)src/plugins/agents/droid/schema.ts (2)
DroidReasoningEffort(19-19)DroidAgentConfigSchema(10-17)src/plugins/agents/droid/commandBuilder.ts (1)
buildDroidCommandArgs(17-45)
src/tui/output-parser.ts (1)
src/plugins/agents/droid/outputParser.ts (5)
DroidCostAccumulator(406-472)parseDroidJsonlLine(535-593)formatDroidEventForDisplay(499-533)formatDroidCostSummary(474-497)DroidJsonlMessage(46-56)
🪛 LanguageTool
website/content/docs/plugins/agents/droid.mdx
[uncategorized] ~8-~8: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...t plugin integrates with the droid CLI so Ralph TUI can execute tasks with Factor...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~137-~137: Possible missing preposition found.
Context: ...--print and -p pass the prompt 2. Tracing: --output-format stream-json when t...
(AI_HYDRA_LEO_MISSING_TO)
[duplication] ~157-~157: Possible typo: you repeated a word.
Context: ...d auth. ### "Tracing not showing" 1. Ensure enableTracing = true2. EnsuresubagentTracingDetailis not"off"` ...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (27)
website/lib/navigation.ts (1)
64-64: LGTM!The navigation entry for Factory Droid follows the established pattern and is correctly placed under Plugins → Agents, consistent with the other agent plugins (Claude, OpenCode).
src/config/schema.ts (1)
111-112: LGTM!The
agentCommandfield addition is consistent with the corresponding type definition intypes.tsand follows the existing schema patterns for optional string fields.src/config/types.ts (1)
155-157: LGTM!The
agentCommandfield addition is well-documented and consistently placed alongside related agent configuration fields. The type aligns with the corresponding Zod schema inschema.ts.src/plugins/agents/builtin/index.ts (1)
7-7: LGTM!The Droid agent registration follows the established pattern used for Claude and OpenCode agents. The import, registration call, and export are consistent with existing conventions.
Also applies to: 21-21, 25-25
website/content/docs/plugins/agents/droid.mdx (1)
1-165: Documentation is comprehensive and well-structured.The documentation covers all essential aspects: prerequisites, configuration options, troubleshooting, and how the agent constructs CLI commands. The content aligns with the implementation.
Minor: Consider adding a comma on line 8 for readability: "integrates with the
droidCLI**,** so Ralph TUI can execute tasks...".src/tui/components/RunApp.tsx (1)
319-323: LGTM!The
StreamingOutputParseris now correctly initialised with the agent plugin identifier, enabling agent-specific output parsing for Droid vs Claude. The fallback chain (defaultAgent→agent→'claude') is sensible.Note that this parser instance is created once at mount time via
useRefinitialiser, so if the agent configuration changes mid-session, the parser won't reflect the new agent. This appears intentional given that the parser accumulates state across an iteration.src/engine/index.ts (2)
727-729: Parser selection logic is correct.The conditional parser creation based on agent ID enables Droid-specific JSONL parsing whilst preserving the existing Claude path.
753-759: LGTM — Droid message normalisation is handled consistently.The implementation correctly normalises Droid JSONL messages to Claude format before feeding them to the subagent parser. The pattern is applied consistently in both the streaming
push()path and theflush()path, ensuring no messages are missed.The use of
isDroidJsonlMessageas a type guard before callingtoClaudeJsonlMessagesis appropriate.Also applies to: 787-793
src/plugins/agents/droid/schema.ts (1)
1-20: LGTM!The Zod schema is well-structured and follows best practices:
DroidReasoningEffortSchemacorrectly constrains reasoning effort to valid values- Optional fields with sensible defaults (
enableTracing: true,skipPermissions: false) align with documentation.passthrough()allows forward compatibility with additional options- Type exports via
z.inferprovide compile-time safetysrc/plugins/agents/droid/commandBuilder.ts (1)
1-45: LGTM - Clean command builder implementation.The implementation correctly builds the Droid CLI arguments with proper flag ordering. The ABOUTME comment satisfies the coding guidelines.
One minor consideration: the function doesn't validate empty prompts. If an empty string is passed as
prompt, it would still be included in the args array. This is likely fine as the calling code should validate, but worth noting.src/config/index.ts (2)
562-570: LGTM - Proper Droid-specific configuration validation.The validation correctly uses
safeParseand formats error messages with paths. This plugin-specific validation pattern matches how tracker validation would work.One consideration: the validation occurs after checking if the agent plugin exists (line 558), which is good - no point validating options for a non-existent plugin.
330-347: LGTM - Clean shorthand resolution for agentCommand.The coalescing logic
storedConfig.agent ?? storedConfig.agentCommandcorrectly prioritises the explicitagentfield over the newagentCommandalias, maintaining backwards compatibility..beads/issues.jsonl (1)
61-66: Issue tracker data - no code review required.These are issue tracking entries documenting the Factory Droid implementation tasks. The entries are well-structured and provide good traceability for the PR changes.
src/plugins/agents/droid/index.ts (1)
70-84: LGTM - Clean delegation to command builder.The
buildArgsmethod correctly delegates tobuildDroidCommandArgswith all necessary parameters. The tracing enablement logic (this.enableTracing && options?.subagentTracing === true) ensures both the config setting and runtime option must be true.src/tui/output-parser.ts (3)
129-157: Potential duplicate cost summary in batch parsing.In
parseAgentOutput, the cost summary is always appended at line 156 after processing all lines. However, if the Droid output contains a final event (liketype: 'result'),formatDroidEventForDisplaymay already include cost information, leading to potential duplication.The streaming parser (
StreamingOutputParser) handles this vialastCostSummarydeduplication, but the batchparseAgentOutputfunction doesn't have this protection.Consider whether the batch parser needs similar deduplication logic, or verify that
formatDroidEventForDisplaynever includes cost information.
363-390: Good deduplication mechanism for streaming cost summaries.The
formatDroidCostSummaryIfFinalmethod correctly:
- Only emits on final-like events (
result,final,done,summary, or boolean flags)- Tracks
lastCostSummaryto prevent duplicate outputThis ensures users see the cost summary exactly once at the end of execution.
411-419: LGTM - Proper state reset for new iterations.The
reset()method correctly clears all state including the newlastCostSummaryand reinitialises theDroidCostAccumulator. Creating a fresh accumulator (rather than clearing the existing one) is a clean approach.src/plugins/agents/droid/outputParser.ts (10)
1-7: LGTM!The file header includes the required ABOUTME JSDoc comment explaining the file's purpose, and the import is appropriate for the Claude message conversion functionality.
8-61: LGTM!The interfaces are well-structured with appropriate optional fields and good use of a discriminated union for parse results. The literal
source: 'droid'type enables reliable type guarding.
62-84: LGTM!The helper functions are robust:
asRecordcorrectly handles null,readStringproperly validates non-empty strings, andreadNumberusesNumber.isFiniteto reject invalid numeric values including NaN and Infinity.
86-151: LGTM!The content extraction functions handle diverse payload shapes gracefully, supporting both camelCase and snake_case field variants with sensible fallback logic.
211-269: LGTM!The tool result parsing functions are well-structured and leverage the helper functions properly for safe extraction.
271-300: LGTM!The error extraction logic handles various error representations comprehensively, including the special case for non-zero exit codes.
302-366: LGTM!The cost extraction function comprehensively handles various field naming conventions across different API response formats and correctly returns undefined when no cost data is present.
474-533: LGTM!The formatting functions are well-structured with clear logic for building human-readable output strings, handling edge cases appropriately.
535-593: LGTM!The main parsing function has comprehensive error handling and cleanly extracts all relevant fields using the helper functions. The discriminated union return type enables proper error handling by callers.
662-731: LGTM!The type guard is effective for discriminating Droid messages. The
toClaudeJsonlMessagesconversion function properly handles the one-to-many mapping when a single Droid message contains multiple tool calls or results, generating appropriate Claude-compatible messages for each.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@website/content/docs/plugins/agents/droid.mdx`:
- Around line 137-140: The phrase "when tracing enabled" is missing the verb;
update the documentation text that currently reads "Tracing: `--output-format
stream-json` when tracing enabled" to "Tracing: `--output-format stream-json`
when tracing is enabled" so the sentence is grammatically correct and clear.
🧹 Nitpick comments (1)
website/content/docs/plugins/agents/droid.mdx (1)
60-71: Consider usingskipPermissions = falsein the full config example.The "Full Config" example sets
skipPermissions = true, which bypasses safety prompts. Since this is an advanced/dangerous setting (as noted in the warning callout later), showing it astruein the example may inadvertently encourage unsafe usage. Consider setting it tofalsehere to promote safer defaults, or add an inline comment warning users.📝 Suggested change
[agents.options] model = "claude-opus-4-5-20251101" reasoningEffort = "high" -skipPermissions = true +skipPermissions = false # Set to true only for trusted, fully autonomous tasks enableTracing = true
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/content/docs/plugins/agents/droid.mdx
🧰 Additional context used
🪛 LanguageTool
website/content/docs/plugins/agents/droid.mdx
[uncategorized] ~8-~8: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...t plugin integrates with the droid CLI so Ralph TUI can execute tasks with Factor...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~139-~139: Possible missing preposition found.
Context: ...--print and -p pass the prompt 2. Tracing: --output-format stream-json when t...
(AI_HYDRA_LEO_MISSING_TO)
[duplication] ~159-~159: Possible typo: you repeated a word.
Context: ...d auth. ### "Tracing not showing" 1. Ensure enableTracing = true2. EnsuresubagentTracingDetailis not"off"` ...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (2)
website/content/docs/plugins/agents/droid.mdx (2)
1-167: Documentation is well-structured and comprehensive.The documentation covers all essential aspects of the Factory Droid agent plugin: prerequisites, configuration (both shorthand and full), options reference, tracing, autonomous operation, and troubleshooting. The use of callouts for tips, info, and warnings is appropriate, and the command example in "How It Works" clearly demonstrates what Ralph TUI constructs under the hood.
84-86: The external documentation link is accessible and valid. The URL returns HTTP 200 with proper HTML content, confirming it resolves correctly and provides the expected content for Factory Droid's available models documentation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Remove redundant '-p' from DROID_NON_INTERACTIVE_FLAGS (--print suffices) - Fix null check in parseToolCall to prevent null assignment to arguments - Fix totalTokens accumulation to add event totals instead of overwriting - Add missing agentName dependency to useMemo in RightPanel - Fix grammar in docs: "when tracing enabled" -> "when tracing is enabled"
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@website/content/docs/plugins/agents/droid.mdx`:
- Around line 128-135: Update the example CLI usage: remove the incorrect `-p`
flag from the snippet that currently shows `droid --print -p "<prompt>"` and
instead pass the prompt as the positional argument immediately after `--print`,
keeping the rest of the flags (`--model`, `--reasoning-effort`,
`--skip-permissions-unsafe`, `--output-format`, `--cwd`) unchanged.
♻️ Duplicate comments (3)
src/plugins/agents/droid/outputParser.ts (2)
178-184: The null assignment issue has been addressed.The condition now correctly checks
args !== null, preventing null from being assigned toarguments.
442-456: ThetotalTokensaccumulation logic has been corrected.The implementation now properly accumulates: when
cost.totalTokensis provided, it's added to the running total (line 443); when absent, the sum of component tokens is added instead (lines 448-455). This preserves previously accumulated values.website/content/docs/plugins/agents/droid.mdx (1)
137-140: Grammar fix has been applied.Line 139 now correctly reads "when tracing is enabled".
🧹 Nitpick comments (2)
src/tui/components/RightPanel.tsx (1)
474-479: Consider extracting themodelDisplayparsing logic into a shared utility.The same model parsing pattern (splitting on
/, constructing{ provider, model, full, display }) is duplicated here, inTaskOutputView(lines 572-577), and inHeader.tsxandProgressDashboard.tsx. Extracting this into a utility function would reduce repetition and ensure consistent behaviour.Example utility function
// In theme.ts or a new utils file export function parseModelDisplay(model: string | undefined): { provider: string; model: string; full: string; display: string; } | null { if (!model) return null; const [provider, modelName] = model.includes('/') ? model.split('/') : ['', model]; return { provider, model: modelName, full: model, display: provider ? `${provider}/${modelName}` : modelName, }; }src/plugins/agents/droid/outputParser.ts (1)
654-660:getStatereturns mutable internal arrays by reference.The
messagesandfallbackarrays are returned directly, allowing callers to mutate the parser's internal state. If immutability is intended, return copies instead.Suggested fix
getState(): { messages: DroidJsonlMessage[]; fallback: string[]; costSummary: DroidCostSummary } { return { - messages, - fallback, + messages: [...messages], + fallback: [...fallback], costSummary: costAccumulator.getSummary(), }; },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/plugins/agents/droid/config.tssrc/plugins/agents/droid/outputParser.tssrc/tui/components/Header.tsxsrc/tui/components/ProgressDashboard.tsxsrc/tui/components/RightPanel.tsxwebsite/content/docs/plugins/agents/droid.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/plugins/agents/droid/config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with 'ABOUTME: '
Files:
src/tui/components/RightPanel.tsxsrc/tui/components/Header.tsxsrc/plugins/agents/droid/outputParser.tssrc/tui/components/ProgressDashboard.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with "ABOUTME: "
Files:
src/tui/components/RightPanel.tsxsrc/tui/components/Header.tsxsrc/plugins/agents/droid/outputParser.tssrc/tui/components/ProgressDashboard.tsx
🧬 Code graph analysis (3)
src/tui/components/RightPanel.tsx (2)
src/tui/theme.ts (1)
colors(9-58)src/tui/output-parser.ts (1)
parseAgentOutput(116-182)
src/tui/components/Header.tsx (1)
src/tui/theme.ts (1)
colors(9-58)
src/tui/components/ProgressDashboard.tsx (1)
src/tui/theme.ts (1)
colors(9-58)
🪛 LanguageTool
website/content/docs/plugins/agents/droid.mdx
[uncategorized] ~8-~8: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...t plugin integrates with the droid CLI so Ralph TUI can execute tasks with Factor...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~139-~139: Possible missing preposition found.
Context: ...ode**: --print passes the prompt 2. Tracing: --output-format stream-json when t...
(AI_HYDRA_LEO_MISSING_TO)
[duplication] ~159-~159: Possible typo: you repeated a word.
Context: ...d auth. ### "Tracing not showing" 1. Ensure enableTracing = true2. EnsuresubagentTracingDetailis not"off"` ...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (4)
src/tui/components/RightPanel.tsx (1)
568-569: LGTM! TheagentNamedependency issue has been addressed.The
useMemodependency array now correctly includesagentName, ensuring the memoised value recomputes when the agent changes.src/tui/components/Header.tsx (1)
161-166: LGTM! Consistent model display formatting.The
displayfield correctly handles models without a provider prefix, avoiding the leading/issue mentioned in the PR objectives.src/tui/components/ProgressDashboard.tsx (1)
93-98: LGTM! Model display formatting is consistent with other components.src/plugins/agents/droid/outputParser.ts (1)
375-385: Unreachable code on line 384.Line 384 (
return { arguments: argumentsValue };) is unreachable because thecatchblock on line 382 already returns the same value. The control flow either succeeds and returns on line 379, or throws and returns from line 382.Suggested fix
if (typeof argumentsValue === 'string') { try { const parsed = JSON.parse(argumentsValue) as Record<string, unknown>; if (parsed && typeof parsed === 'object') { return parsed; } } catch { return { arguments: argumentsValue }; } - return { arguments: argumentsValue }; + // JSON.parse succeeded but result wasn't an object - wrap as string argument + return { arguments: argumentsValue }; }Actually, this line is reachable when
JSON.parsesucceeds but the result is not an object (e.g., parsing"null"or"123"). The conditionparsed && typeof parsed === 'object'would be false. Consider adding a comment to clarify this edge case.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- TodoItem component displays checkbox for each todo - Checking marks todo complete with line-through and gray text styling - Unchecking restores incomplete state via toggleTodo function - All acceptance criteria implemented in US-001 iteration
- Add localStorage persistence to useTodos hook - Load saved todos on initial render - Save todos whenever state changes - Handle storage errors gracefully
- Wrap droid command with `script` to provide pseudo-TTY (fixes "raw mode is not supported" error when Ink initializes without TTY) - Fix argument ordering so `exec` subcommand comes before flags - Add `cd` prefix to shell command to respect working directory - Add ANSI stripping and JSON extraction for cleaner output parsing - Filter user/input events from droid JSONL output - Pass agentPlugin prop through RunAppWrapper to RunApp so parser correctly detects droid agent - Add setAgentPlugin method to StreamingOutputParser for dynamic updates
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@examples/todo-app/package.json`:
- Around line 14-22: The project is missing the React Vite plugin and a Vite
config so TSX/JSX won't be transformed; add the dependency
"@vitejs/plugin-react" to devDependencies in package.json and create a
vite.config.ts that imports defineConfig from 'vite' and the default export from
'@vitejs/plugin-react', then export defineConfig({ plugins: [react()] }) so Vite
can process src/index.tsx and other .tsx files.
In `@examples/todo-app/src/components/AddTodo.tsx`:
- Around line 14-20: In handleSubmit, you check text.trim() but call onAdd with
the untrimmed text; compute const trimmed = text.trim() (or reuse the trimmed
result), use trimmed in the condition and pass trimmed to onAdd (keep
setText("") as-is), so the AddTodo component's handleSubmit uses the trimmed
value when invoking onAdd.
In `@src/plugins/agents/droid/index.ts`:
- Around line 71-73: The code mutates readonly meta.supportsSubagentTracing in
the class (check the meta declaration and the enableTracing check), so either
remove the readonly modifier from the meta declaration if meta is intended to be
mutable, or stop mutating it and introduce a separate instance property (e.g.,
supportsSubagentTracing or effectiveSupportsSubagentTracing) initialized from
this.meta.supportsSubagentTracing; then change the if-block to set that new
property (if (!this.enableTracing) this.supportsSubagentTracing = false) and
update all other references to use the new property instead of mutating
this.meta.
🧹 Nitpick comments (7)
examples/todo-app/src/components/AddTodo.tsx (1)
22-26: Redundant Enter key handler.The
handleKeyDownhandler is unnecessary since the input is inside a<form>element. Pressing Enter in an input field within a form automatically triggers the form'sonSubmitevent.♻️ Proposed simplification
- const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => { - if (e.key === "Enter") { - handleSubmit(e); - } - }; - return ( <form onSubmit={handleSubmit} className="flex gap-2 mb-4"> <input type="text" value={text} onChange={(e) => setText(e.target.value)} - onKeyDown={handleKeyDown} placeholder="Add a new todo..." className="flex-1 px-4 py-2 border border-gray-300 rounded-lg focus:outline-none focus:ring-2 focus:ring-blue-500" />examples/todo-app/tailwind.config.js (1)
1-8: Missing required ABOUTME JSDoc comment.As per coding guidelines, all
.jsfiles should start with a file-level JSDoc comment explaining the file's purpose, prefixed withABOUTME:.Proposed fix
+/** + * ABOUTME: Tailwind CSS configuration for the todo-app example + */ + /** `@type` {import('tailwindcss').Config} */ export default { content: ["./index.html", "./src/**/*.{js,ts,jsx,tsx}"],examples/todo-app/src/hooks/useTodos.ts (1)
11-21: Consider validating parsed data structure.If localStorage contains malformed or unexpected data (e.g., from a previous app version or manual tampering),
JSON.parsemay succeed but return data that doesn't match theTodo[]shape, potentially causing runtime errors downstream.Optional: Add basic validation
function loadTodosFromStorage(): Todo[] { try { const stored = localStorage.getItem(STORAGE_KEY); if (stored) { - return JSON.parse(stored); + const parsed = JSON.parse(stored); + if (Array.isArray(parsed)) { + return parsed; + } } } catch { // Ignore parse errors, return empty array } return []; }examples/todo-app/src/index.tsx (1)
12-13: Consider wrapping with StrictMode for development.React's
StrictModehelps identify potential problems by double-invoking certain lifecycle methods and detecting unsafe practices during development.Optional: Add StrictMode
+import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import { App } from "./components/App"; import "./index.css"; const container = document.getElementById("root"); if (!container) throw new Error("Root element not found"); const root = createRoot(container); -root.render(<App />); +root.render( + <StrictMode> + <App /> + </StrictMode> +);website/content/docs/plugins/agents/droid.mdx (1)
45-54: Example shows non-default value without explanation.The shorthand config example shows
skipPermissions = false, but the Options Reference (line 79) and the Autonomous Operation section (line 113) both state the default istrue. Consider either:
- Removing
skipPermissions = falsefrom the basic example (sincetrueis the default)- Adding a comment explaining this overrides the default
This could confuse users about the actual default behaviour.
src/tui/output-parser.ts (1)
54-56: Consider stricter agent plugin matching.The
isDroidAgentfunction usesincludes('droid')which could match unintended plugin names (e.g., a hypothetical "android-agent" or "droidvania"). Consider using a more precise check:function isDroidAgent(agentPlugin?: string): boolean { - return agentPlugin?.toLowerCase().includes('droid') ?? false; + return agentPlugin?.toLowerCase() === 'droid' ?? false; }Alternatively, if there are variants like "factory-droid", use a word-boundary check or exact match against known Droid plugin IDs.
src/plugins/agents/droid/index.ts (1)
52-54: Silent failure on configuration parse error.When
DroidAgentConfigSchema.safeParsefails, the method returns silently without logging a warning. This could make debugging configuration issues difficult. Consider adding a warning:if (!parsed.success) { + console.warn('[droid] Invalid agent configuration:', parsed.error.message); return; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
examples/todo-app/index.htmlexamples/todo-app/package.jsonexamples/todo-app/postcss.config.jsexamples/todo-app/src/components/AddTodo.tsxexamples/todo-app/src/components/App.tsxexamples/todo-app/src/components/TodoItem.tsxexamples/todo-app/src/components/TodoList.tsxexamples/todo-app/src/hooks/useTodos.tsexamples/todo-app/src/index.cssexamples/todo-app/src/index.tsxexamples/todo-app/src/types/todo.tsexamples/todo-app/tailwind.config.jsexamples/todo-app/tsconfig.jsonsrc/commands/run.tsxsrc/plugins/agents/droid/commandBuilder.tssrc/plugins/agents/droid/config.tssrc/plugins/agents/droid/index.tssrc/plugins/agents/droid/outputParser.tssrc/plugins/agents/droid/schema.tssrc/tui/components/RunApp.tsxsrc/tui/output-parser.tswebsite/content/docs/plugins/agents/droid.mdx
✅ Files skipped from review due to trivial changes (2)
- examples/todo-app/postcss.config.js
- examples/todo-app/src/index.css
🚧 Files skipped from review as they are similar to previous changes (4)
- src/plugins/agents/droid/config.ts
- src/tui/components/RunApp.tsx
- src/plugins/agents/droid/schema.ts
- src/plugins/agents/droid/commandBuilder.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with 'ABOUTME: '
Files:
examples/todo-app/tailwind.config.jsexamples/todo-app/src/index.tsxexamples/todo-app/src/hooks/useTodos.tsexamples/todo-app/src/components/App.tsxexamples/todo-app/src/components/TodoItem.tsxexamples/todo-app/src/components/TodoList.tsxexamples/todo-app/src/types/todo.tssrc/tui/output-parser.tssrc/plugins/agents/droid/index.tsexamples/todo-app/src/components/AddTodo.tsxsrc/commands/run.tsxsrc/plugins/agents/droid/outputParser.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with "ABOUTME: "
Files:
examples/todo-app/tailwind.config.jsexamples/todo-app/src/index.tsxexamples/todo-app/src/hooks/useTodos.tsexamples/todo-app/src/components/App.tsxexamples/todo-app/src/components/TodoItem.tsxexamples/todo-app/src/components/TodoList.tsxexamples/todo-app/src/types/todo.tssrc/tui/output-parser.tssrc/plugins/agents/droid/index.tsexamples/todo-app/src/components/AddTodo.tsxsrc/commands/run.tsxsrc/plugins/agents/droid/outputParser.ts
🧬 Code graph analysis (7)
examples/todo-app/src/index.tsx (1)
examples/todo-app/src/components/App.tsx (1)
App(9-23)
examples/todo-app/src/hooks/useTodos.ts (1)
examples/todo-app/src/types/todo.ts (1)
Todo(5-10)
examples/todo-app/src/components/App.tsx (3)
examples/todo-app/src/hooks/useTodos.ts (1)
useTodos(31-68)examples/todo-app/src/components/AddTodo.tsx (1)
AddTodo(11-46)examples/todo-app/src/components/TodoList.tsx (1)
TodoList(14-35)
examples/todo-app/src/components/TodoItem.tsx (1)
examples/todo-app/src/types/todo.ts (1)
Todo(5-10)
examples/todo-app/src/components/TodoList.tsx (2)
examples/todo-app/src/types/todo.ts (1)
Todo(5-10)examples/todo-app/src/components/TodoItem.tsx (1)
TodoItem(13-37)
src/tui/output-parser.ts (1)
src/plugins/agents/droid/outputParser.ts (5)
DroidCostAccumulator(406-474)parseDroidJsonlLine(562-624)formatDroidEventForDisplay(501-542)formatDroidCostSummary(476-499)DroidJsonlMessage(46-56)
src/plugins/agents/droid/index.ts (5)
src/plugins/agents/index.ts (1)
BaseAgentPlugin(23-23)src/plugins/agents/droid/config.ts (1)
DROID_DEFAULT_COMMAND(6-6)src/plugins/agents/droid/schema.ts (2)
DroidReasoningEffort(19-19)DroidAgentConfigSchema(10-17)src/plugins/agents/droid/commandBuilder.ts (1)
buildDroidCommandArgs(17-51)src/plugins/agents/builtin/index.ts (1)
createDroidAgent(25-25)
🪛 Biome (2.1.2)
src/plugins/agents/droid/outputParser.ts
[error] 547-547: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 547-547: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 547-547: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 547-547: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 547-547: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🪛 LanguageTool
website/content/docs/plugins/agents/droid.mdx
[uncategorized] ~8-~8: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...t plugin integrates with the droid CLI so Ralph TUI can execute tasks with Factor...
(COMMA_COMPOUND_SENTENCE_2)
[duplication] ~160-~160: Possible typo: you repeated a word.
Context: ...d auth. ### "Tracing not showing" 1. Ensure enableTracing = true2. EnsuresubagentTracingDetailis not"off"` ...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (18)
examples/todo-app/index.html (1)
1-12: LGTM!Standard HTML5 entry point for a Vite + React application. The structure is correct and the direct
.tsxmodule reference will be handled by Vite's TypeScript transformation.examples/todo-app/src/types/todo.ts (1)
1-10: LGTM!The file correctly includes the required ABOUTME JSDoc comment and defines a clean, well-typed interface for the Todo entity.
examples/todo-app/src/components/TodoItem.tsx (1)
1-37: LGTM!Well-structured component with proper TypeScript typing, clean conditional styling for completed state, and appropriate separation of concerns. The ABOUTME comment is correctly included per coding guidelines.
examples/todo-app/src/components/TodoList.tsx (1)
1-35: LGTM!Well-structured component with proper ABOUTME documentation, clear TypeScript interface, good empty state handling, and correct list rendering with stable keys.
examples/todo-app/src/hooks/useTodos.ts (1)
31-67: LGTM!The hook is well-structured with proper use of
useStatelazy initialisation,useCallbackfor stable references, anduseEffectfor persistence. The CRUD operations are implemented correctly with immutable state updates.examples/todo-app/src/components/App.tsx (1)
1-23: LGTM!Clean component composition with proper ABOUTME documentation. The App component correctly orchestrates the todo functionality by wiring the
useTodoshook to the presentation components.examples/todo-app/src/index.tsx (1)
1-13: LGTM!Correct React 18 entry point implementation with proper ABOUTME documentation and defensive null check for the root element.
website/content/docs/plugins/agents/droid.mdx (1)
1-168: Documentation is comprehensive and well-structured.The documentation covers all key aspects of the Droid agent plugin: prerequisites, configuration options, tracing, and troubleshooting. The command construction example aligns with the implementation in
commandBuilder.ts.src/commands/run.tsx (1)
538-539: LGTM: Clean prop threading for agent plugin.The
agentPluginprop is correctly added toRunAppWrapperProps, destructured, and passed through toRunApp. This enables downstream components to access the resolved agent plugin name for Droid-specific parsing logic.Also applies to: 564-564, 691-691, 911-911
src/tui/output-parser.ts (2)
224-237: LGTM: Droid-aware streaming parser initialisation.The constructor correctly initialises Droid-specific state when the agent plugin indicates Droid. The lazy initialisation of
DroidCostAccumulatoronly when needed is efficient.
378-405: Well-designed duplicate prevention for cost summaries.The
formatDroidCostSummaryIfFinalhelper correctly identifies final events and useslastCostSummaryto prevent duplicate cost summary output. The heuristics for detecting final events (checking type and raw flags) are comprehensive.src/plugins/agents/droid/index.ts (2)
120-124: Good: Correct argument ordering forexecsubcommand.The code correctly ensures the
execsubcommand comes first afterdroid, followed by default flags, user flags, and then remaining arguments. This aligns with CLI conventions where subcommands must precede flags.
134-148: Shell escaping functions are secure.The
simpleEscapeandfullEscapefunctions properly handle shell metacharacters including single quotes, backslashes, newlines, carriage returns, and tabs. Using$'...'syntax for complex strings is the correct approach for bash/zsh.src/plugins/agents/droid/outputParser.ts (4)
547-551: ANSI escape sequence regex is correct.The static analysis warnings about control characters are false positives. The regex intentionally uses
\x1b(ESC) and\x07(BEL) characters to match ANSI escape sequences. This is the standard approach for stripping terminal control codes. The regex covers:
- CSI sequences:
ESC[...letter- OSC sequences:
ESC]...BEL- Character set designators:
ESC(A,ESC)B, etc.- Bare CSI-style sequences without ESC prefix
448-456: Cost accumulation logic is now correct.The previous review flagged an issue where
totalTokenswas being overwritten. The current implementation correctly uses+=to add the computed event total to the existingsummary.totalTokens, preserving previously accumulated values.
697-762: Comprehensive Droid-to-Claude message conversion.The
toClaudeJsonlMessagesfunction properly handles tool calls, tool results, and base messages. It correctly spreads the base properties and adds type-specific fields. The handling of multiple tool calls/results as separate messages aligns with Claude's JSONL format expectations.
626-691: Well-structured streaming parser factory.The
createDroidStreamingJsonlParserfactory encapsulates buffer management, message accumulation, and cost tracking cleanly. The separation ofpush(streaming) andflush(finalisation) methods follows a standard streaming parser pattern.examples/todo-app/tsconfig.json (1)
1-17: LGTM!This is a well-structured TypeScript configuration for a modern React application using a bundler. The settings are appropriate:
"bundler"moduleResolution aligns with modern tooling (Vite/esbuild),"react-jsx"enables the new JSX transform, and strict mode is correctly enabled.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add @vitejs/plugin-react and vite.config.ts for todo-app example - Fix AddTodo.tsx to use trimmed text when calling onAdd - Fix droid plugin readonly meta mutation by introducing effectiveSupportsSubagentTracing instance property with meta getter
- Remove redundant '-p' from DROID_NON_INTERACTIVE_FLAGS (--print suffices) - Fix null check in parseToolCall to prevent null assignment to arguments - Fix totalTokens accumulation to add event totals instead of overwriting - Add missing agentName dependency to useMemo in RightPanel - Fix grammar in docs: "when tracing enabled" -> "when tracing is enabled"
feat: Factory Droid agent plugin
1 participant
Overview
Add a Factory Droid agent plugin that integrates the
droidCLI with ralph-tui, making Factory Droid a first-class autonomous agent alongside Claude Code and OpenCode.Problem
Ralph-TUI supports Claude Code and OpenCode, but teams using Factory Droid lack native support. This plugin enables Factory Droid’s capabilities (codebase indexing, reasoning effort control, stream JSON tracing) within the existing ralph-tui orchestration loop.
Goals
What’s Included
Agent Integration
agentCommand: 'droid'selects Factory Droid--printand-pprompt--cwdConfiguration
model→--modelreasoningEffort→--reasoning-effortskipPermissions→--skip-permissions-unsafe(with warning)enableTracingtoggles--output-format stream-jsonTracing + JSONL Parsing
Documentation
/docs/plugins/agents/droidsrc/plugins/agents/droid/README.mdUI Fix
/when model lacks provider prefix in Header, Dashboard, and Output viewFiles / Structure
Prerequisites
droid) installed and on PATHFACTORY_API_KEYconfigured (ordroid auth)Commits Included (only)
Testing
Previously verified in the original branch (typecheck/lint/build). Not rerun after cherry-pick.
Summary by CodeRabbit
New Features
Documentation
Examples
✏️ Tip: You can customize this high-level summary in your review settings.