Add Gemini CLI as first-class agent#435
Open
openasocket wants to merge 338 commits intoRunMaestro:0.16.0-RCfrom
Open
Add Gemini CLI as first-class agent#435openasocket wants to merge 338 commits intoRunMaestro:0.16.0-RCfrom
openasocket wants to merge 338 commits intoRunMaestro:0.16.0-RCfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds first-class Gemini CLI support across CLI spawner/commands, output parsing, session storage and stats, process listeners (workspace approval), renderer UI (modals, wiring), PATH probing, IPC changes, new public types/exports, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer UI
participant Main as Main / IPC
participant Spawner as Agent Spawner
participant Gemini as Gemini CLI
participant Parser as GeminiOutputParser
participant StatsStore as Gemini Stats Store
participant Storage as GeminiSessionStorage
Renderer->>Main: Request spawn (gemini-cli, prompt, sessionId, approvedDirs)
Main->>Spawner: spawnAgent('gemini-cli', cwd, prompt, sessionId)
Spawner->>Spawner: detectGemini() → path/source
Spawner->>Gemini: spawn process (--output-format=stream-json, args)
Gemini-->>Spawner: NDJSON lines (init, message, tool_use, result)
Spawner->>Parser: parseJsonLine(line)
Parser-->>Spawner: ParsedEvent (type, sessionId, usage, text)
Spawner->>Main: emit thinking-chunk / result events
Spawner->>StatsStore: persist per-turn usage (input/output/cache/reasoning)
Main->>Storage: persist session file (~/.gemini/history)
Gemini-->>Main: stderr line indicating workspace denial
Main->>Renderer: emit workspace-approval modal request (deniedPath, error)
Renderer->>Main: approve workspace dir
Main->>Spawner: restart process with additionalWorkspaceDirs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR adds Google Gemini CLI as a fully integrated first-class agent in Maestro, achieving feature parity with Claude Code and Codex on 18/20 integration dimensions. The implementation is comprehensive, well-tested, and follows the established agent integration patterns. Key accomplishments:
Code quality:
Design decisions:
The integration is production-ready with no critical issues found. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Gemini CLI Integration] --> B[Core Agent Plumbing]
A --> C[Output Processing]
A --> D[Session Management]
A --> E[Group Chat]
A --> F[UI/CLI]
B --> B1[Agent Definitions<br/>batchModeArgs, jsonOutputArgs<br/>resumeArgs, modelArgs]
B --> B2[Capabilities<br/>18/20 features enabled<br/>supportsJsonOutput: true]
B --> B3[Type System<br/>Added to ToolType union<br/>AGENT_DEFINITIONS]
C --> C1[Output Parser<br/>6 event types: init, message<br/>tool_use, tool_result, error, result]
C --> C2[StdoutHandler<br/>Text routing: partial vs complete<br/>Stats emission, Axios suppression]
C --> C3[StderrHandler<br/>Capacity error detection<br/>Noise suppression, model extraction]
C --> C4[Error Patterns<br/>20+ patterns, 7 categories<br/>Model-specific messages]
D --> D1[Session Storage<br/>~/.gemini/history/ reader<br/>Pagination, search, delete]
D --> D2[Stats Listener<br/>Per-turn accumulation<br/>Persistent electron-store]
D --> D3[Session Origins<br/>Naming, starring<br/>Metadata tracking]
E --> E1[Moderator Spawn<br/>--no-sandbox flag<br/>CWD: group chat folder]
E --> E2[Participant Spawn<br/>--include-directories<br/>Project + chat folder]
E --> E3[Workspace Approval<br/>Modal for sandbox violations<br/>User-approved directories]
F --> F1[UI Components<br/>Wizard, modals, selectors<br/>Context usage display]
F --> F2[CLI Tooling<br/>agent-spawner service<br/>Playbook/send commands]
F --> F3[Error Recovery<br/>Change Model action<br/>Capacity error handling]
style A fill:#4a9eff,stroke:#2563eb,color:#fff
style B fill:#10b981,stroke:#059669,color:#fff
style C fill:#f59e0b,stroke:#d97706,color:#fff
style D fill:#8b5cf6,stroke:#7c3aed,color:#fff
style E fill:#ec4899,stroke:#db2777,color:#fff
style F fill:#06b6d4,stroke:#0891b2,color:#fff
Last reviewed commit: 5351b62 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/shared/pathUtils.ts (1)
203-215:⚠️ Potential issue | 🟡 MinorHandle expected filesystem errors explicitly instead of silently swallowing all errors.
The catch blocks at lines 213 and 246 ignore all filesystem failures without distinguishing between expected and unexpected errors. Filter for expected errors (ENOENT/ENOTDIR when directories don't exist) and let unexpected errors propagate to callers for proper error tracking.
Suggested pattern (apply to both catch blocks)
- } catch { - // Ignore errors reading versions directory - } + } catch (error) { + const code = (error as NodeJS.ErrnoException)?.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') { + throw error; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/pathUtils.ts` around lines 203 - 215, The try/catch around reading versionsDir currently swallows all errors; change the catch to inspect the thrown error (from fs.readdirSync/fs.existsSync) and only ignore expected filesystem-not-found errors (err.code === 'ENOENT' or 'ENOTDIR'), rethrow any other errors so callers can observe unexpected failures; apply the same pattern to the other similar catch block and reference variables like versionsDir, detectedPaths, versionBin and the use of compareVersions when locating version directories.src/renderer/hooks/batch/useInlineWizard.ts (1)
628-677:⚠️ Potential issue | 🟡 MinorUpdate unsupported-agent message to match the new supported list.
Line 630 adds
gemini-cli(andopencodeis already supported), but the error string still says “Claude, Claude Code, or Codex.” This becomes misleading for users. Recommend updating the message (or deriving it fromsupportedWizardAgents) to list all supported agents.Suggested fix
- error: `The inline wizard is not supported for ${agentType}. Please use Claude, Claude Code, or Codex.`, + error: `The inline wizard is not supported for ${agentType}. Please use Claude Code, Codex, OpenCode, or Gemini CLI.`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/batch/useInlineWizard.ts` around lines 628 - 677, The error message for unsupported agents is outdated; update the string passed to setTabState and logger.warn to reflect the current supportedWizardAgents list (which includes 'claude-code', 'codex', 'opencode', 'gemini-cli') or dynamically derive the human-readable list from supportedWizardAgents; modify the block that handles unsupported agents (the else-if using supportedWizardAgents, logger.warn, and setTabState) so the user-facing message enumerates the actual supported agents instead of the hardcoded “Claude, Claude Code, or Codex.”src/cli/commands/list-sessions.ts (1)
16-70:⚠️ Potential issue | 🟠 MajorAdd conditional routing for gemini-cli session listing.
Line 16 adds
gemini-clitoSUPPORTED_TYPES, but the implementation unconditionally routes all session listings throughlistClaudeSessions(line 66). This function is Claude-specific—it reads from~/.claude/projects/usingencodeClaudeProjectPath(). No Gemini-aware listing helper exists. Gemini sessions cannot be retrieved this way and will return empty results or fail silently.Add a conditional check: if
agent.toolType === 'gemini-cli', route to a Gemini-aware listing helper (or create one if it doesn't exist). Otherwise, Gemini agents added toSUPPORTED_TYPESwill appear to work but produce incorrect results.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/list-sessions.ts` around lines 16 - 70, SUPPORTED_TYPES was extended to include 'gemini-cli' but listSessions still always calls listClaudeSessions; update listSessions to branch on agent.toolType and call a Gemini-aware listing helper for 'gemini-cli' (e.g., listGeminiSessions) while keeping listClaudeSessions for 'claude-code'. If listGeminiSessions doesn't exist, add it (or a wrapper) to perform Gemini-specific project path encoding and session enumeration (analogous to encodeClaudeProjectPath/use of projectPath), and ensure error/json output behavior remains the same when routing to the new helper.src/main/group-chat/group-chat-agent.ts (1)
196-241:⚠️ Potential issue | 🟠 MajorGemini include-directories are dropped for SSH sessions.
finalArgsincludes the Gemini workspace approvals, but SSH wrapping usesconfigResolution.args, so remote spawns lose those flags. If Gemini runs via SSH, sandbox access to the cwd/group chat dir can fail. Consider passingfinalArgsintowrapSpawnWithSsh(or re-deriving Gemini args post-wrap) to keep local/SSH behavior consistent.🐛 Suggested fix to preserve Gemini include-directories when SSH wrapping
- const sshWrapped = await wrapSpawnWithSsh( - { - command, - args: configResolution.args, + const sshWrapped = await wrapSpawnWithSsh( + { + command, + args: finalArgs, cwd, prompt, customEnvVars: configResolution.effectiveCustomEnvVars ?? effectiveEnvVars, promptArgs: agentConfig?.promptArgs, noPromptSeparator: agentConfig?.noPromptSeparator, agentBinaryName: agentConfig?.binaryName, }, sessionOverrides.sshRemoteConfig, sshStore );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-agent.ts` around lines 196 - 241, The SSH wrapping call is using configResolution.args (losing gemini include-directory flags computed into finalArgs), so remote spawns drop workspace approvals; update the wrapSpawnWithSsh invocation to pass finalArgs (or re-derive geminiDirArgs and append them after wrap) instead of configResolution.args so the wrapped spawn receives the same args (refer to finalArgs, wrapSpawnWithSsh, and configResolution.args and update the args parameter accordingly).src/renderer/components/Wizard/screens/AgentSelectionScreen.tsx (1)
38-80:⚠️ Potential issue | 🟡 MinorComment doesn’t match tile order.
The comment says Gemini CLI is “shown first,” but it appears last among supported tiles. Update the comment or reorder the array to match the intended UI.🔧 Suggested comment-only fix
- * Supported agents: Claude Code, Codex, OpenCode, Factory Droid, Gemini CLI (shown first) + * Supported agents: Claude Code, Codex, OpenCode, Factory Droid, Gemini CLI🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Wizard/screens/AgentSelectionScreen.tsx` around lines 38 - 80, The file-level comment for AGENT_TILES incorrectly states "Gemini CLI (shown first)" while the AGENT_TILES array lists 'gemini-cli' last; either update the comment to reflect the current ordering or move the 'gemini-cli' object (id: 'gemini-cli', name: 'Gemini CLI') to the first position in the supported agents list so the comment and the AGENT_TILES constant match; make the change in the AGENT_TILES declaration and keep the rest of the tile objects unchanged.
🧹 Nitpick comments (4)
src/renderer/components/Wizard/services/conversationManager.ts (1)
470-491: Remove raw bufferconsole.login the renderer.
These logs dump user/agent content to the dev console on success; prefer existingwizardDebugLogger(or guard behind a debug-only flag) to avoid noise and data exposure.🧹 Suggested change
- console.log('[WizardConversation] Exit code 0 — parsing response', { - agentType: this.session?.agentType, - outputBufferLength: this.session?.outputBuffer?.length || 0, - outputBufferPreview: this.session?.outputBuffer?.slice(0, 200) || '(empty)', - thinkingBufferLength: this.session?.thinkingBuffer?.length || 0, - thinkingBufferPreview: this.session?.thinkingBuffer?.slice(0, 200) || '(empty)', - }); - const parsedResponse = this.parseAgentOutput(); - - // DEBUG: Show parsed result - console.log('[WizardConversation] Parsed response', { - parseSuccess: parsedResponse.parseSuccess, - hasStructured: !!parsedResponse.structured, - confidence: parsedResponse.structured?.confidence, - ready: parsedResponse.structured?.ready, - messageLength: parsedResponse.structured?.message?.length || 0, - messagePreview: parsedResponse.structured?.message?.slice(0, 200) || '(none)', - rawTextLength: parsedResponse.rawText?.length || 0, - rawTextPreview: parsedResponse.rawText?.slice(0, 200) || '(empty)', - });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Wizard/services/conversationManager.ts` around lines 470 - 491, Replace the two raw console.log calls that print full buffers in the Gemini wizard response path with the existing wizardDebugLogger (or guard them behind a debug-only flag like this.wizardDebugEnabled); call wizardDebugLogger.debug('[WizardConversation] ...', { agentType: this.session?.agentType, outputBufferLength: ..., outputBufferPreview: this.session?.outputBuffer?.slice(0,200) || '(empty)', thinkingBufferLength: ..., thinkingBufferPreview: ... }) and similarly for the parsedResponse log (use parsedResponse fields but avoid dumping full rawText/structured.message — keep only length and preview), or skip logging those previews entirely when debug is disabled; locate the logs around the parseAgentOutput() call and replace console.log usages with wizardDebugLogger or wrapped conditional checks.src/renderer/components/NewGroupChatModal.tsx (1)
357-361: Beta detection logic duplicated.This beta agent check mirrors the pattern in
AgentSelector.tsx(lines 88-92). Both locations will need updates when agents graduate from beta. Consider extracting to a shared constant likeBETA_AGENT_IDSin a common location.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/NewGroupChatModal.tsx` around lines 357 - 361, The beta detection is duplicated (see the isBeta check using tile.id in NewGroupChatModal and the similar logic in AgentSelector); extract the list of beta agent IDs into a single exported constant like BETA_AGENT_IDS in a shared module (e.g., constants or config) and update both the isBeta check in NewGroupChatModal (currently computing isBeta from tile.id) and the corresponding check in AgentSelector to reference BETA_AGENT_IDS.includes(tile.id), ensuring both components import the shared constant.src/main/process-manager/types.ts (1)
109-115: Consider removingsessionIdfromGeminiSessionStatsEvent.The
sessionIdfield inGeminiSessionStatsEventis redundant since it's already passed as the first argument to the event handler (line 129). Other events likeusage,agent-error, andtool-executiondon't includesessionIdin their payload types since it's provided separately.♻️ Optional: Remove redundant sessionId
export interface GeminiSessionStatsEvent { - sessionId: string; inputTokens: number; outputTokens: number; cacheReadTokens: number; reasoningTokens: number; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/types.ts` around lines 109 - 115, Remove the redundant sessionId property from the GeminiSessionStatsEvent interface and update all places that construct or consume that event to pass sessionId as the separate event argument (as other events do). Specifically, edit the GeminiSessionStatsEvent type to drop sessionId, then update any emit/dispatch sites that build GeminiSessionStatsEvent payloads (search for GeminiSessionStatsEvent and usages like the event emitter/handler that sends stats) to stop including sessionId in the payload and ensure the sessionId is passed as the first argument to the event handler; also update any type annotations or tests that expect payload.sessionId to reference the outer sessionId parameter instead.src/__tests__/cli/services/agent-spawner.test.ts (1)
682-721: Consider adding PATH fallback test for Gemini detection.The tests cover custom path detection and the unavailable case, but they don't test the PATH fallback scenario (when custom path is invalid but Gemini is found in PATH). The
detectClaudesuite above includes this case at line 523-550. Adding a similar test would ensure parity.🧪 Optional: Add PATH fallback test
it('should fall back to PATH detection when custom path is invalid', async () => { mockGetAgentCustomPath.mockImplementation((agentId: string) => { if (agentId === 'gemini-cli') { return '/invalid/path/to/gemini'; } return undefined; }); vi.mocked(fs.promises.stat).mockRejectedValue(new Error('ENOENT')); mockSpawn.mockReturnValue(mockChild); const { detectGemini } = await import('../../../cli/services/agent-spawner'); const resultPromise = detectGemini(); await new Promise((resolve) => setTimeout(resolve, 0)); mockStdout.emit('data', Buffer.from('/usr/local/bin/gemini\n')); await new Promise((resolve) => setTimeout(resolve, 0)); mockChild.emit('close', 0); const result = await resultPromise; expect(result.available).toBe(true); expect(result.path).toBe('/usr/local/bin/gemini'); expect(result.source).toBe('path'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/cli/services/agent-spawner.test.ts` around lines 682 - 721, Add a PATH-fallback unit test for detectGemini mirroring the detectClaude PATH case: mockGetAgentCustomPath to return an invalid custom path for 'gemini-cli', make vi.mocked(fs.promises.stat) reject (ENOENT) to simulate missing file, ensure mockSpawn returns mockChild, simulate the spawned process writing a PATH result to mockStdout (e.g. '/usr/local/bin/gemini\n') and then emit mockChild.close(0), then await detectGemini() and assert result.available === true, result.path === '/usr/local/bin/gemini' and result.source === 'path'; reference functions/fixtures detectGemini, mockGetAgentCustomPath, fs.promises.stat, mockSpawn, mockStdout, and mockChild to locate where to add the test.
🤖 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/agents/capabilities.ts`:
- Around line 199-225: The comment for the 'gemini-cli' capability is incorrect:
the supportsReadOnlyMode comment lists an invalid value ("plan"); update the
comment for the supportsReadOnlyMode entry (in the 'gemini-cli' object) to
reflect the actual approval-mode values ("default", "auto_edit", "yolo"), or if
you determine the readonly behavior isn't applicable in this version, set
supportsReadOnlyMode to false instead; locate the 'gemini-cli' object and change
either the boolean or the inline comment on supportsReadOnlyMode accordingly.
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 903-910: The code builds gemini CLI --include-directories from
local paths (cwd, groupChatFolder, os.homedir()) via buildGeminiWorkspaceDirArgs
and then passes them into wrapSpawnWithSsh, which allows local-only paths to
reach the remote host; update the logic in the spots that call
buildGeminiWorkspaceDirArgs (references: buildGeminiWorkspaceDirArgs usage near
where participantFinalArgs is created and the similar block later) to consult
matchingSession.sshRemoteConfig?.workingDirOverride when SSH is active and use
that as the base instead of local cwd, or map local paths to their remote
equivalents (or omit os.homedir() when remote execution is enabled); ensure the
wrapSpawnWithSsh call receives normalized/remote-resolvable paths so remote
Gemini CLI won’t get local-only paths.
In `@src/main/ipc/handlers/agentSessions.ts`:
- Around line 282-310: The current try/catch in the session-parsing block
silently swallows JSON parse errors; instead import and call the Sentry
utilities (captureException or captureMessage) from 'src/utils/sentry' in the
catch so parse failures are reported while still falling back to zeroed stats.
Specifically, wrap the JSON.parse and the loop that uses session.messages,
accumulateGeminiTokens, messageCount, inputTokens, outputTokens and
cachedInputTokens as now, but in the catch call captureException(err, { extra: {
content } }) or captureMessage with context so the error and the raw content are
logged to Sentry; do not rethrow unless higher-level code expects it.
- Around line 238-262: In accumulateGeminiTokens, change the input accumulation
to sum relevant token fields instead of using the || chain so we don't drop
coexisting values; specifically, compute input by preferring a canonical token
field (e.g., input_tokens) if present, otherwise sum obj.input + obj.prompt and
also include promptTokens/inputTokens variants as needed (use asNumber for each
and default to 0); update the input assignment in accumulateGeminiTokens
accordingly and leave output/cached logic unchanged.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 11-40: extractDeniedPath currently only matches POSIX/tilde paths
so Windows paths like "C:\Users\..." are missed; update the function
(extractDeniedPath) to accept Windows paths by allowing drive-letter prefixes
and backslashes in the regexes or by normalizing backslashes to forward slashes
before matching, ensure the file-extension check (/\.\w+$/) and parent-directory
extraction (lastIndexOf('/')) work after normalization, and add a pattern or
branch to handle UNC paths (\\\\server\\share) so Windows sandbox denial
messages are correctly extracted and returned.
In `@src/main/storage/gemini-session-storage.ts`:
- Around line 665-688: The removal range currently sets endIndex =
userMessageIndex + 1 when no Gemini reply is found, which leaves intermediate
info/warning messages orphaned; change the scan so it records the index where
the loop stopped (the next 'user' message or end of array) and use that as
endIndex. Concretely, add a scanEnd (or reuse pairedResponseIndex semantics)
updated inside the for-loop for each visited index (e.g., scanEnd = i + 1) and
if you hit a 'gemini' set pairedResponseIndex and break, or if you hit a 'user'
break leaving scanEnd pointing to the first subsequent user; then compute
endIndex = pairedResponseIndex !== -1 ? pairedResponseIndex + 1 : scanEnd and
set removedCount = endIndex - userMessageIndex so all intermediate info/warning
messages are included when no Gemini reply exists.
- Around line 295-301: The code that computes startedAt/lastActiveAt and then
startTime/endTime/durationSeconds (variables session.startTime,
session.lastUpdated, startedAt, lastActiveAt, startTime, endTime,
durationSeconds) can produce NaN if the JSON timestamps are malformed; update
the logic to validate parsed times using Date.parse() (or new
Date(...).getTime()) and if parsing yields NaN fallback to stats.mtimeMs (or new
Date(stats.mtimeMs).toISOString() for the startedAt/lastActiveAt strings), then
compute startTime/endTime from those validated values and derive durationSeconds
with Math.max(0, Math.floor((endTime - startTime)/1000)); ensure you explicitly
check isNaN(startTime) || isNaN(endTime) and replace with fallback values before
calculating durationSeconds.
- Around line 199-240: Update error handling so expected "not found" cases
remain silent but unexpected failures (especially JSON parsing and permission/IO
errors) are reported to Sentry using captureException: in getHistoryDir and
findSessionFiles keep the existing silent catches for missing dirs/files; in
searchSessions and findSessionFile wrap JSON.parse and any readFile logic so
parse errors and other unexpected exceptions call captureException(error,
{contexts: {filePath, function: 'searchSessions'|'findSessionFile'}}) and then
continue/return as before; in deleteMessagePair leave the fire-and-forget
cleanup silent. Ensure you reference the functions getHistoryDir,
findSessionFiles, searchSessions, deleteMessagePair, and findSessionFile when
placing the captureException calls and include the offending file path or
session id in the context.
In `@src/renderer/App.tsx`:
- Around line 393-418: In onApproveWorkspaceDir replace the current blind
.catch(console.warn) on window.maestro.process.kill with explicit Sentry
reporting and selective handling: import captureException and/or captureMessage
from src/utils/sentry, then in the catch inspect the error and only suppress
known/expected error cases (e.g., "process already exited"/specific error code)
while for all other errors call captureException(error, { extra: {
processSessionId, sessionId, activeTabId } }) (and optionally show a user-facing
modal via closeModal/useModalStore) or rethrow so they don’t silently fail;
update the handler around window.maestro.process.kill in onApproveWorkspaceDir
to perform this behavior using the processSessionId and session/context info
already available.
In `@src/renderer/components/AppModals.tsx`:
- Around line 1706-1721: The WorkspaceApprovalModal currently reads sshRemoteId
from session.sshRemoteId which can be undefined for SSH-backed sessions; change
the sshRemoteId prop so it falls back to session.sessionSshRemoteConfig.remoteId
(i.e., locate the session via sessions.find(s => s.id ===
workspaceApprovalData.sessionId) and pass session.sshRemoteId ||
session.sessionSshRemoteConfig?.remoteId) to ensure remote directory operations
use the correct SSH remote; keep all other props (sessionName, onApprove/onDeny)
unchanged.
In `@src/renderer/components/Wizard/services/conversationManager.ts`:
- Around line 804-829: The extractResultFromStreamJson function is concatenating
msg.content raw (which may be an array) causing “[object Object]” output; update
the Gemini CLI branch in extractResultFromStreamJson to normalize content to a
string before pushing: if msg.content is a string use it, if it's an array
iterate its elements and join their text fields (or call the existing helper
extractGeminiContent used in storage) and only then push the resulting string
into textParts so concatenation produces the intended text; apply the same
normalization logic to other similar consumers
(inlineWizardConversation/inlineWizardDocumentGeneration) as noted.
In `@src/renderer/services/contextGroomer.ts`:
- Around line 155-161: The description for the 'gemini-cli' entry incorrectly
implies the CLI enforces a "plan (read-only)" mode; update the string associated
with 'gemini-cli' in contextGroomer (the 'gemini-cli' key) to rephrase that mode
to indicate it is intended as read-only or "plan" mode but not enforced by this
PR (e.g., "plan (intended read-only; CLI flag not wired here)"), so the grooming
prompt is not misled about enforced behavior.
In `@src/renderer/services/inlineWizardDocumentGeneration.ts`:
- Around line 636-652: The gemini-cli branch currently only checks for the
presence of "--output-format" as a separate arg and can miss
"--output-format=..." forms, causing duplicate/conflicting flags; update the
logic in the 'gemini-cli' case that builds args (the args array and the
agentWithBatch handling) to first remove any existing output-format flags in
both forms (e.g., "--output-format" followed by a value and
"--output-format=...") from args, then unconditionally push the normalized
"--output-format", "stream-json" pair, and finally append
agentWithBatch.batchModeArgs if present so stream-json is enforced without
duplicates.
---
Outside diff comments:
In `@src/cli/commands/list-sessions.ts`:
- Around line 16-70: SUPPORTED_TYPES was extended to include 'gemini-cli' but
listSessions still always calls listClaudeSessions; update listSessions to
branch on agent.toolType and call a Gemini-aware listing helper for 'gemini-cli'
(e.g., listGeminiSessions) while keeping listClaudeSessions for 'claude-code'.
If listGeminiSessions doesn't exist, add it (or a wrapper) to perform
Gemini-specific project path encoding and session enumeration (analogous to
encodeClaudeProjectPath/use of projectPath), and ensure error/json output
behavior remains the same when routing to the new helper.
In `@src/main/group-chat/group-chat-agent.ts`:
- Around line 196-241: The SSH wrapping call is using configResolution.args
(losing gemini include-directory flags computed into finalArgs), so remote
spawns drop workspace approvals; update the wrapSpawnWithSsh invocation to pass
finalArgs (or re-derive geminiDirArgs and append them after wrap) instead of
configResolution.args so the wrapped spawn receives the same args (refer to
finalArgs, wrapSpawnWithSsh, and configResolution.args and update the args
parameter accordingly).
In `@src/renderer/components/Wizard/screens/AgentSelectionScreen.tsx`:
- Around line 38-80: The file-level comment for AGENT_TILES incorrectly states
"Gemini CLI (shown first)" while the AGENT_TILES array lists 'gemini-cli' last;
either update the comment to reflect the current ordering or move the
'gemini-cli' object (id: 'gemini-cli', name: 'Gemini CLI') to the first position
in the supported agents list so the comment and the AGENT_TILES constant match;
make the change in the AGENT_TILES declaration and keep the rest of the tile
objects unchanged.
In `@src/renderer/hooks/batch/useInlineWizard.ts`:
- Around line 628-677: The error message for unsupported agents is outdated;
update the string passed to setTabState and logger.warn to reflect the current
supportedWizardAgents list (which includes 'claude-code', 'codex', 'opencode',
'gemini-cli') or dynamically derive the human-readable list from
supportedWizardAgents; modify the block that handles unsupported agents (the
else-if using supportedWizardAgents, logger.warn, and setTabState) so the
user-facing message enumerates the actual supported agents instead of the
hardcoded “Claude, Claude Code, or Codex.”
In `@src/shared/pathUtils.ts`:
- Around line 203-215: The try/catch around reading versionsDir currently
swallows all errors; change the catch to inspect the thrown error (from
fs.readdirSync/fs.existsSync) and only ignore expected filesystem-not-found
errors (err.code === 'ENOENT' or 'ENOTDIR'), rethrow any other errors so callers
can observe unexpected failures; apply the same pattern to the other similar
catch block and reference variables like versionsDir, detectedPaths, versionBin
and the use of compareVersions when locating version directories.
---
Nitpick comments:
In `@src/__tests__/cli/services/agent-spawner.test.ts`:
- Around line 682-721: Add a PATH-fallback unit test for detectGemini mirroring
the detectClaude PATH case: mockGetAgentCustomPath to return an invalid custom
path for 'gemini-cli', make vi.mocked(fs.promises.stat) reject (ENOENT) to
simulate missing file, ensure mockSpawn returns mockChild, simulate the spawned
process writing a PATH result to mockStdout (e.g. '/usr/local/bin/gemini\n') and
then emit mockChild.close(0), then await detectGemini() and assert
result.available === true, result.path === '/usr/local/bin/gemini' and
result.source === 'path'; reference functions/fixtures detectGemini,
mockGetAgentCustomPath, fs.promises.stat, mockSpawn, mockStdout, and mockChild
to locate where to add the test.
In `@src/main/process-manager/types.ts`:
- Around line 109-115: Remove the redundant sessionId property from the
GeminiSessionStatsEvent interface and update all places that construct or
consume that event to pass sessionId as the separate event argument (as other
events do). Specifically, edit the GeminiSessionStatsEvent type to drop
sessionId, then update any emit/dispatch sites that build
GeminiSessionStatsEvent payloads (search for GeminiSessionStatsEvent and usages
like the event emitter/handler that sends stats) to stop including sessionId in
the payload and ensure the sessionId is passed as the first argument to the
event handler; also update any type annotations or tests that expect
payload.sessionId to reference the outer sessionId parameter instead.
In `@src/renderer/components/NewGroupChatModal.tsx`:
- Around line 357-361: The beta detection is duplicated (see the isBeta check
using tile.id in NewGroupChatModal and the similar logic in AgentSelector);
extract the list of beta agent IDs into a single exported constant like
BETA_AGENT_IDS in a shared module (e.g., constants or config) and update both
the isBeta check in NewGroupChatModal (currently computing isBeta from tile.id)
and the corresponding check in AgentSelector to reference
BETA_AGENT_IDS.includes(tile.id), ensuring both components import the shared
constant.
In `@src/renderer/components/Wizard/services/conversationManager.ts`:
- Around line 470-491: Replace the two raw console.log calls that print full
buffers in the Gemini wizard response path with the existing wizardDebugLogger
(or guard them behind a debug-only flag like this.wizardDebugEnabled); call
wizardDebugLogger.debug('[WizardConversation] ...', { agentType:
this.session?.agentType, outputBufferLength: ..., outputBufferPreview:
this.session?.outputBuffer?.slice(0,200) || '(empty)', thinkingBufferLength:
..., thinkingBufferPreview: ... }) and similarly for the parsedResponse log (use
parsedResponse fields but avoid dumping full rawText/structured.message — keep
only length and preview), or skip logging those previews entirely when debug is
disabled; locate the logs around the parseAgentOutput() call and replace
console.log usages with wizardDebugLogger or wrapped conditional checks.
Comment on lines
199
to
225
| /** | ||
| * Gemini CLI - Google's Gemini model CLI | ||
| * Gemini CLI - Google's Gemini model CLI (v0.29.5) | ||
| * https://github.com/google-gemini/gemini-cli | ||
| * | ||
| * PLACEHOLDER: Most capabilities set to false until Gemini CLI is stable | ||
| * and can be tested. Update this configuration when integrating the agent. | ||
| * Verified capabilities based on Gemini CLI v0.29.5 flags and output format. | ||
| */ | ||
| 'gemini-cli': { | ||
| supportsResume: false, | ||
| supportsReadOnlyMode: false, | ||
| supportsJsonOutput: false, | ||
| supportsSessionId: false, | ||
| supportsImageInput: true, // Gemini supports multimodal | ||
| supportsImageInputOnResume: false, // Not yet investigated | ||
| supportsSlashCommands: false, | ||
| supportsSessionStorage: false, | ||
| supportsCostTracking: false, | ||
| supportsUsageStats: false, | ||
| supportsBatchMode: false, | ||
| requiresPromptToStart: false, // Not yet investigated | ||
| supportsStreaming: true, // Likely streams | ||
| supportsResultMessages: false, | ||
| supportsModelSelection: false, // Not yet investigated | ||
| supportsStreamJsonInput: false, | ||
| supportsThinkingDisplay: false, // Not yet investigated | ||
| supportsContextMerge: false, // Not yet investigated - PLACEHOLDER | ||
| supportsContextExport: false, // Not yet investigated - PLACEHOLDER | ||
| supportsResume: true, // --resume latest|index | ||
| supportsReadOnlyMode: true, // --approval-mode plan | ||
| supportsJsonOutput: true, // --output-format json|stream-json | ||
| supportsSessionId: true, // session_id in JSON output | ||
| supportsImageInput: false, // Gemini is multimodal but CLI has no image input flag | ||
| supportsImageInputOnResume: false, // No image flag | ||
| supportsSlashCommands: false, // Gemini uses /slash commands but not exposed in JSON | ||
| supportsSessionStorage: true, // ~/.gemini/tmp/project/chats/ | ||
| supportsCostTracking: false, // Free tier / no cost data in output | ||
| supportsUsageStats: true, // Token stats in JSON output | ||
| supportsBatchMode: true, // -p flag for non-interactive | ||
| requiresPromptToStart: true, // Needs -p for batch mode | ||
| supportsStreaming: true, // stream-json output | ||
| supportsResultMessages: true, // 'result' event in stream-json | ||
| supportsModelSelection: true, // -m/--model flag | ||
| supportsStreamJsonInput: false, // No stdin JSON streaming | ||
| supportsThinkingDisplay: true, // Tracks thought tokens | ||
| supportsContextMerge: true, // Can receive transferred context | ||
| supportsContextExport: true, // Can export context for transfer | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Gemini CLI v0.29.5 documentation flags --resume --approval-mode -p model
💡 Result:
For Gemini CLI v0.29.5, these flags are documented as follows:
-
--resume/-r(string): Resume a previous session.- Resume most recent:
gemini --resumeorgemini --resume latest - Resume by index:
gemini --resume 5 - Resume by UUID:
gemini --resume <session-uuid>[1][2]
- Resume most recent:
-
--approval-mode(string): Sets how Gemini CLI asks for (or skips) approval when running tools.- Values:
default,auto_edit,yolo - Note:
--yolo/-yis deprecated; use--approval-mode=yoloinstead. [2][3]
- Values:
-
-p/--prompt(string): Provide a prompt and run in non-interactive/headless mode (run prompt, print output, exit). Example:gemini -p "query"[3] -
--model/-m(string): Choose the Gemini model for the session. Example:gemini -p "query" -m gemini-2.5-flash[3][4]
Example combining them:
gemini --resume latest --approval-mode auto_edit -m gemini-2.5-pro -p "Update the README with new install steps"Sources: [1][2][3][4]
Fix supportsReadOnlyMode comment — approval-mode value is incorrect.
Verification of Gemini CLI v0.29.5 flags confirms:
--resume latest|index✓-p(non-interactive/headless mode) ✓--model/-m✓
However, the approval-mode comment is wrong. Valid values are default, auto_edit, yolo — not plan. Either update the comment to reflect actual values or disable supportsReadOnlyMode if the "plan" mode behavior doesn't exist in this version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/agents/capabilities.ts` around lines 199 - 225, The comment for the
'gemini-cli' capability is incorrect: the supportsReadOnlyMode comment lists an
invalid value ("plan"); update the comment for the supportsReadOnlyMode entry
(in the 'gemini-cli' object) to reflect the actual approval-mode values
("default", "auto_edit", "yolo"), or if you determine the readonly behavior
isn't applicable in this version, set supportsReadOnlyMode to false instead;
locate the 'gemini-cli' object and change either the boolean or the inline
comment on supportsReadOnlyMode accordingly.
Comment on lines
+238
to
+262
| function accumulateGeminiTokens(source: unknown): GeminiTokenAccumulator { | ||
| if (!source || typeof source !== 'object') { | ||
| return { input: 0, output: 0, cached: 0 }; | ||
| } | ||
| const obj = source as Record<string, unknown>; | ||
| const input = | ||
| asNumber(obj.input) || | ||
| asNumber(obj.prompt) || | ||
| asNumber(obj.promptTokens) || | ||
| asNumber(obj.inputTokens) || | ||
| asNumber(obj.input_tokens); | ||
| const output = | ||
| asNumber(obj.output) || | ||
| asNumber(obj.completion) || | ||
| asNumber(obj.outputTokens) || | ||
| asNumber(obj.output_tokens) || | ||
| asNumber(obj.responseTokens); | ||
| const cached = | ||
| asNumber(obj.cached) || | ||
| asNumber(obj.cacheRead) || | ||
| asNumber(obj.cache_read) || | ||
| asNumber(obj.cachedInputTokens) || | ||
| asNumber(obj.cached_input_tokens); | ||
| return { input, output, cached }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the file and understand the context
fd -t f "agentSessions.ts" --type fRepository: RunMaestro/Maestro
Length of output: 100
🏁 Script executed:
# Let's look at the function and surrounding context
cat -n src/main/ipc/handlers/agentSessions.ts | head -n 270 | tail -n 50Repository: RunMaestro/Maestro
Length of output: 1741
🏁 Script executed:
# Search for GeminiTokenAccumulator type definition
rg "GeminiTokenAccumulator" -A 5 -B 5Repository: RunMaestro/Maestro
Length of output: 1288
🏁 Script executed:
# Look for Gemini-related code and how tokens are processed
rg "accumulateGeminiTokens" -B 5 -A 10Repository: RunMaestro/Maestro
Length of output: 2245
🏁 Script executed:
# Check for any tests or documentation about token accumulation
fd -t f "*.test.ts" -o "*.spec.ts" | xargs rg -l "accumulateGemini|GeminiToken" 2>/dev/null || echo "No tests found"Repository: RunMaestro/Maestro
Length of output: 255
🏁 Script executed:
# Search for stream-json or Gemini API references in documentation
rg "stream.*json|gemini.*token" -i --type md --type txtRepository: RunMaestro/Maestro
Length of output: 775
🏁 Script executed:
# Look at more context around how token sources are created and populated
cat -n src/main/ipc/handlers/agentSessions.ts | head -n 330 | tail -n 80Repository: RunMaestro/Maestro
Length of output: 2949
🏁 Script executed:
# Search for Gemini API or token schema information
rg "tokens|tokenUsage|tokenCounts" -B 3 -A 3 | head -100Repository: RunMaestro/Maestro
Length of output: 5963
🏁 Script executed:
# Look for type definitions related to message tokens
rg "interface.*[Tt]oken|type.*[Tt]oken" --type ts | grep -i geminiRepository: RunMaestro/Maestro
Length of output: 759
🏁 Script executed:
# Check if there are any imports from Gemini SDK or type definitions
rg "from.*gemini|import.*gemini" -iRepository: RunMaestro/Maestro
Length of output: 5125
🏁 Script executed:
# Look for how msg.tokens is populated - search in the same file
cat -n src/main/ipc/handlers/agentSessions.ts | grep -A 5 -B 5 "msg.tokens\|msg.tokenUsage"Repository: RunMaestro/Maestro
Length of output: 621
🏁 Script executed:
# Look at the test file for agentSessions
cat -n src/__tests__/main/ipc/handlers/agentSessions.test.ts | head -200Repository: RunMaestro/Maestro
Length of output: 7402
🏁 Script executed:
# Search for actual token data in tests
rg "tokens|prompt|input" src/__tests__/main/ipc/handlers/agentSessions.test.ts -B 2 -A 2 | head -150Repository: RunMaestro/Maestro
Length of output: 3433
🏁 Script executed:
# Look for Gemini output parser tests that show token structure
cat -n src/__tests__/main/parsers/gemini-output-parser.test.ts | head -300Repository: RunMaestro/Maestro
Length of output: 10332
🏁 Script executed:
# Check if there are any actual example Gemini session files
find . -name "*.json" -path "*test*" -o -name "*gemini*" | grep -i test | head -20Repository: RunMaestro/Maestro
Length of output: 248
🏁 Script executed:
# Search for actual Gemini API response structures in code
rg "prompt.*token|promptTokens" -B 3 -A 3Repository: RunMaestro/Maestro
Length of output: 7608
🏁 Script executed:
# Look at how Gemini stats are captured and stored
cat -n src/main/process-listeners/gemini-stats-listener.ts | head -100Repository: RunMaestro/Maestro
Length of output: 4578
🏁 Script executed:
# Check if there are any example Gemini session JSON files in tests
find . -type f -name "*.json" -path "*test*" | xargs grep -l "input.*prompt\|prompt.*input" 2>/dev/null | head -5Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Look at the GeminiOutputParser to see how it extracts stats
cat -n src/main/parsers/gemini-output-parser.ts | grep -A 30 "extractUsageFromStats"Repository: RunMaestro/Maestro
Length of output: 3157
🏁 Script executed:
# Check what structure token data in session files actually has
rg "msg.tokens|tokenUsage|tokenCounts" -B 5 -A 15 src/main/ipc/handlers/agentSessions.tsRepository: RunMaestro/Maestro
Length of output: 1202
🏁 Script executed:
# Look for any documentation or comments about Gemini session token structure
rg "session.*token|token.*session" -i -B 2 -A 2 src/main/storage/gemini-session-storage.ts | head -50Repository: RunMaestro/Maestro
Length of output: 44
Sum input and prompt tokens instead of picking first non-zero value.
The function uses || operator to select the first non-zero field, but input and prompt can both be present in the same metadata object and should be summed. In GeminiOutputParser.extractUsageFromStats() (lines 429-430), nested model stats explicitly sum these: totalInput += (tokens.input || 0) + (tokens.prompt || 0);. When session files contain both fields, the current logic would undercount by dropping one.
Change the input accumulation to:
const input = (asNumber(obj.input) || 0) + (asNumber(obj.prompt) || 0) + (asNumber(obj.promptTokens) || 0) + (asNumber(obj.inputTokens) || 0) + (asNumber(obj.input_tokens) || 0);Or, if only certain field combinations should sum (e.g., input + prompt pair, but others are aliases), prioritize input_tokens and fall back to input + prompt:
const input = asNumber(obj.input_tokens) || (asNumber(obj.input) || 0) + (asNumber(obj.prompt) || 0) || asNumber(obj.promptTokens) || asNumber(obj.inputTokens);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/handlers/agentSessions.ts` around lines 238 - 262, In
accumulateGeminiTokens, change the input accumulation to sum relevant token
fields instead of using the || chain so we don't drop coexisting values;
specifically, compute input by preferring a canonical token field (e.g.,
input_tokens) if present, otherwise sum obj.input + obj.prompt and also include
promptTokens/inputTokens variants as needed (use asNumber for each and default
to 0); update the input assignment in accumulateGeminiTokens accordingly and
leave output/cached logic unchanged.
Comment on lines
+804
to
+829
| * - Gemini: NDJSON with { type: 'message', role: 'assistant', content: '...' } | ||
| */ | ||
| private extractResultFromStreamJson(output: string): string | null { | ||
| const agentType = this.session?.agentType; | ||
|
|
||
| try { | ||
| const lines = output.split('\n'); | ||
|
|
||
| // For Gemini CLI: concatenate all assistant message content | ||
| if (agentType === 'gemini-cli') { | ||
| const textParts: string[] = []; | ||
| for (const line of lines) { | ||
| if (!line.trim()) continue; | ||
| try { | ||
| const msg = JSON.parse(line); | ||
| if (msg.type === 'message' && msg.role === 'assistant' && msg.content) { | ||
| textParts.push(msg.content); | ||
| } | ||
| } catch { | ||
| // Ignore non-JSON lines | ||
| } | ||
| } | ||
| if (textParts.length > 0) { | ||
| return textParts.join(''); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and examine the actual code at those lines
fd "conversationManager.ts" srcRepository: RunMaestro/Maestro
Length of output: 124
🏁 Script executed:
# Search for Gemini-related tests and fixtures
rg -i "gemini" src __tests__ -g "*.{ndjson,json,txt,ts,tsx,js}" --max-count 20Repository: RunMaestro/Maestro
Length of output: 50441
🏁 Script executed:
# Search for Gemini NDJSON message structure samples
rg '"type"\s*:\s*"message"' . -g "*.{ndjson,json,txt}"Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Search for gemini-cli specific handling in codebase
rg "gemini-cli" src __tests__ -g "*.{ts,tsx,js}" -A 5 -B 2Repository: RunMaestro/Maestro
Length of output: 50441
🏁 Script executed:
# Examine the specific file and lines in question
sed -n '800,835p' src/renderer/components/Wizard/services/conversationManager.tsRepository: RunMaestro/Maestro
Length of output: 1251
🏁 Script executed:
# Search for Gemini NDJSON test fixtures or example data
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.fixture*" -o -name "*.mock*" \) -exec grep -l "gemini.*message\|gemini.*content" {} \; | head -5Repository: RunMaestro/Maestro
Length of output: 177
🏁 Script executed:
# Look specifically for message structure examples in tests
rg "type.*message.*role.*assistant" . -A 3 -B 1 --max-count 10Repository: RunMaestro/Maestro
Length of output: 12777
🏁 Script executed:
# Search for extractResultFromStreamJson or similar methods that parse Gemini output
rg "extractResultFromStreamJson|extractGeminiContent" src -A 10 -B 2Repository: RunMaestro/Maestro
Length of output: 11380
🏁 Script executed:
# Find the GeminiContentPart interface definition
rg "interface GeminiContentPart" src -A 5Repository: RunMaestro/Maestro
Length of output: 404
🏁 Script executed:
# Check if there are any actual test fixtures or live examples showing content as array
rg "content.*\[\s*\{" src/__tests__ -g "*.ts" -B 2 -A 2Repository: RunMaestro/Maestro
Length of output: 9154
Normalize Gemini content field (string vs parts array) to prevent [object Object] in output.
Gemini NDJSON messages can have content as either a string OR an array of content parts: [{text: '...'}, {text: '...'}]. The current code directly pushes msg.content without checking the type—if content is an array, the result is [object Object] or empty output, breaking structured parsing.
This pattern is already correctly handled in src/main/storage/gemini-session-storage.ts via the extractGeminiContent() function. The same vulnerability also exists in src/renderer/services/inlineWizardConversation.ts and src/renderer/services/inlineWizardDocumentGeneration.ts.
Normalize content to string before concatenating:
Suggested fix
if (agentType === 'gemini-cli') {
const textParts: string[] = [];
+ const normalizeGeminiContent = (content: unknown): string => {
+ if (typeof content === 'string') return content;
+ if (Array.isArray(content)) {
+ return content
+ .map((part) =>
+ typeof part?.text === 'string' ? part.text : ''
+ )
+ .join('');
+ }
+ return '';
+ };
for (const line of lines) {
if (!line.trim()) continue;
try {
const msg = JSON.parse(line);
- if (msg.type === 'message' && msg.role === 'assistant' && msg.content) {
- textParts.push(msg.content);
- }
+ if (msg.type === 'message' && msg.role === 'assistant' && msg.content) {
+ const normalized = normalizeGeminiContent(msg.content);
+ if (normalized) textParts.push(normalized);
+ }
} catch {
// Ignore non-JSON lines
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * - Gemini: NDJSON with { type: 'message', role: 'assistant', content: '...' } | |
| */ | |
| private extractResultFromStreamJson(output: string): string | null { | |
| const agentType = this.session?.agentType; | |
| try { | |
| const lines = output.split('\n'); | |
| // For Gemini CLI: concatenate all assistant message content | |
| if (agentType === 'gemini-cli') { | |
| const textParts: string[] = []; | |
| for (const line of lines) { | |
| if (!line.trim()) continue; | |
| try { | |
| const msg = JSON.parse(line); | |
| if (msg.type === 'message' && msg.role === 'assistant' && msg.content) { | |
| textParts.push(msg.content); | |
| } | |
| } catch { | |
| // Ignore non-JSON lines | |
| } | |
| } | |
| if (textParts.length > 0) { | |
| return textParts.join(''); | |
| } | |
| } | |
| * - Gemini: NDJSON with { type: 'message', role: 'assistant', content: '...' } | |
| */ | |
| private extractResultFromStreamJson(output: string): string | null { | |
| const agentType = this.session?.agentType; | |
| try { | |
| const lines = output.split('\n'); | |
| // For Gemini CLI: concatenate all assistant message content | |
| if (agentType === 'gemini-cli') { | |
| const textParts: string[] = []; | |
| const normalizeGeminiContent = (content: unknown): string => { | |
| if (typeof content === 'string') return content; | |
| if (Array.isArray(content)) { | |
| return content | |
| .map((part) => | |
| typeof part?.text === 'string' ? part.text : '' | |
| ) | |
| .join(''); | |
| } | |
| return ''; | |
| }; | |
| for (const line of lines) { | |
| if (!line.trim()) continue; | |
| try { | |
| const msg = JSON.parse(line); | |
| if (msg.type === 'message' && msg.role === 'assistant' && msg.content) { | |
| const normalized = normalizeGeminiContent(msg.content); | |
| if (normalized) textParts.push(normalized); | |
| } | |
| } catch { | |
| // Ignore non-JSON lines | |
| } | |
| } | |
| if (textParts.length > 0) { | |
| return textParts.join(''); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/Wizard/services/conversationManager.ts` around lines
804 - 829, The extractResultFromStreamJson function is concatenating msg.content
raw (which may be an array) causing “[object Object]” output; update the Gemini
CLI branch in extractResultFromStreamJson to normalize content to a string
before pushing: if msg.content is a string use it, if it's an array iterate its
elements and join their text fields (or call the existing helper
extractGeminiContent used in storage) and only then push the resulting string
into textParts so concatenation produces the intended text; apply the same
normalization logic to other similar consumers
(inlineWizardConversation/inlineWizardDocumentGeneration) as noted.
Comment on lines
+155
to
+161
| 'gemini-cli': ` | ||
| Gemini CLI is Google's AI coding assistant powered by Gemini models. | ||
| It can read and edit files, run shell commands, search code, fetch web pages, and use Google Search. | ||
| It supports MCP (Model Context Protocol) servers for extensibility. | ||
| It has approval modes: default, auto_edit, yolo, and plan (read-only). | ||
| Models include gemini-2.5-pro, gemini-2.5-flash, and gemini-3 preview models. | ||
| It supports tool calling with 15+ built-in tools. |
There was a problem hiding this comment.
Clarify the “plan (read-only)” wording for Gemini CLI.
The notes imply enforced read‑only behavior, but the CLI read‑only flag isn’t wired yet in this PR. Consider rephrasing to avoid misleading the grooming prompt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/services/contextGroomer.ts` around lines 155 - 161, The
description for the 'gemini-cli' entry incorrectly implies the CLI enforces a
"plan (read-only)" mode; update the string associated with 'gemini-cli' in
contextGroomer (the 'gemini-cli' key) to rephrase that mode to indicate it is
intended as read-only or "plan" mode but not enforced by this PR (e.g., "plan
(intended read-only; CLI flag not wired here)"), so the grooming prompt is not
misled about enforced behavior.
Comment on lines
+636
to
+652
| case 'gemini-cli': { | ||
| // Gemini CLI requires stream-json output for structured response parsing | ||
| const args = [...(agent.args || [])]; | ||
|
|
||
| // Ensure stream-json output format for proper parsing | ||
| if (!args.includes('--output-format')) { | ||
| args.push('--output-format', 'stream-json'); | ||
| } | ||
|
|
||
| // Add auto-approve for batch mode | ||
| const agentWithBatch = agent as any; | ||
| if (agentWithBatch.batchModeArgs) { | ||
| args.push(...agentWithBatch.batchModeArgs); | ||
| } | ||
|
|
||
| return args; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Gemini CLI --output-format stream-json documentation
💡 Result:
Gemini CLI documents --output-format stream-json under Headless mode → Output formats → Streaming JSON output. It describes this mode as newline-delimited JSON (JSONL) events and lists event types like init, message, tool_use, tool_result, error, and result. [1]
It’s also referenced in the configuration reference, which notes stream-json as a supported output format alongside json for structured/scripting use. [2]
Example:
gemini -p "hello" --output-format stream-jsonSources: [1] [2]
🏁 Script executed:
# Check the code file to understand context
fd inlineWizardDocumentGeneration.ts -t f --exec cat -n {} \; | head -100Repository: RunMaestro/Maestro
Length of output: 3874
🏁 Script executed:
# Look for any Gemini CLI related information in the codebase
rg "gemini-cli" -A 5 -B 5Repository: RunMaestro/Maestro
Length of output: 50375
🏁 Script executed:
# Check for any existing output-format handling in the codebase
rg "output-format" -t ts -t tsxRepository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Get lines around 636-652 in the inlineWizardDocumentGeneration.ts file
fd inlineWizardDocumentGeneration.ts -type f -exec sed -n '620,660p' {} \;Repository: RunMaestro/Maestro
Length of output: 232
🏁 Script executed:
# Get lines around 636-652 in the inlineWizardDocumentGeneration.ts file
fd inlineWizardDocumentGeneration.ts -t f -exec sed -n '620,660p' {} \;Repository: RunMaestro/Maestro
Length of output: 292
🏁 Script executed:
# Find the file first, then read it
file_path=$(find . -name "inlineWizardDocumentGeneration.ts" -type f)
cat -n "$file_path" | sed -n '620,660p'Repository: RunMaestro/Maestro
Length of output: 1474
Normalize --output-format to avoid conflicting Gemini args.
The current check for --output-format only matches two-arg syntax (--output-format json) but misses single-arg syntax (--output-format=json). This can result in duplicate or conflicting flags. Since stream-json is required for structured parsing, strip any existing output-format arg (both syntaxes) before enforcing it.
🔧 Proposed fix
case 'gemini-cli': {
// Gemini CLI requires stream-json output for structured response parsing
- const args = [...(agent.args || [])];
+ const rawArgs = [...(agent.args || [])];
+ const args: string[] = [];
+ for (let i = 0; i < rawArgs.length; i++) {
+ const arg = rawArgs[i];
+ if (arg === '--output-format') {
+ // Skip flag and its value
+ i++;
+ continue;
+ }
+ if (arg.startsWith('--output-format=')) {
+ continue;
+ }
+ args.push(arg);
+ }
// Ensure stream-json output format for proper parsing
- if (!args.includes('--output-format')) {
- args.push('--output-format', 'stream-json');
- }
+ args.push('--output-format', 'stream-json');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case 'gemini-cli': { | |
| // Gemini CLI requires stream-json output for structured response parsing | |
| const args = [...(agent.args || [])]; | |
| // Ensure stream-json output format for proper parsing | |
| if (!args.includes('--output-format')) { | |
| args.push('--output-format', 'stream-json'); | |
| } | |
| // Add auto-approve for batch mode | |
| const agentWithBatch = agent as any; | |
| if (agentWithBatch.batchModeArgs) { | |
| args.push(...agentWithBatch.batchModeArgs); | |
| } | |
| return args; | |
| } | |
| case 'gemini-cli': { | |
| // Gemini CLI requires stream-json output for structured response parsing | |
| const rawArgs = [...(agent.args || [])]; | |
| const args: string[] = []; | |
| for (let i = 0; i < rawArgs.length; i++) { | |
| const arg = rawArgs[i]; | |
| if (arg === '--output-format') { | |
| // Skip flag and its value | |
| i++; | |
| continue; | |
| } | |
| if (arg.startsWith('--output-format=')) { | |
| continue; | |
| } | |
| args.push(arg); | |
| } | |
| // Ensure stream-json output format for proper parsing | |
| args.push('--output-format', 'stream-json'); | |
| // Add auto-approve for batch mode | |
| const agentWithBatch = agent as any; | |
| if (agentWithBatch.batchModeArgs) { | |
| args.push(...agentWithBatch.batchModeArgs); | |
| } | |
| return args; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/services/inlineWizardDocumentGeneration.ts` around lines 636 -
652, The gemini-cli branch currently only checks for the presence of
"--output-format" as a separate arg and can miss "--output-format=..." forms,
causing duplicate/conflicting flags; update the logic in the 'gemini-cli' case
that builds args (the args array and the agentWithBatch handling) to first
remove any existing output-format flags in both forms (e.g., "--output-format"
followed by a value and "--output-format=...") from args, then unconditionally
push the normalized "--output-format", "stream-json" pair, and finally append
agentWithBatch.batchModeArgs if present so stream-json is enforced without
duplicates.
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 (1)
src/main/group-chat/group-chat-router.ts (1)
1330-1348:⚠️ Potential issue | 🟠 MajorSynthesis spawn bypasses SSH wrapping.
Line 1330-1348 spawns directly with local
command/cwd/args; unlike the moderator and participant paths, this flow never callswrapSpawnWithSsh. Remote moderator sessions can diverge or fail during synthesis.💡 Suggested parity fix
+let spawnCommand = command; +let spawnArgs = finalArgs; +let spawnCwd = synthCwd; +let spawnPrompt: string | undefined = synthesisPrompt; +let spawnEnvVars = + configResolution.effectiveCustomEnvVars ?? + getCustomEnvVarsCallback?.(chat.moderatorAgentId); + +if (sshStore && chat.moderatorConfig?.sshRemoteConfig) { + const sshWrapped = await wrapSpawnWithSsh( + { + command, + args: finalArgs, + cwd: synthCwd, + prompt: synthesisPrompt, + customEnvVars: spawnEnvVars, + promptArgs: agent.promptArgs, + noPromptSeparator: agent.noPromptSeparator, + agentBinaryName: agent.binaryName, + }, + chat.moderatorConfig.sshRemoteConfig, + sshStore + ); + spawnCommand = sshWrapped.command; + spawnArgs = sshWrapped.args; + spawnCwd = sshWrapped.cwd; + spawnPrompt = sshWrapped.prompt; + spawnEnvVars = sshWrapped.customEnvVars; +} const spawnResult = processManager.spawn({ sessionId, toolType: chat.moderatorAgentId, - cwd: synthCwd, - command, - args: finalArgs, + cwd: spawnCwd, + command: spawnCommand, + args: spawnArgs, readOnlyMode: true, - prompt: synthesisPrompt, + prompt: spawnPrompt, contextWindow: getContextWindowValue(agent, agentConfigValues), - customEnvVars: - configResolution.effectiveCustomEnvVars ?? - getCustomEnvVarsCallback?.(chat.moderatorAgentId), + customEnvVars: spawnEnvVars,Based on learnings: When implementing features that spawn agent processes, support SSH remote execution by checking for sshRemoteConfig, using wrapSpawnWithSsh() from ssh-spawn-wrapper.ts, and passing the SSH store adapter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 1330 - 1348, The synthesis path currently calls processManager.spawn directly (the spawnResult block) and therefore skips SSH wrapping; modify the logic around the processManager.spawn call to detect sshRemoteConfig and, when present, call wrapSpawnWithSsh (from ssh-spawn-wrapper.ts) to transform the command, args, and cwd and supply the SSH store adapter before invoking processManager.spawn; ensure you pass the same options (sessionId, toolType, prompt, contextWindow, customEnvVars, promptArgs, noPromptSeparator, shell/runInShell/sendPromptViaStdin/sendPromptViaStdinRaw) into the wrapped spawn so synthesis uses the remote execution path just like moderator/participant flows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/main/group-chat/group-chat-agent.test.ts`:
- Around line 614-620: The SSH path assertion can pass vacuously when there are
zero --include-directories flags; before asserting allDirPaths entries equal
'/home/remoteuser/project' and not contain os.homedir(), add an explicit check
that includeDirIndices (or allDirPaths) is non-empty — e.g., assert
includeDirIndices.length > 0 — so the subsequent equality and notContain checks
in the group chat agent test (references: includeDirIndices, sshCallArgs,
allDirPaths) actually validate real entries rather than succeeding with an empty
array.
In `@src/__tests__/main/group-chat/group-chat-router.test.ts`:
- Around line 945-951: The test currently allows a false positive when there are
zero --include-directories entries; before checking that allDirPaths.every(...)
and that it does not contain os.homedir(), add an explicit assertion that the
expected number of include-directory entries are present (e.g. assert
includeDirIndices.length > 0 or assert allDirPaths.length === expectedCount) so
the subsequent checks actually validate real entries; reference
includeDirIndices, sshCallArgs and allDirPaths when adding the count/assertion.
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 513-518: The code currently appends '--no-sandbox' unconditionally
for Gemini by setting geminiNoSandbox and building finalArgs; change this to
only add '--no-sandbox' when there is an active read-only guard. Update the
logic around geminiNoSandbox/finalArgs to check the read-only indicator (e.g.,
verify chat.readOnlyMode is true or that agent.readOnlyArgs /
configResolution.args contains the read-only flags) before setting
geminiNoSandbox when chat.moderatorAgentId === 'gemini-cli', so
moderator/synthesis runs unsandboxed only if a read-only guard is present.
---
Outside diff comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 1330-1348: The synthesis path currently calls processManager.spawn
directly (the spawnResult block) and therefore skips SSH wrapping; modify the
logic around the processManager.spawn call to detect sshRemoteConfig and, when
present, call wrapSpawnWithSsh (from ssh-spawn-wrapper.ts) to transform the
command, args, and cwd and supply the SSH store adapter before invoking
processManager.spawn; ensure you pass the same options (sessionId, toolType,
prompt, contextWindow, customEnvVars, promptArgs, noPromptSeparator,
shell/runInShell/sendPromptViaStdin/sendPromptViaStdinRaw) into the wrapped
spawn so synthesis uses the remote execution path just like
moderator/participant flows.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/__tests__/main/group-chat/group-chat-agent.test.tssrc/__tests__/main/group-chat/group-chat-router.test.tssrc/main/group-chat/group-chat-agent.tssrc/main/group-chat/group-chat-router.ts
Comment on lines
+614
to
+620
| // All --include-directories paths should be the remote cwd only | ||
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | ||
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | ||
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | ||
| // Should NOT contain local home directory or local config paths | ||
| expect(allDirPaths).not.toContain(os.homedir()); | ||
| }); |
There was a problem hiding this comment.
Avoid vacuous success in the SSH path assertion.
Line 617 can evaluate true with zero --include-directories entries. Assert at least one include-dir flag before checking path equality.
💡 Suggested test hardening
const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]);
+expect(includeDirIndices.length).toBeGreaterThan(0);
expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // All --include-directories paths should be the remote cwd only | |
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | |
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | |
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | |
| // Should NOT contain local home directory or local config paths | |
| expect(allDirPaths).not.toContain(os.homedir()); | |
| }); | |
| // All --include-directories paths should be the remote cwd only | |
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | |
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | |
| expect(includeDirIndices.length).toBeGreaterThan(0); | |
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | |
| // Should NOT contain local home directory or local config paths | |
| expect(allDirPaths).not.toContain(os.homedir()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/group-chat/group-chat-agent.test.ts` around lines 614 -
620, The SSH path assertion can pass vacuously when there are zero
--include-directories flags; before asserting allDirPaths entries equal
'/home/remoteuser/project' and not contain os.homedir(), add an explicit check
that includeDirIndices (or allDirPaths) is non-empty — e.g., assert
includeDirIndices.length > 0 — so the subsequent equality and notContain checks
in the group chat agent test (references: includeDirIndices, sshCallArgs,
allDirPaths) actually validate real entries rather than succeeding with an empty
array.
Comment on lines
+945
to
+951
| // All --include-directories paths should be the remote cwd only | ||
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | ||
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | ||
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | ||
| // Should NOT contain the local home directory or local config paths | ||
| expect(allDirPaths).not.toContain(os.homedir()); | ||
| }); |
There was a problem hiding this comment.
Strengthen the SSH include-directories assertion.
Line 948 can pass even when no --include-directories entries exist. Add an explicit count/assertion before the every(...) check.
💡 Suggested test hardening
const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]);
+expect(includeDirIndices.length).toBeGreaterThan(0);
expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // All --include-directories paths should be the remote cwd only | |
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | |
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | |
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | |
| // Should NOT contain the local home directory or local config paths | |
| expect(allDirPaths).not.toContain(os.homedir()); | |
| }); | |
| // All --include-directories paths should be the remote cwd only | |
| // (buildAgentArgs adds one, buildGeminiWorkspaceDirArgs adds another for cwd) | |
| const allDirPaths = includeDirIndices.map((i: number) => sshCallArgs[i + 1]); | |
| expect(includeDirIndices.length).toBeGreaterThan(0); | |
| expect(allDirPaths.every((p: string) => p === '/home/remoteuser/project')).toBe(true); | |
| // Should NOT contain the local home directory or local config paths | |
| expect(allDirPaths).not.toContain(os.homedir()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/group-chat/group-chat-router.test.ts` around lines 945 -
951, The test currently allows a false positive when there are zero
--include-directories entries; before checking that allDirPaths.every(...) and
that it does not contain os.homedir(), add an explicit assertion that the
expected number of include-directory entries are present (e.g. assert
includeDirIndices.length > 0 or assert allDirPaths.length === expectedCount) so
the subsequent checks actually validate real entries; reference
includeDirIndices, sshCallArgs and allDirPaths when adding the count/assertion.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/renderer/components/Wizard/services/conversationManager.ts (1)
822-847:⚠️ Potential issue | 🟠 MajorNormalize Gemini
contentfield (string vs parts array) to prevent[object Object]in output.Gemini NDJSON messages can have
contentas either a string OR an array of content parts:[{text: '...'}, {text: '...'}]. The current code directly pushesmsg.contentwithout checking the type—if content is an array,deltaParts.join('')orcompleteParts.join('')will produce[object Object], breaking the parsed output.🐛 Proposed fix to normalize content
if (agentType === 'gemini-cli') { const deltaParts: string[] = []; const completeParts: string[] = []; + const normalizeContent = (content: unknown): string => { + if (typeof content === 'string') return content; + if (Array.isArray(content)) { + return content + .map((part) => (typeof part?.text === 'string' ? part.text : '')) + .join(''); + } + return ''; + }; for (const line of lines) { if (!line.trim()) continue; try { const msg = JSON.parse(line); if (msg.type === 'message' && msg.role === 'assistant' && msg.content) { + const text = normalizeContent(msg.content); + if (!text) continue; if (msg.delta === true) { - deltaParts.push(msg.content); + deltaParts.push(text); } else { - completeParts.push(msg.content); + completeParts.push(text); } } } catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Wizard/services/conversationManager.ts` around lines 822 - 847, In the agentType === 'gemini-cli' parsing block, normalize msg.content before pushing to deltaParts/completeParts: detect if msg.content is an array (Array.isArray(msg.content)) and map each part to its textual field (e.g., part.text || part.content || String(part)) then join into a single string; otherwise use String(msg.content) for non-array values—then push that normalized string into deltaParts or completeParts so deltaParts.join('') / completeParts.join('') produce correct text instead of "[object Object]".src/main/ipc/handlers/agentSessions.ts (1)
240-245:⚠️ Potential issue | 🟠 MajorInput token extraction undercounts when fields coexist.
On Line 240, the
||chain treats input fields as mutually exclusive. If a payload contains bothinputandprompt, one side is dropped, which under-reports totals.💡 Proposed fix
- const input = - asNumber(obj.input) || - asNumber(obj.prompt) || - asNumber(obj.promptTokens) || - asNumber(obj.inputTokens) || - asNumber(obj.input_tokens); + const canonicalInput = asNumber(obj.input_tokens); + const input = + canonicalInput || + (asNumber(obj.input) + asNumber(obj.prompt)) || + asNumber(obj.promptTokens) || + asNumber(obj.inputTokens);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/agentSessions.ts` around lines 240 - 245, The current extraction for `input` in the agent session handler uses an `||` chain (variable `input` computed from `asNumber(obj.input) || asNumber(obj.prompt) || asNumber(obj.promptTokens) || asNumber(obj.inputTokens) || asNumber(obj.input_tokens)`), which drops coexisting fields and undercounts; change it to aggregate (sum) all available numeric token fields instead: call `asNumber` on each of `obj.input`, `obj.prompt`, `obj.promptTokens`, `obj.inputTokens`, and `obj.input_tokens`, treat non-numeric/undefined results as 0, and set `input` to the total so multiple fields contribute rather than only the first truthy one.
🧹 Nitpick comments (5)
src/renderer/components/Wizard/services/conversationManager.ts (1)
469-491: Remove or convert debugconsole.logstatements to usewizardDebugLogger.These
console.logstatements appear to be debug artifacts. The rest of the file consistently useswizardDebugLoggerfor logging. Consider removing these or converting them to use the established logger to maintain consistency and avoid polluting the console in production.♻️ Suggested refactor
if (code === 0) { - // DEBUG: Trace Gemini wizard response data flow - console.log('[WizardConversation] Exit code 0 — parsing response', { - agentType: this.session?.agentType, - outputBufferLength: this.session?.outputBuffer?.length || 0, - outputBufferPreview: this.session?.outputBuffer?.slice(0, 200) || '(empty)', - thinkingBufferLength: this.session?.thinkingBuffer?.length || 0, - thinkingBufferPreview: this.session?.thinkingBuffer?.slice(0, 200) || '(empty)', - }); + wizardDebugLogger.log('data', 'Exit code 0 — parsing response', { + agentType: this.session?.agentType, + outputBufferLength: this.session?.outputBuffer?.length || 0, + outputBufferPreview: this.session?.outputBuffer?.slice(0, 200) || '(empty)', + thinkingBufferLength: this.session?.thinkingBuffer?.length || 0, + thinkingBufferPreview: this.session?.thinkingBuffer?.slice(0, 200) || '(empty)', + }); const parsedResponse = this.parseAgentOutput(); - // DEBUG: Show parsed result - console.log('[WizardConversation] Parsed response', { - parseSuccess: parsedResponse.parseSuccess, - hasStructured: !!parsedResponse.structured, - confidence: parsedResponse.structured?.confidence, - ready: parsedResponse.structured?.ready, - messageLength: parsedResponse.structured?.message?.length || 0, - messagePreview: parsedResponse.structured?.message?.slice(0, 200) || '(none)', - rawTextLength: parsedResponse.rawText?.length || 0, - rawTextPreview: parsedResponse.rawText?.slice(0, 200) || '(empty)', - }); + wizardDebugLogger.log('data', 'Parsed response', { + parseSuccess: parsedResponse.parseSuccess, + hasStructured: !!parsedResponse.structured, + confidence: parsedResponse.structured?.confidence, + ready: parsedResponse.structured?.ready, + messageLength: parsedResponse.structured?.message?.length || 0, + messagePreview: parsedResponse.structured?.message?.slice(0, 200) || '(none)', + rawTextLength: parsedResponse.rawText?.length || 0, + rawTextPreview: parsedResponse.rawText?.slice(0, 200) || '(empty)', + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Wizard/services/conversationManager.ts` around lines 469 - 491, Replace the two debug console.log calls in the exit-code-0 branch with the established wizardDebugLogger usage: call wizardDebugLogger.debug (or appropriate level) instead of console.log and pass the same contextual object (agentType, outputBuffer length/preview, thinkingBuffer length/preview) for the first log and parsedResponse details (parseSuccess, structured flags, confidence, ready, messageLength/preview, rawTextLength/preview) for the second; locate these calls around the this.parseAgentOutput() invocation in conversationManager.ts and remove any leftover console.log references so all debug output consistently uses wizardDebugLogger.src/__tests__/renderer/components/Wizard/services/conversationManager.test.ts (1)
405-409: Removeas anytype assertions foragentType: 'gemini-cli'— the type is already properly defined.Since
'gemini-cli'is already included in theToolTypedefinition insrc/shared/types.ts, theas anycasts are unnecessary and harmful. They bypass TypeScript's type checking and mask potential issues. Remove the assertions throughout the test file (lines 406, 445, 485, 521, 562, 603, 646) and use the literal string directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/Wizard/services/conversationManager.test.ts` around lines 405 - 409, Remove the unnecessary "as any" casts on agentType in the test — replace occurrences like conversationManager.startConversation({ agentType: 'gemini-cli' as any, ... }) with the literal agentType: 'gemini-cli' so TypeScript uses the existing ToolType; update all similar calls in this test file (other startConversation invocations and any other objects with agentType) to remove the casts and let the compiler enforce the correct ToolType.src/cli/services/agent-sessions.ts (1)
373-394: Config path logic is duplicated fromreadOriginsStore.Both
readOriginsStore(lines 80-91) andreadAgentOriginsStoreshare nearly identical platform-specific config directory resolution. Per PR summary, this is intentional to avoid circular imports. Consider extracting to a small utility if this pattern grows further.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/services/agent-sessions.ts` around lines 373 - 394, The platform-specific config directory resolution used in readAgentOriginsStore duplicates the logic in readOriginsStore; extract that logic into a single helper (e.g., getConfigDir or resolveConfigDirectory) and use it from both readOriginsStore and readAgentOriginsStore to avoid duplication while keeping it local to the module to prevent circular imports; update readAgentOriginsStore to call the new helper to compute configDir and then build filePath and read/parse the JSON as before.src/renderer/components/AppModals.tsx (1)
1706-1730: SSH remote ID fallback correctly implemented; minor optimization available.The SSH remote ID fallback now properly checks both
sshRemoteIdandsessionSshRemoteConfig.remoteId, matching the established pattern used elsewhere (GitLogViewer at lines 1249-1254, AutoRunSetupModal at lines 1268-1273).However, the session is looked up twice — once for
sessionNameand once forsshRemoteId. Consider extracting the lookup to reduce redundancy.♻️ Optional: Extract session lookup
{/* --- WORKSPACE APPROVAL MODAL (Gemini sandbox) --- */} - {workspaceApprovalData && ( - <WorkspaceApprovalModal - theme={theme} - deniedPath={workspaceApprovalData.deniedPath} - errorMessage={workspaceApprovalData.errorMessage} - sessionName={ - sessions.find((s) => s.id === workspaceApprovalData.sessionId)?.name || 'Gemini CLI' - } - sshRemoteId={(() => { - const s = sessions.find((s) => s.id === workspaceApprovalData.sessionId); - return ( - s?.sshRemoteId || - (s?.sessionSshRemoteConfig?.enabled - ? s.sessionSshRemoteConfig.remoteId - : undefined) || - undefined - ); - })()} - onApprove={(directory) => - onApproveWorkspaceDir(workspaceApprovalData.sessionId, directory) - } - onDeny={onDenyWorkspaceDir} - /> - )} + {workspaceApprovalData && + (() => { + const approvalSession = sessions.find( + (s) => s.id === workspaceApprovalData.sessionId + ); + return ( + <WorkspaceApprovalModal + theme={theme} + deniedPath={workspaceApprovalData.deniedPath} + errorMessage={workspaceApprovalData.errorMessage} + sessionName={approvalSession?.name || 'Gemini CLI'} + sshRemoteId={ + approvalSession?.sshRemoteId || + (approvalSession?.sessionSshRemoteConfig?.enabled + ? approvalSession.sessionSshRemoteConfig.remoteId + : undefined) + } + onApprove={(directory) => + onApproveWorkspaceDir(workspaceApprovalData.sessionId, directory) + } + onDeny={onDenyWorkspaceDir} + /> + ); + })()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AppModals.tsx` around lines 1706 - 1730, The session is being looked up twice for WorkspaceApprovalModal (once for sessionName and once for sshRemoteId); extract a single const (e.g., const session = sessions.find(s => s.id === workspaceApprovalData.sessionId)) before the JSX and reuse it when computing sessionName and sshRemoteId to remove the duplicate lookup and simplify the component props.src/main/process-manager/handlers/StdoutHandler.ts (1)
381-389: Consider extracting the nested error message logic into a helper for readability.The chained type assertions work correctly but are hard to follow. A small helper function would improve clarity.
♻️ Optional refactor for readability
+function extractToolStateError(toolState: unknown): string | null { + if (!toolState || typeof toolState !== 'object') return null; + const state = toolState as Record<string, unknown>; + if (typeof state.error === 'string') return state.error; + if (state.error && typeof state.error === 'object') { + const errObj = state.error as Record<string, unknown>; + if (typeof errObj.message === 'string') return errObj.message; + } + return null; +} // In the handler: - const errorMsg = - (event.toolState as Record<string, unknown>)?.error && - typeof ((event.toolState as Record<string, unknown>).error as Record<string, unknown>) - ?.message === 'string' - ? (((event.toolState as Record<string, unknown>).error as Record<string, unknown>) - .message as string) - : typeof (event.toolState as Record<string, unknown>)?.error === 'string' - ? ((event.toolState as Record<string, unknown>).error as string) - : null; + const errorMsg = extractToolStateError(event.toolState);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 381 - 389, The nested extraction for errorMsg in StdoutHandler.ts is hard to read; create a small helper (e.g., extractErrorMessage or getNestedErrorMessage) that accepts event.toolState and returns the string or null, then replace the inline ternary with a call to that helper; ensure the helper handles both object-with-message and string error shapes and preserves the exact type checks used currently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/main/ipc/handlers/agentSessions.test.ts`:
- Around line 654-742: Add a new unit test in the existing
describe('parseGeminiSessionContent') block that verifies mixed token fields are
counted correctly: create a Gemini message whose tokens object contains both
input and prompt (e.g., tokens: { input: 5, prompt: 7, output: 10 }), call
parseGeminiSessionContent with that content and assert that result.inputTokens
equals the sum of input+prompt (12) and result.outputTokens equals 10; this
prevents regressions where one of those fields is ignored. Reference
parseGeminiSessionContent and add the test alongside the other it(...) cases in
the same test file.
In `@src/cli/services/agent-sessions.ts`:
- Around line 433-436: The catch block that handles missing .project_root
currently returns directPath which can incorrectly match a different project
with the same basename; change the catch to return null instead so the caller
(the session discovery fallback scan in the agent-sessions service) will run the
fallback scan; locate the try/catch that reads ".project_root" in
src/cli/services/agent-sessions.ts (the function responsible for resolving
session project roots) and replace the return of directPath in the catch with
return null, and ensure any callers of this resolver already treat null as a
signal to perform the fallback scan.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 379-406: Update the comment above the Gemini sandbox check in
StdoutHandler to reflect that the parser normalizes Gemini CLI's raw tool_result
into a ParsedEvent with event.type === 'tool_use'; replace the inaccurate
"tool_result error events" wording with something like "Detect Gemini CLI
sandbox violations from normalized tool_use events carrying tool result status"
so it accurately describes why the code checks event.type === 'tool_use' (the
logic around managedProcess.toolType === 'gemini-cli', extracting errorMsg,
calling extractDeniedPath, logging via logger.info, and emitting
'workspace-approval-request' remains unchanged).
---
Duplicate comments:
In `@src/main/ipc/handlers/agentSessions.ts`:
- Around line 240-245: The current extraction for `input` in the agent session
handler uses an `||` chain (variable `input` computed from `asNumber(obj.input)
|| asNumber(obj.prompt) || asNumber(obj.promptTokens) ||
asNumber(obj.inputTokens) || asNumber(obj.input_tokens)`), which drops
coexisting fields and undercounts; change it to aggregate (sum) all available
numeric token fields instead: call `asNumber` on each of `obj.input`,
`obj.prompt`, `obj.promptTokens`, `obj.inputTokens`, and `obj.input_tokens`,
treat non-numeric/undefined results as 0, and set `input` to the total so
multiple fields contribute rather than only the first truthy one.
In `@src/renderer/components/Wizard/services/conversationManager.ts`:
- Around line 822-847: In the agentType === 'gemini-cli' parsing block,
normalize msg.content before pushing to deltaParts/completeParts: detect if
msg.content is an array (Array.isArray(msg.content)) and map each part to its
textual field (e.g., part.text || part.content || String(part)) then join into a
single string; otherwise use String(msg.content) for non-array values—then push
that normalized string into deltaParts or completeParts so deltaParts.join('') /
completeParts.join('') produce correct text instead of "[object Object]".
---
Nitpick comments:
In
`@src/__tests__/renderer/components/Wizard/services/conversationManager.test.ts`:
- Around line 405-409: Remove the unnecessary "as any" casts on agentType in the
test — replace occurrences like conversationManager.startConversation({
agentType: 'gemini-cli' as any, ... }) with the literal agentType: 'gemini-cli'
so TypeScript uses the existing ToolType; update all similar calls in this test
file (other startConversation invocations and any other objects with agentType)
to remove the casts and let the compiler enforce the correct ToolType.
In `@src/cli/services/agent-sessions.ts`:
- Around line 373-394: The platform-specific config directory resolution used in
readAgentOriginsStore duplicates the logic in readOriginsStore; extract that
logic into a single helper (e.g., getConfigDir or resolveConfigDirectory) and
use it from both readOriginsStore and readAgentOriginsStore to avoid duplication
while keeping it local to the module to prevent circular imports; update
readAgentOriginsStore to call the new helper to compute configDir and then build
filePath and read/parse the JSON as before.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 381-389: The nested extraction for errorMsg in StdoutHandler.ts is
hard to read; create a small helper (e.g., extractErrorMessage or
getNestedErrorMessage) that accepts event.toolState and returns the string or
null, then replace the inline ternary with a call to that helper; ensure the
helper handles both object-with-message and string error shapes and preserves
the exact type checks used currently.
In `@src/renderer/components/AppModals.tsx`:
- Around line 1706-1730: The session is being looked up twice for
WorkspaceApprovalModal (once for sessionName and once for sshRemoteId); extract
a single const (e.g., const session = sessions.find(s => s.id ===
workspaceApprovalData.sessionId)) before the JSX and reuse it when computing
sessionName and sshRemoteId to remove the duplicate lookup and simplify the
component props.
In `@src/renderer/components/Wizard/services/conversationManager.ts`:
- Around line 469-491: Replace the two debug console.log calls in the
exit-code-0 branch with the established wizardDebugLogger usage: call
wizardDebugLogger.debug (or appropriate level) instead of console.log and pass
the same contextual object (agentType, outputBuffer length/preview,
thinkingBuffer length/preview) for the first log and parsedResponse details
(parseSuccess, structured flags, confidence, ready, messageLength/preview,
rawTextLength/preview) for the second; locate these calls around the
this.parseAgentOutput() invocation in conversationManager.ts and remove any
leftover console.log references so all debug output consistently uses
wizardDebugLogger.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/__tests__/cli/commands/list-sessions.test.tssrc/__tests__/cli/services/agent-sessions.test.tssrc/__tests__/main/ipc/handlers/agentSessions.test.tssrc/__tests__/main/process-manager/handlers/StdoutHandler.test.tssrc/__tests__/renderer/components/Wizard/services/conversationManager.test.tssrc/cli/commands/list-sessions.tssrc/cli/services/agent-sessions.tssrc/main/ipc/handlers/agentSessions.tssrc/main/process-manager/handlers/StdoutHandler.tssrc/renderer/App.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/Wizard/services/conversationManager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/main/process-manager/handlers/StdoutHandler.test.ts
Comment on lines
+654
to
+742
| describe('parseGeminiSessionContent', () => { | ||
| it('should parse messages and return zeroed tokens when no token data in session', () => { | ||
| const content = JSON.stringify({ | ||
| messages: [{ type: 'user' }, { type: 'gemini' }, { type: 'user' }, { type: 'gemini' }], | ||
| }); | ||
| const result = parseGeminiSessionContent(content, 1024); | ||
| expect(result.messages).toBe(4); | ||
| expect(result.inputTokens).toBe(0); | ||
| expect(result.outputTokens).toBe(0); | ||
| expect(result.cachedInputTokens).toBe(0); | ||
| expect(result.sizeBytes).toBe(1024); | ||
| }); | ||
|
|
||
| it('should fall back to persistedStats when message-level tokens are 0', () => { | ||
| const content = JSON.stringify({ | ||
| messages: [{ type: 'user' }, { type: 'gemini' }], | ||
| }); | ||
| const persistedStats = { | ||
| inputTokens: 500, | ||
| outputTokens: 1200, | ||
| cacheReadTokens: 100, | ||
| reasoningTokens: 50, | ||
| }; | ||
| const result = parseGeminiSessionContent(content, 2048, persistedStats); | ||
| expect(result.messages).toBe(2); | ||
| expect(result.inputTokens).toBe(500); | ||
| expect(result.outputTokens).toBe(1200); | ||
| expect(result.cachedInputTokens).toBe(100); | ||
| expect(result.sizeBytes).toBe(2048); | ||
| }); | ||
|
|
||
| it('should NOT fall back to persistedStats when message-level tokens are non-zero', () => { | ||
| // Hypothetical: if Gemini ever adds token data to messages | ||
| const content = JSON.stringify({ | ||
| messages: [{ type: 'user', tokens: { input: 10, output: 20 } }], | ||
| }); | ||
| const persistedStats = { | ||
| inputTokens: 500, | ||
| outputTokens: 1200, | ||
| cacheReadTokens: 100, | ||
| reasoningTokens: 50, | ||
| }; | ||
| const result = parseGeminiSessionContent(content, 512, persistedStats); | ||
| // Should use the message-level data, not the persisted fallback | ||
| expect(result.inputTokens).toBe(10); | ||
| expect(result.outputTokens).toBe(20); | ||
| }); | ||
|
|
||
| it('should handle empty/invalid JSON gracefully with persistedStats fallback', () => { | ||
| const persistedStats = { | ||
| inputTokens: 300, | ||
| outputTokens: 600, | ||
| cacheReadTokens: 50, | ||
| reasoningTokens: 0, | ||
| }; | ||
| const result = parseGeminiSessionContent('not valid json', 100, persistedStats); | ||
| expect(result.messages).toBe(0); | ||
| // Parse failed, tokens are 0, so persisted stats should be used | ||
| expect(result.inputTokens).toBe(300); | ||
| expect(result.outputTokens).toBe(600); | ||
| expect(result.cachedInputTokens).toBe(50); | ||
| }); | ||
|
|
||
| it('should report corrupted session JSON to Sentry', () => { | ||
| parseGeminiSessionContent('not valid json', 256); | ||
| expect(mockCaptureException).toHaveBeenCalledWith(expect.any(SyntaxError), { | ||
| context: 'parseGeminiSessionContent', | ||
| sizeBytes: 256, | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle missing messages array', () => { | ||
| const content = JSON.stringify({ sessionId: 'abc-123' }); | ||
| const result = parseGeminiSessionContent(content, 50); | ||
| expect(result.messages).toBe(0); | ||
| expect(result.inputTokens).toBe(0); | ||
| expect(result.outputTokens).toBe(0); | ||
| }); | ||
|
|
||
| it('should not use persistedStats when undefined', () => { | ||
| const content = JSON.stringify({ | ||
| messages: [{ type: 'user' }], | ||
| }); | ||
| const result = parseGeminiSessionContent(content, 100); | ||
| expect(result.inputTokens).toBe(0); | ||
| expect(result.outputTokens).toBe(0); | ||
| expect(result.cachedInputTokens).toBe(0); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add a regression test for mixed Gemini input token fields.
Please add a case where a single token object contains both input and prompt so undercounting can’t regress unnoticed.
🧪 Suggested test case
+ it('should combine mixed input fields from a single token object', () => {
+ const content = JSON.stringify({
+ messages: [{ type: 'user', tokens: { input: 10, prompt: 5, output: 3 } }],
+ });
+ const result = parseGeminiSessionContent(content, 256);
+ expect(result.inputTokens).toBe(15);
+ expect(result.outputTokens).toBe(3);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/main/ipc/handlers/agentSessions.test.ts` around lines 654 -
742, Add a new unit test in the existing describe('parseGeminiSessionContent')
block that verifies mixed token fields are counted correctly: create a Gemini
message whose tokens object contains both input and prompt (e.g., tokens: {
input: 5, prompt: 7, output: 10 }), call parseGeminiSessionContent with that
content and assert that result.inputTokens equals the sum of input+prompt (12)
and result.outputTokens equals 10; this prevents regressions where one of those
fields is ignored. Reference parseGeminiSessionContent and add the test
alongside the other it(...) cases in the same test file.
Comment on lines
+433
to
+436
| } catch { | ||
| // No .project_root file — basename match is the best we have | ||
| return directPath; | ||
| } |
There was a problem hiding this comment.
Basename match without .project_root may return wrong directory.
When .project_root doesn't exist, the function returns the directory matching the basename. If two projects share the same basename (e.g., both named "app"), this could return sessions from a different project. Consider returning null to trigger the fallback scan instead.
🛡️ Proposed fix
} catch {
- // No .project_root file — basename match is the best we have
- return directPath;
+ // No .project_root file — fall through to scan all subdirs
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch { | |
| // No .project_root file — basename match is the best we have | |
| return directPath; | |
| } | |
| } catch { | |
| // No .project_root file — fall through to scan all subdirs | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/services/agent-sessions.ts` around lines 433 - 436, The catch block
that handles missing .project_root currently returns directPath which can
incorrectly match a different project with the same basename; change the catch
to return null instead so the caller (the session discovery fallback scan in the
agent-sessions service) will run the fallback scan; locate the try/catch that
reads ".project_root" in src/cli/services/agent-sessions.ts (the function
responsible for resolving session project roots) and replace the return of
directPath in the catch with return null, and ensure any callers of this
resolver already treat null as a signal to perform the fallback scan.
Comment on lines
+379
to
+406
| // Detect Gemini CLI sandbox violations from tool_result error events | ||
| if (event.type === 'tool_use' && managedProcess.toolType === 'gemini-cli' && event.toolState) { | ||
| const errorMsg = | ||
| (event.toolState as Record<string, unknown>)?.error && | ||
| typeof ((event.toolState as Record<string, unknown>).error as Record<string, unknown>) | ||
| ?.message === 'string' | ||
| ? (((event.toolState as Record<string, unknown>).error as Record<string, unknown>) | ||
| .message as string) | ||
| : typeof (event.toolState as Record<string, unknown>)?.error === 'string' | ||
| ? ((event.toolState as Record<string, unknown>).error as string) | ||
| : null; | ||
|
|
||
| if (errorMsg && /path.*not.*in.*workspace|permission.*denied.*sandbox/i.test(errorMsg)) { | ||
| const deniedPath = extractDeniedPath(errorMsg); | ||
| if (deniedPath) { | ||
| logger.info('[ProcessManager] Gemini sandbox violation detected', 'WorkspaceApproval', { | ||
| sessionId, | ||
| deniedPath, | ||
| errorMessage: errorMsg, | ||
| }); | ||
| this.emitter.emit('workspace-approval-request', sessionId, { | ||
| deniedPath, | ||
| errorMessage: errorMsg, | ||
| timestamp: Date.now(), | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find files related to Gemini parser and event handling
fd -t f -e ts -e tsx | grep -i 'gemini.*parser\|output.*parser\|event.*type' | head -20Repository: RunMaestro/Maestro
Length of output: 683
🏁 Script executed:
# Search for tool_result and tool_use event type handling
rg -n "tool_result|tool_use" --type ts -A 3 -B 3 | head -100Repository: RunMaestro/Maestro
Length of output: 7944
🏁 Script executed:
# Look specifically for Gemini-related parser files
find . -type f -name "*gemini*" -o -name "*parser*" | grep -E "\.(ts|tsx|js)$" | head -20Repository: RunMaestro/Maestro
Length of output: 1085
🏁 Script executed:
# Check the event interface/type definitions
rg -n "event\.type\s*===" --type ts -B 5 -A 5 | grep -E "tool_use|tool_result" -A 5 -B 5 | head -50Repository: RunMaestro/Maestro
Length of output: 1945
🏁 Script executed:
# Read the Gemini output parser to understand event types
cat -n src/main/parsers/gemini-output-parser.ts | head -150Repository: RunMaestro/Maestro
Length of output: 5064
🏁 Script executed:
# Search for where tool_use and tool_result events are created in the parser
rg -n "type.*tool_use|type.*tool_result" src/main/parsers/gemini-output-parser.ts -A 5 -B 5Repository: RunMaestro/Maestro
Length of output: 1007
🏁 Script executed:
# Check the StdoutHandler more broadly to understand event structure
cat -n src/main/process-manager/handlers/StdoutHandler.ts | sed -n '370,410p'Repository: RunMaestro/Maestro
Length of output: 1915
Update comment to reflect event type normalization.
The Gemini output parser normalizes raw tool_result events from Gemini CLI to tool_use in the ParsedEvent format before they reach StdoutHandler. The code correctly checks for event.type === 'tool_use', but the comment at line 379 incorrectly references "tool_result error events." Update the comment to accurately describe the event type being handled, e.g., "Detect Gemini CLI sandbox violations from normalized tool_use events carrying tool result status."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 379 - 406,
Update the comment above the Gemini sandbox check in StdoutHandler to reflect
that the parser normalizes Gemini CLI's raw tool_result into a ParsedEvent with
event.type === 'tool_use'; replace the inaccurate "tool_result error events"
wording with something like "Detect Gemini CLI sandbox violations from
normalized tool_use events carrying tool result status" so it accurately
describes why the code checks event.type === 'tool_use' (the logic around
managedProcess.toolType === 'gemini-cli', extracting errorMsg, calling
extractDeniedPath, logging via logger.info, and emitting
'workspace-approval-request' remains unchanged).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/__tests__/main/agents/capabilities.test.ts (1)
118-125:⚠️ Potential issue | 🟡 MinorTest comments repeat the same stale Gemini docs (
planand tmp/chats path).Lines 118 and 124 should be updated to match the corrected source comments so tests don’t preserve incorrect operator guidance.
💡 Suggested comment-only cleanup
- expect(capabilities.supportsReadOnlyMode).toBe(true); // --approval-mode plan (experimental) + expect(capabilities.supportsReadOnlyMode).toBe(true); // Soft read-only via prompt; approval modes: default|auto_edit|yolo ... - expect(capabilities.supportsSessionStorage).toBe(true); // ~/.gemini/tmp/<hash>/chats/ + expect(capabilities.supportsSessionStorage).toBe(true); // ~/.gemini/history/{project}/session-*.jsonIn Gemini CLI v0.29.5 docs, confirm the valid `--approval-mode` values and the default on-disk history/session path format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/agents/capabilities.test.ts` around lines 118 - 125, Update the stale inline test comments for capabilities.supportsReadOnlyMode and capabilities.supportsSessionStorage to match current Gemini docs: change the supportsReadOnlyMode comment to reflect the correct --approval-mode valid values and default behavior (not “plan”) and change the supportsSessionStorage comment to the corrected on-disk history/session path format (replace "~/.gemini/tmp/<hash>/chats/" with the current documented path format). Keep the assertions unchanged; only edit the comment text next to capabilities.supportsReadOnlyMode and capabilities.supportsSessionStorage to the accurate, up-to-date operator guidance.src/main/agents/capabilities.ts (1)
206-208:⚠️ Potential issue | 🟡 Minor
supportsReadOnlyModecomment still references an invalid Gemini approval mode.Line 207 mentions
--approval-mode plan, but this value has already been flagged and should be corrected to avoid future config mistakes.💡 Suggested comment fix
- supportsReadOnlyMode: true, // --approval-mode plan (experimental; currently enforced via system prompt) + supportsReadOnlyMode: true, // Soft read-only via system prompt; Gemini approval modes are default|auto_edit|yoloFor Gemini CLI v0.29.5, what are the valid values for `--approval-mode`? Is `plan` a valid value?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/agents/capabilities.ts` around lines 206 - 208, The inline comment for supportsReadOnlyMode incorrectly references an invalid Gemini approval-mode value ("--approval-mode plan"); update the comment next to supportsReadOnlyMode in capabilities.ts to remove or replace the "plan" example with only validated/accurate values (or a generic placeholder like "--approval-mode <mode>") and keep the note that it's experimental and enforced via system prompt; ensure the supportsReadOnlyMode line and its comment no longer mention the deprecated/invalid "plan" token.
🤖 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/agents/capabilities.ts`:
- Line 213: Update the inline comment next to supportsSessionStorage to reflect
the actual session file path used by the implementation: replace the outdated
"~/.gemini/tmp/<project_hash>/chats/" with the current pattern
"~/.gemini/history/{project_name}/session-{timestamp}-{sessionId}.json" so the
comment near supportsSessionStorage in src/main/agents/capabilities.ts
accurately matches the storage behavior implemented in
src/main/storage/gemini-session-storage.ts.
---
Duplicate comments:
In `@src/__tests__/main/agents/capabilities.test.ts`:
- Around line 118-125: Update the stale inline test comments for
capabilities.supportsReadOnlyMode and capabilities.supportsSessionStorage to
match current Gemini docs: change the supportsReadOnlyMode comment to reflect
the correct --approval-mode valid values and default behavior (not “plan”) and
change the supportsSessionStorage comment to the corrected on-disk
history/session path format (replace "~/.gemini/tmp/<hash>/chats/" with the
current documented path format). Keep the assertions unchanged; only edit the
comment text next to capabilities.supportsReadOnlyMode and
capabilities.supportsSessionStorage to the accurate, up-to-date operator
guidance.
In `@src/main/agents/capabilities.ts`:
- Around line 206-208: The inline comment for supportsReadOnlyMode incorrectly
references an invalid Gemini approval-mode value ("--approval-mode plan");
update the comment next to supportsReadOnlyMode in capabilities.ts to remove or
replace the "plan" example with only validated/accurate values (or a generic
placeholder like "--approval-mode <mode>") and keep the note that it's
experimental and enforced via system prompt; ensure the supportsReadOnlyMode
line and its comment no longer mention the deprecated/invalid "plan" token.
| supportsImageInput: false, // Gemini is multimodal but CLI has no --image flag for batch mode | ||
| supportsImageInputOnResume: false, // No image flag | ||
| supportsSlashCommands: false, // Has 20+ slash commands in interactive mode but not exposed in JSON output | ||
| supportsSessionStorage: true, // ~/.gemini/tmp/<project_hash>/chats/ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, let's find all files related to Gemini session storage
fd 'gemini.*storage' src --type f
# Also search for any references to the storage paths mentioned
rg -n '\.gemini/(tmp|history)|session-.*\.json|/chats' src/ --type ts --type tsx -B2 -A2Repository: RunMaestro/Maestro
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the specific file and line in question
sed -n '210,220p' src/main/agents/capabilities.ts
# Also check the actual Gemini session storage implementation
cat src/main/storage/gemini-session-storage.ts
# And the test file for context
cat src/__tests__/main/storage/gemini-session-storage.test.tsRepository: RunMaestro/Maestro
Length of output: 50376
Update session-storage path comment to match current implementation.
Line 213 documents ~/.gemini/tmp/<project_hash>/chats/, but the implementation at src/main/storage/gemini-session-storage.ts uses ~/.gemini/history/{project_name}/session-{timestamp}-{sessionId}.json. Align the comment with the actual storage path.
Suggested fix
- supportsSessionStorage: true, // ~/.gemini/tmp/<project_hash>/chats/
+ supportsSessionStorage: true, // ~/.gemini/history/{project}/session-*.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| supportsSessionStorage: true, // ~/.gemini/tmp/<project_hash>/chats/ | |
| supportsSessionStorage: true, // ~/.gemini/history/{project}/session-*.json |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/agents/capabilities.ts` at line 213, Update the inline comment next
to supportsSessionStorage to reflect the actual session file path used by the
implementation: replace the outdated "~/.gemini/tmp/<project_hash>/chats/" with
the current pattern
"~/.gemini/history/{project_name}/session-{timestamp}-{sessionId}.json" so the
comment near supportsSessionStorage in src/main/agents/capabilities.ts
accurately matches the storage behavior implemented in
src/main/storage/gemini-session-storage.ts.
Contributor
Author
Review Fixes AppliedAll 9 major and 4 minor CodeRabbit review items addressed across 13 commits. No merge conflicts — branch rebased cleanly on main. Major Fixes
Minor Fixes
Verification
|
Replace 62 inline `process.platform` checks across 25 main/shared/CLI files with utility functions from `src/shared/platformDetection.ts`. Functions (not constants) allow tests to override `process.platform` via Object.defineProperty. Also eliminates 4 copy-pasted `which`/`where` command selections via `getWhichCommand()`.
…latform Replace navigator.userAgent and navigator.platform checks with centralized helpers from platformUtils.ts (isWindowsPlatform, isMacOSPlatform, isLinuxPlatform). shortcutFormatter.ts no longer uses a module-level constant, enabling simpler test mocking via window.maestro.platform instead of dynamic imports with vi.resetModules().
Replace remaining inline `isWindows() ? 'where' : 'which'` ternaries in path-prober.ts and shellDetector.ts with the centralized getWhichCommand() helper. Add two tests for the Linux-specific note in GeneralTab's Power Management section.
* Add 'bills-bot' entry to symphony-registry.json Added new entry for 'bills-bot' with details including description, URL, category, tags, and maintainer information. * Replace bills-bot with volvox-bot in registry
Previously, pressing Cmd+J to switch modes left focus in an indeterminate state, requiring an extra click before typing. Now the input field receives focus immediately, matching the existing pattern used by the newTab shortcut.
Replace magic number 50ms with FOCUS_AFTER_RENDER_DELAY_MS constant, shared between toggleMode and newTab handlers per review feedback.
The toggleMode handler now calls setActiveFocus and inputRef.focus() after toggling. Update mock contexts in the three tests that exercise this path so they no longer throw on the missing properties.
Use vi.useFakeTimers() and vi.advanceTimersByTime(50) to verify that
setActiveFocus('main') is called synchronously and inputRef.focus()
is called after the render delay in all three toggleMode test cases.
- Add TERMINAL_SHORTCUTS export to shortcuts.ts with newTerminalTab (Ctrl+Shift+`) and clearTerminal (Cmd+K) - Extend MainPanelHandle with clearActiveTerminal(), focusActiveTerminal(), openTerminalSearch() methods - Add terminalSearchOpen state in MainPanel; wire searchOpen/onSearchClose props to TerminalView - Add handleOpenTerminalTab and mainPanelRef to keyboardHandlerRef context in App.tsx - Guard Ctrl+[A-Z] sequences in terminal mode so xterm.js receives them (Ctrl+C, Ctrl+D, etc.) - Route Cmd+W/Cmd+Shift+[]/Cmd+1-9/Cmd+0 to terminal tab navigation when inputMode === 'terminal' - Override Cmd+K to clear terminal (instead of Quick Actions) when in terminal mode - Add Cmd+F handler to open terminal search overlay when in terminal mode - Add Ctrl+Shift+` handler to create new terminal tab from any mode (switches to terminal mode if needed) - Focus active terminal after Cmd+J mode toggle to terminal mode
…tures - Add missing setStarTabCallback, setReorderTabCallback, setToggleBookmarkCallback to web-server-factory test mock - Add missing onRemoteStarTab, onRemoteReorderTab, onRemoteToggleBookmark handlers and broadcastSessionState mock to useRemoteIntegration test - Update TerminalOutput test to match corrected alignment toggle behavior (ba80730) - Move useCallback hooks above early return in TabBar.tsx to fix ESLint rules-of-hooks
- Cmd+J now opens a new terminal tab (like Cmd+T for AI tabs) instead of toggling mode; handleOpenTerminalTab auto-creates tab and sets inputMode - Remove redundant toggleInputMode() after handleOpenTerminalTab in Ctrl+Shift+` handler (it was toggling mode back to 'ai' after creation) - Add attachCustomKeyEventHandler in XTerminal so Meta-key and Ctrl+Shift combos bubble through xterm to the window handler (fixes Cmd+K, Cmd+W, etc.) - setActiveTab now sets inputMode:'ai' so clicking an AI tab from terminal mode switches back to AI view; tighten early-return to skip mutation only when already in ai mode - Update 3 keyboard handler tests and add 1 new tabHelpers test
- Cmd+Shift+R (renameTab) now opens TerminalTabRenameModal when in terminal mode, targeting the active terminal tab. Previously the handler only checked AI tabs (which required agentSessionId), so pressing the shortcut in terminal mode silently did nothing. - Add session.projectRoot as fallback cwd in TerminalView.spawnPtyForTab, guarding against sessions where both tab.cwd and session.cwd are empty. - Remove unused useState import from TerminalView.tsx.
…nList The SessionList refactor (4c2f639) imported HamburgerMenuContent from a separate file but never created it, causing a module-not-found error in the SessionList test suite. Extract the component from the original SessionList.tsx into its own file with the same implementation.
…data RightPanel was consuming flatFileList (FlatTreeNode[]) where it needed tree-structured FileNode[], causing double-flattening and duplicate entries. Added filteredFileTree to the store as the proper bridge between useFileTreeManagement and RightPanel.
Applies provider-specific read-only args from centralized agent definitions (e.g. --permission-mode plan for Claude Code, --sandbox read-only for Codex) without duplicating logic.
New trigger that polls markdown files for unchecked tasks (- [ ]) and fires events per file when content changes and pending tasks exist. Includes content hashing to avoid re-triggering, picomatch glob matching, 6 new template variables, YAML validation, and full renderer support.
…ype pill The session name pill had flex-shrink-0 which prevented it from shrinking when the right panel was narrow, causing the date to overlap the USER/AUTO pill. Now uses flex-shrink so it truncates gracefully and expands when space is available.
- Remove default terminal tab creation from new sessions and restoration; tabs are now created on demand via the + button or Cmd+J - Fix duplicate 'Starting terminal...' messages by not clearing loadingWrittenRef in React's null ref callback branch - Add + button popover in TabBar offering 'New AI Chat' and 'New Terminal' choices with keyboard shortcut hints - Enable keyboard shortcuts (Cmd+T, Cmd+W, tab nav) in terminal mode; Cmd+T creates a new AI tab, navigation crosses all tab types via navigateToNextUnifiedTab/navigateToPrevUnifiedTab - Preserve xterm.js scrollback buffer by keeping TerminalView always mounted (hidden with display:none) rather than unmounting on mode switch - Allow closing the last terminal tab; switches inputMode back to 'ai' - Update all affected tests to reflect new behaviors (7 test cases across 6 files)
- Add hook/extension lifecycle patterns to StderrHandler info filter (Loading extension, Hook execution, Created execution plan, etc.) - Change non-info stderr emission from 'stderr' to 'data' so Gemini response text renders as normal output instead of with STDERR label - Add tests for StderrHandler Gemini filtering (5 tests) and StdoutHandler partial vs complete text handling (2 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pedramamini
force-pushed
the
0.16.0-RC
branch
2 times, most recently
from
132d88f to
26d77ee
Compare
Add MAX_BUFFER_SIZE (5MB) for stream-JSON mode and MAX_BATCH_BUFFER_SIZE (10MB) for batch mode jsonBuffer accumulation. Batch mode truncates at limit; stream-JSON mode clears stale buffer and restarts with incoming data. Switch streamedText from O(n²) string concatenation to streamedChunks array with push(); add getStreamedText() helper that joins at read time with fallback to legacy streamedText field. Updated StdoutHandler (6 sites) and ExitHandler (5 sites). 10 new tests covering buffer guards and chunked accumulation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Regex patterns for Gemini CLI info filtering, SSH info suppression, and
Axios dump detection were recreated inside handleData() on every stderr
event — 13+ regex compilations per event on the main thread. Hoisted all
patterns to module-level constants (GEMINI_INFO_PATTERNS, SSH_INFO_PATTERNS,
GEMINI_AXIOS_DUMP, etc.) and consolidated multi-test sequences into single
alternation regexes.
More critically, log entry text grew without bound during streaming sessions.
The 500ms time-based grouping in useBatchedSessionUpdates concatenated all
output into a single LogEntry's text field, which was then re-processed by
DOMPurify, ANSI conversion, and split('\n') on every 150ms render flush.
Added a 512KB cap with truncation marker to all three accumulation paths
(AI tab logs, shell stdout, shell stderr) and the thinking chunk handler.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ulation (TASK-m02) Changed token source iteration from accumulate-all to first-match semantics. When the same token data appears in multiple locations (msg.tokens, msg.tokenUsage, msg.tokenCounts, msg.metadata.tokens), only the first source with non-zero values is used, preventing 2-4x overcounting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…edundant fs.stat() (TASK-m03) Add sizeBytes to SessionFileInfo interface, captured during discovery to avoid redundant fs.stat() calls in Claude/Codex/Gemini processing loops. When Gemini persistedStats exist, use lightweight regex message counter (countGeminiMessages) instead of full JSON.parse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…K-m04) - WorkspaceApprovalModal: wrap sortedFiles in useMemo - GroupChatModal: wrap availableTiles/selectedAgentConfig/selectedTile in useMemo, wrap AgentConfigPanel handlers in useCallback - AppModals: extract approvalSession lookup above JSX, eliminating IIFE and 3x duplicate sessions.find() - AgentCreationDialog: extract per-agent card into memoized AgentConfigRow component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…config.ts (TASK-m05) Moved buildGeminiWorkspaceDirArgs from local copies in group-chat-router.ts and group-chat-agent.ts to a single shared export in group-chat-config.ts. Added path.resolve() normalization before dedup so equivalent paths are not duplicated. Evaluated spawnGroupChatAgent() helper extraction but declined — the 5 spawn sites differ too much in readOnlyMode, error handling, and post-spawn logic to justify a shared abstraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…(TASK-m06) Add a configurable 5-minute timeout for pending participant responses. On expiry, clears pending set, logs a warning with timed-out participant names, and triggers moderator synthesis with available responses. Timeout is cleared on: last participant responding, explicit clearPendingParticipants, or new user message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion (TASK-m07) Move addParticipantToChat() before session map population and wrap in try/catch. On storage rejection (duplicate name from concurrent calls), the spawned process is killed via processManager.kill() before the error propagates. Session mapping is only stored after successful storage write. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace bare catch {} in group-chat-moderator.ts with logger.debug for unexpected errors
- Add logger.warn when skipping empty Gemini session files in listSessions and searchSessions
- Return error: 'Session file is corrupted' from readSessionMessages catch block
- Detect ENOENT in ExitHandler.handleError; emit agent_not_found with install suggestion
- Add agent_not_found to AgentErrorType union in shared/types.ts
- Log unknown Gemini event types at debug level in gemini-output-parser.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unnecessary `as any` and `as ToolType` casts, extract extractToolStateError() helper replacing 6-cast ternary, add missing fields to ProcessManagerEvents/ProcessConfig/AgentCapabilities interfaces, and use AgentErrorType instead of string in AgentError definitions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace special characters in participant names with underscores before embedding them in session IDs, preventing invalid characters from appearing in identifiers used for process tracking and file operations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace textPreview with textLength in debug logs to avoid logging content - Replace raw JSON event dump in synopsis warning with structural metadata - Add sanitizeStderr() to redact API keys/tokens from error messages - Use path.basename() instead of full paths in Sentry captureException calls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Number.isNaN() guard for startTime/endTime in CLI agent-sessions.ts to return durationSeconds=0 when timestamps are invalid, matching the existing fix in main process gemini-session-storage.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add the same --no-sandbox / readOnlyCliEnforced check to participant spawns that already exists in moderator and synthesis spawns, ensuring consistent behavior across all group chat spawn paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… tests (TASK-m14) gemini-cli is already in the ToolType union, so the 7 `as any` casts in conversationManager.test.ts were redundant. Removed all of them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lures (TASK-m15) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…K-m16) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
40 tests covering: JSON stdout parsing, partial streaming, exit codes, malformed output, getGeminiCommand resolution, usage stats accumulation, timeout behavior, model/session ID validation, ENOENT errors, and custom env passthrough. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ASK-T02) Add 33 new tests covering Gemini NDJSON extraction, Claude Code output, Codex output, empty output, malformed JSON, mixed content types, tool call output, partial streaming, error events, and very long output scenarios. Total: 54 tests (21 existing + 33 new). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ASK-T03) Add 7 corrupted state tests covering: corrupted log file (readLog throws), undefined message content, empty from field, invalid timestamps, corrupted JSON storage, missing metadata file, and null bytes in output. Total: 47 tests passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…TASK-T04) 17 new tests: - Search: case-insensitive matching, matchPreview truncation at 200 chars, multi-match counting (user/assistant), all-mode fallback priority (title→user→assistant), corrupted JSON graceful handling, title preview fallback, multi-session results, info/error/warning message exclusion, last-resort query preview - Pagination: cursor-not-found reset to first page, cursor-at-last-item empty result, default limit (100), empty directory, stat failure exclusion, totalCount consistency across pages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 14 new tests covering getModeratorSynthesisPrompt, moderator lifecycle (spawn → send → kill), and synthesis pipeline integration (spawn → synthesis → cleanup with early-return guard paths). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove `parsedJson` from `raw` object literals in 5 output parsers.
The `raw` property on AgentError has a narrow type ({exitCode?, stderr?,
stdout?, errorLine?}) that doesn't include `parsedJson`. The parsed JSON
data is correctly placed at the top-level `parsedJson` field instead.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Latest Update: Performance Fixes + GEMINI-RC-04 Fix Tasks CompletePerformance & Memory Fixes (
|
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
5 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
Changes by area
Core agent plumbing (types, definitions, capabilities)
gemini-clitoToolTypeunion,AGENT_DEFINITIONS, andAGENT_CAPABILITIESbatchModeArgs(-y),jsonOutputArgs(--output-format stream-json),resumeArgs(--resume),modelArgs(-m),yoloModeArgs,workingDirArgs(--include-directories)gemini-clito everyRecord<ToolType>map, hardcoded array, andVALID_MODERATOR_AGENT_IDSOutput parser (
gemini-output-parser.ts)init,message,tool_use,tool_result,error,resultmodels.{name}.tokensobject)parameters→inputon tool events for renderer compatibility with OpenCode/Codex shapeError handling
auth_expired,rate_limited,token_exhaustion,network_error,permission_denied,agent_crashed)Session storage (
gemini-session-storage.ts)~/.gemini/history/{project}/session-*.jsonfilesAgentSessionStorageimplementation:listSessions,listSessionsPaginated,readSessionMessages,searchSessions,deleteMessagePair,getAllNamedSessionsagentSessionOriginsStoredeleteMessagePair(creates.bakbefore modify, restores on write failure)Token usage tracking
gemini-session-statselectron-store for persisting per-turn token countsgemini-stats-listener.ts: accumulates per-turn usage (Gemini reports per-turn, not cumulative) keyed by agent session UUIDparseGeminiSessionContentfor history displayGeminiSessionStatsEventtype for the stats pipelineWorkspace sandbox handling
WorkspaceApprovalModal: user-facing modal when Gemini CLI hits sandbox violations, showing the denied path, directory contents preview, and security warningextractDeniedPath(): extracts parent directory from sandbox error messagesworkspace-approval-requestevent →process:workspace-approvalchannel → modaladditionalWorkspaceDirsonProcessConfig: approved directories passed as--include-directorieson next spawnGroup chat integration
buildGeminiWorkspaceDirArgs()helper adds--include-directoriesfor project dir, group chat folder, and home dir--no-sandboxflagskipBatchForReadOnlyguard inbuildAgentArgs()prevents-y+--approval-modeconflictText routing and thinking display
StdoutHandlerto emit both partial (delta) and complete text events correctlystreamedText+ emitted asthinking-chunkfor live streamingthinking-chunkAND viaemitDataBufferedfor immediate displayuseBatchedSessionUpdatesto preserve thinking/tool logs whenshowThinking: 'on'(not just'sticky')CLI tooling
agent-spawner.ts: new service for spawning Gemini in batch playbook/send operationsrun-playbook.tsandsend.tsto supportgemini-cliagent typelist-sessions.ts: added Gemini to session listingUI integration
AgentSelectionScreen: Gemini selectable during onboardingNewInstanceModal,NewGroupChatModal,EditGroupChatModal,AgentSelector: all include GeminiAGENT_ARTIFACTSandAGENT_TARGET_NOTESfor GeminicontextUsage.ts: handles Gemini context percentage displayKey design decisions
readOnlyArgsis empty (definitions.ts:184)Gemini CLI's
--approval-mode planrequiresexperimental.planto be enabled in~/.gemini/settings.json. Since this isn't GA,readOnlyArgsis set to[]and read-only behavior is enforced via system prompt. This means the moderator in group chat relies on prompt-level enforcement rather than CLI-level enforcement. Will re-enable when the feature goes GA.--no-sandboxfor moderator/synthesisGemini CLI's workspace sandbox blocks access to paths outside CWD. The moderator needs to coordinate across multiple participant workspaces, so disabling the sandbox is necessary. Combined with read-only mode (even if only prompt-enforced), this is an acceptable tradeoff.
Duplicated
buildGeminiWorkspaceDirArgshelperThis helper exists in both
group-chat-agent.tsandgroup-chat-router.tsto avoid a circular import dependency. The function is small (~15 lines) and the duplication is preferable to restructuring the module graph.Per-turn token accumulation
Gemini CLI reports usage per-turn (not cumulative like Claude Code). The
gemini-stats-listeneraccumulates these into a persistent store keyed by agent session UUID, then merges them when session history is loaded.Workspace approval flow
Rather than auto-approving directories, we show a modal with the denied path, directory contents preview, and a security warning. The user must explicitly click "Approve & Restart". The approved directory is stored on the session and passed as
--include-directorieson the next spawn.Security considerations
--no-sandboxon moderator is mitigated by read-only mode (prompt-enforced) and scoped to group chat onlyadditionalWorkspaceDirsvalues originate from Gemini's own error messages and require explicit user approval via modalchild_process.spawn(not shell-interpreted), preventing command injection~/.gemini/history/usingpath.resolve()+path.join()Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements