feat: multi-agent system with CLI wrappers and chat UI#49
Merged
Conversation
Establish the foundational type system and IPC contract for the agent MVP. Adds shared types for chat, streaming, tools, MCP, permissions, and LLM config in agentTypes.ts, 14 new IPC channels (10 chat + 4 MCP), matching WindowApi methods, preload bridge implementations, and 17 tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add streaming LLM client with provider adapter pattern — Anthropic and
OpenAI-compatible adapters (covers OpenAI, Ollama, Together, Groq).
Shared SSE parser, ${env:VAR} API key interpolation, AbortController
cancellation. Move all test files from packages/*/src/ to tests/unit/
with @shared/@main/@Renderer vitest aliases.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 built-in tools (file_read, file_write, file_list, terminal_exec, search_files, git_status, git_diff, browser_read) with JSON Schema inputs and direct main-process executors. ToolRegistry provides mode-filtered listing, LLM-ready conversion, and dynamic MCP tool registration for Step 7. 53 new tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AgentManager orchestrates the core agent loop: prompt assembly, LLM streaming via LlmClient, tool execution with Promise-based approval gates, retry tracking (5 per tool), configurable turn limits, and chat persistence to .aide/local/chat.json. All 7 CHAT_* IPC handlers wired in index.ts. Lifecycle integrated with activateWorkspace/finishQuit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Chat panel (chatPane) with ModeSelector (Ask/Edit/Agent), MessageList with sticky auto-scroll, MessageBubble with markdown rendering and streaming cursor, ToolCallCard with inline Allow/Deny approval, ChatInput with auto-resize and stop button, WorkingSetPicker for edit mode. useChat hook manages IPC subscriptions with rAF-throttled streaming. Shared markdownRenderer.ts extracted from MarkdownPreviewPane. Cmd+K Cmd+A keybinding and agent.open command to open the panel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Agent > LLM Configuration section in Settings with provider dropdown,
model input, masked API key field (Show/Hide toggle, ENV badge for
${env:VAR} syntax), optional base URL, max turns, and max tokens.
loadLlmConfig() now reads from settings cascade instead of hardcoded
values. Agent config updates push to AgentManager immediately on change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three-tier permission system (confirm/auto-approve/autopilot) with per-tool overrides and glob pattern matching for terminal commands. Settings UI under Agent > Permissions with ToolPermissionsEditor for per-tool configuration. Auto-approved tool calls display an "auto" badge in ToolCallCard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Logs config state, API key resolution, request params, and response status across loadLlmConfig, LlmClient, AnthropicProvider, and AgentManager to surface silent failures during agent chat. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… and settings editor Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wrap external CLI agents as monitored sessions inside the IDE. Users can select built-in, Claude Code, or Codex as their agent backend per workspace. Claude Code is spawned per-message with --output-format stream-json and --resume for conversation continuity. Structured JSON output is parsed and normalized into a chat-like UI with streaming, tool progress, and status indicators. Cost display is omitted for CLI sessions since they use the user's own plan authentication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…(Step 8) Introduces a full conversation persistence and browsing layer so each agent pane can load a specific named conversation instead of always reopening the single most-recent session. ## New files - `packages/shared/src/conversationTypes.ts` — `ConversationMeta`, `ConversationCreateOpts`, `ConversationListChangedPayload` types, and `deriveTitle()` helper (auto-title from first message). - `packages/main/src/conversationStore.ts` — `ConversationStore` class: reads/writes conversation metadata index and message payloads under `.aide/local/conversations/<id>/`. Supports create, get, getMostRecent, loadIndex, saveMessages, loadMessages, updateMeta, delete. - `packages/renderer/src/components/panes/ChatHistoryPane.tsx` — new dockview panel for browsing conversation history (create, rename, delete, open in new pane). - `packages/renderer/src/hooks/useConversationHistory.ts` — hook backing ChatHistoryPane; subscribes to `onConversationListChanged` for live updates. - `packages/renderer/src/styles/chat-history-pane.css` — styles for the history panel. ## AgentManager changes - Removed single `workspaceSessions` map; sessions are now keyed by their conversation ID from the store. - `getHistory(workspaceId, conversationId?)` — loads a specific conversation when `conversationId` is provided, falls back to most-recent, falls back to legacy single-file path when no store is available. - `persistSession` writes to `ConversationStore` when available (messages + metadata), retaining the legacy atomic-write path as fallback. - `maybeAutoTitle` — on the very first user message, derives a title via `deriveTitle()`, updates store metadata, and broadcasts `CONVERSATION_LIST_CHANGED` to the renderer. - `destroy()` is now async and persists all in-memory sessions before clearing. ## CliAgentManager changes - Same multi-conversation refactor: removed `workspaceSessions` map, sessions keyed by conversation ID. - `start(workspaceId, backend, conversationId?)` — resumes an existing conversation by loading its messages and `claudeSessionId` (for `--resume`) from the store. - `getSessionById(sessionId)` added alongside `getSession(workspaceId)`. - Persists `claudeSessionId` to the store whenever the SDK emits a `session_id` event, enabling future `--resume` support. - `destroy()` is now async with full session persistence. - `maybeAutoTitle` mirrors the built-in agent implementation. ## IPC / preload - 6 new IPC channels in `IpcChannels`: `CONVERSATION_LIST`, `CONVERSATION_CREATE`, `CONVERSATION_DELETE`, `CONVERSATION_RENAME`, `CONVERSATION_GET`, `CONVERSATION_LIST_CHANGED`. - New IPC handlers in `index.ts` for all 6 channels; `CONVERSATION_CREATE` and `CONVERSATION_DELETE`/`RENAME` push `CONVERSATION_LIST_CHANGED` to the renderer after each mutation. - `ConversationStore` instantiated per workspace in `activateWorkspace` and passed to both managers. - `chatGetHistory` and `cliAgentStart` handlers now forward optional `conversationId`. - Preload bridge exposes: `conversationList`, `conversationCreate`, `conversationDelete`, `conversationRename`, `conversationGet`, `onConversationListChanged`. ## Renderer - `ChatPane` / `CliAgentPane` accept optional `conversationId` panel param and auto-update the dockview tab title when `conversationTitle` changes. - `useChat` / `useCliAgent` accept `conversationId`, load conversation title from the store on mount, and subscribe to `onConversationListChanged` for live title updates; both expose `conversationTitle`. - `AppShell` registers `agent.history.open` command to open/focus the `ChatHistoryPane` panel. - `DockviewContainer` registers the `chatHistoryPane` component type. - `shared/src/index.ts` exports new conversation types and `deriveTitle`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdds a full agent/chat subsystem: provider-agnostic LLM client and adapters, streaming control loop with tool-call approval and execution, CLI-backed agent integration, conversation persistence, renderer UI (chat/CLI/history panes, hooks, controls), settings/schema for agent configuration, shared types, and comprehensive unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Renderer as Chat UI (Renderer)
participant Preload as Preload IPC
participant Main as AgentManager (Main)
participant LLM as LlmClient (Main)
participant Tools as ToolRegistry/AgentTools (Main)
User->>Renderer: sendMessage(content)
Renderer->>Preload: chatSendMessage(sessionId, content)
Preload->>Main: sendMessage(sessionId, content)
loop Agent loop (maxTurns)
Main->>LLM: stream(messages, tools, requestId)
LLM-->>Main: LlmStreamEvent (text_delta/tool_use/message_end)
Main->>Renderer: CHAT_STREAM_CHUNK (text delta)
alt tool_use requested
Main->>Main: evaluate auto-approve (tier + patterns)
alt needs approval
Main->>Renderer: CHAT_TOOL_CALL (pending)
Renderer->>User: show approval UI
User->>Renderer: approve/reject
Renderer->>Preload: chatToolApprove/chatToolReject
Preload->>Main: approve/reject
end
Main->>Tools: execute(toolCall)
Tools-->>Main: ToolResult
Main->>LLM: feed tool_result into next turn
end
end
Main->>Renderer: CHAT_STREAM_END (stopReason)
sequenceDiagram
participant User
participant CliUI as CLI Agent UI (Renderer)
participant Preload as Preload IPC
participant CliMgr as CliAgentManager (Main)
participant Process as External CLI (Claude Code)
participant Store as ConversationStore
User->>CliUI: startAgent(backend)
CliUI->>Preload: cliAgentStart(...)
Preload->>CliMgr: start(...)
CliMgr->>Store: loadMessages(conversationId?)
CliMgr->>Process: spawn CLI (with resume if present)
Process-->>CliMgr: stdout stream (NDJSON)
CliMgr->>CliMgr: parse NDJSON -> events
CliMgr->>CliUI: CLI_AGENT_STREAM_DELTA / CLI_AGENT_MESSAGE / CLI_AGENT_STATUS
User->>CliUI: send(prompt)
CliUI->>Preload: cliAgentSend(sessionId, prompt)
Preload->>CliMgr: send -> writes to Process
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
packages/renderer/src/components/chat/WorkingSetPicker.tsx-55-69 (1)
55-69:⚠️ Potential issue | 🟡 MinorGive the icon-only buttons accessible names.
The add button and each remove button currently expose only “+” / “×” as their labels. Screen-reader users won't get the actual action or file name, which makes working-set editing much harder.
♿ Suggested fix
<span key={path} className="chat-working-set__chip" title={path}> {basename(path)} <button className="chat-working-set__chip-remove" + aria-label={`Remove ${basename(path)} from working set`} onClick={() => removeFile(path)} > × </button> </span> ))} <button className="chat-working-set__add" + aria-label="Add file to working set" onClick={() => setDropdownOpen(!dropdownOpen)} > + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/chat/WorkingSetPicker.tsx` around lines 55 - 69, The icon-only buttons in WorkingSetPicker lack accessible names; update the remove button (inside the map that calls removeFile(path)) to include an aria-label (or accessible name) that includes the file name/path, e.g. "Remove {basename(path)} from working set", and update the add button (that toggles setDropdownOpen/dropdownOpen) to have an aria-label like "Add file to working set" so screen readers receive clear actionable text.packages/shared/src/conversationTypes.ts-66-72 (1)
66-72:⚠️ Potential issue | 🟡 MinorPrevent empty conversation titles from whitespace-only input.
If input is blank/whitespace,
deriveTitlecurrently returns''(Line 68 path), which can produce untitled/empty history rows. Add a fallback title.🛠️ Suggested fix
export function deriveTitle(content: string): string { - const cleaned = content.trim().replace(/\n+/g, ' ') + const cleaned = content.trim().replace(/\s+/g, ' ') + if (!cleaned) return 'New Chat' if (cleaned.length <= 40) return cleaned const truncated = cleaned.slice(0, 40) const lastSpace = truncated.lastIndexOf(' ') return (lastSpace > 20 ? truncated.slice(0, lastSpace) : truncated) + '...' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/conversationTypes.ts` around lines 66 - 72, deriveTitle currently returns an empty string when content is blank/whitespace because cleaned becomes ''—update deriveTitle to detect empty cleaned and return a sensible fallback title (e.g., "Untitled conversation" or "New conversation") before applying truncation logic; specifically, inside deriveTitle (variables: cleaned, truncated, lastSpace) add a guard like if (!cleaned) return '<fallback>' so history rows never get an empty title.packages/renderer/src/styles/cli-agent-pane.css-124-128 (1)
124-128:⚠️ Potential issue | 🟡 MinorReplace deprecated
word-break: break-word.Same issue as above—use
overflow-wrap: break-wordfor standard compliance.🔧 Suggested fix
.cli-agent-msg { padding: 4px 0; - word-break: break-word; + overflow-wrap: break-word; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/cli-agent-pane.css` around lines 124 - 128, The CSS rule for the .cli-agent-msg selector uses the deprecated property `word-break: break-word`; replace it with the standard `overflow-wrap: break-word` (and optionally keep `word-break: break-word` only as a non-standard fallback if desired) so update the .cli-agent-msg block to use `overflow-wrap: break-word` to ensure standard-compliant wrapping behavior.packages/renderer/src/styles/cli-agent-pane.css-89-94 (1)
89-94:⚠️ Potential issue | 🟡 MinorReplace deprecated
word-break: break-word.Stylelint flags
word-break: break-wordas deprecated. Useoverflow-wrap: break-wordoroverflow-wrap: anywhereinstead for standard-compliant word wrapping.🔧 Suggested fix
.cli-agent-pane__error-text { - word-break: break-word; + overflow-wrap: break-word; white-space: pre-wrap; max-height: 80px; overflow-y: auto; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/cli-agent-pane.css` around lines 89 - 94, The CSS rule .cli-agent-pane__error-text uses the deprecated property word-break: break-word; replace it with a standards-compliant overflow-wrap declaration (e.g., overflow-wrap: break-word or overflow-wrap: anywhere) and keep the existing white-space, max-height, and overflow-y rules intact so wrapping behavior is preserved while satisfying stylelint and browsers.docs/agentmvp.md-359-369 (1)
359-369:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks.
Line 359 and Line 388 start unlabeled fenced blocks, which triggers markdownlint MD040.
📝 Suggested fix
-``` +```text AgentTerminalPane (or mode on existing TerminalPane) ↓ Spawn CLI: `claude --output-format stream-json` or `codex --full-auto` ... -``` +``` -``` +```text Claude Code CLI needs permission ↓ Calls aide_permission_prompt MCP tool (stdio to cliPermissionServer.ts) ... -``` +```Also applies to: 388-404
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agentmvp.md` around lines 359 - 369, Add language identifiers to the unlabeled fenced code blocks in docs/agentmvp.md: locate the block starting with "AgentTerminalPane (or mode on existing TerminalPane)" and the block starting with "Claude Code CLI needs permission" and change the opening fence from ``` to ```text (or ```plain) so markdownlint MD040 is satisfied; ensure both fenced blocks that contain the flow diagrams are labeled consistently.packages/renderer/src/components/chat/ToolCallCard.tsx-76-77 (1)
76-77:⚠️ Potential issue | 🟡 MinorOnly
completedcalls should render as executed.
approvedis still distinct fromcompleted, butisDonetreats both as "executed". That shows a false success state while the tool may still be waiting to run.🧩 Proposed fix
- const isDone = toolCall.status === 'completed' || toolCall.status === 'approved' + const isDone = toolCall.status === 'completed'Also applies to: 141-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/chat/ToolCallCard.tsx` around lines 76 - 77, The isDone flag incorrectly treats both 'completed' and 'approved' as executed; change the check so isDone = toolCall.status === 'completed' (remove 'approved') to ensure only completed calls render as executed, and update the identical conditional used later in the same file (the second isDone/status check in ToolCallCard) to match this behavior.packages/main/src/providers/sseParser.ts-41-50 (1)
41-50:⚠️ Potential issue | 🟡 MinorReset
currentEventon every blank-line delimiter.A delimiter with no
data:currently leavescurrentEventhanging around, so the nextdata:frame inherits the previous event name. Clear the event state even when nothing is emitted.🧩 Proposed fix
if (line === '' || line === '\r') { // Empty line = event delimiter if (currentData.length > 0) { const data = currentData.join('\n') if (data !== '[DONE]') { yield { event: currentEvent, data } } - currentEvent = undefined - currentData = [] } + currentEvent = undefined + currentData = [] continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/providers/sseParser.ts` around lines 41 - 50, The SSE parser currently only clears currentEvent when currentData has content, so a blank delimiter with no data leaves currentEvent set and causes the next data: frame to inherit it; update the blank-line handling inside the sse parser (references: currentEvent, currentData) to always reset currentEvent = undefined (and clear currentData) regardless of whether currentData.length > 0 so the event name does not leak to subsequent frames.packages/renderer/src/styles/settings-pane.css-603-613 (1)
603-613:⚠️ Potential issue | 🟡 MinorHardcoded color violates theming guidelines.
Line 609 uses
rgba(76, 175, 80, 0.12)instead of a CSS variable. This breaks theme consistency and won't adapt to dark/light mode changes.🎨 Proposed fix using color-mix
.settings-password__env-badge { font-size: 9px; font-weight: 700; letter-spacing: 0.06em; text-transform: uppercase; color: var(--text-success); - background: rgba(76, 175, 80, 0.12); + background: color-mix(in srgb, var(--text-success) 12%, transparent); border-radius: 3px; padding: 2px 6px; line-height: 1; }As per coding guidelines: "Use CSS variable token system with data-theme attribute for theming".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/settings-pane.css` around lines 603 - 613, The .settings-password__env-badge rule uses a hardcoded background rgba(76, 175, 80, 0.12) which breaks theming; replace that literal with a CSS variable (e.g. derive from a --color-success token) and, if translucency is needed, use color-mix() or an alpha-capable token so the background adapts to data-theme; update the background declaration in .settings-password__env-badge to reference the theme token (and adjust any token names like --color-success or --surface to match your design system).packages/main/src/providers/anthropicProvider.ts-71-80 (1)
71-80:⚠️ Potential issue | 🟡 MinorAPI key prefix logged - consider removing for production.
Logging
apiKeyPrefix: config.apiKey.slice(0, 12) + '...'exposes part of the API key. Even partial key exposure in logs could aid attackers, especially if logs are persisted or transmitted.🔒 Suggested change
console.log('[AnthropicProvider] Sending request', { url, model: body.model, maxTokens: body.max_tokens, messageCount: body.messages.length, toolCount: body.tools?.length ?? 0, hasSystem: !!body.system, hasApiKey: !!config.apiKey, - apiKeyPrefix: config.apiKey ? config.apiKey.slice(0, 12) + '...' : '(empty)', })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/providers/anthropicProvider.ts` around lines 71 - 80, The console.log in AnthropicProvider that prints apiKeyPrefix (the call that logs '[AnthropicProvider] Sending request') is leaking part of the API key; remove the partial key output and either omit the apiKeyPrefix field or replace it with a non-sensitive placeholder like '(redacted)' or rely on the existing hasApiKey boolean instead. Update the log object in the console.log call (the one referencing body.model, body.max_tokens, body.messages, body.tools, body.system) so it no longer slices or exposes config.apiKey.packages/renderer/src/hooks/useConversationHistory.ts-51-63 (1)
51-63:⚠️ Potential issue | 🟡 MinorNon-null assertion on
workspaceIdmay cause runtime error.
createConversationusesworkspaceId!but the hook acceptsworkspaceId: string | undefined. If a consumer callscreateConversationbeforeworkspaceIdis set, this will passundefinedto the API.🛡️ Proposed guard
const createConversation = useCallback(async ( backend: AgentBackend, worktreePath?: string, worktreeBranch?: string, ): Promise<ConversationMeta> => { + if (!workspaceId) { + throw new Error('Cannot create conversation without workspaceId') + } - const meta = await window.api.conversationCreate({ - workspaceId: workspaceId!, + const meta = await window.api.conversationCreate({ + workspaceId, backend, worktreePath, worktreeBranch, }) return meta }, [workspaceId])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/hooks/useConversationHistory.ts` around lines 51 - 63, The createConversation callback uses workspaceId! which can be undefined; update createConversation (in useConversationHistory) to guard against missing workspaceId by checking if workspaceId is defined at the start of the function and failing fast (e.g., throw a descriptive Error or return a rejected Promise) before calling window.api.conversationCreate; reference the createConversation function and workspaceId variable so callers can't pass undefined to the API and ensure behavior is deterministic if workspaceId isn't yet set.packages/renderer/src/components/panes/ChatHistoryPane.tsx-51-66 (1)
51-66:⚠️ Potential issue | 🟡 MinorMissing error handling on async CRUD operations.
handleRenameSubmit,handleDelete, andhandleNewChatcall async operations without try/catch. Failures will result in unhandled promise rejections and no user feedback.🛡️ Proposed fix for handleNewChat
const handleNewChat = async (backend: AgentBackend = 'built-in') => { - const meta = await history.createConversation(backend) - params?.onOpenConversation?.(meta) + try { + const meta = await history.createConversation(backend) + params?.onOpenConversation?.(meta) + } catch (err) { + console.error('Failed to create conversation:', err) + // Consider showing a toast or inline error + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/panes/ChatHistoryPane.tsx` around lines 51 - 66, Wrap the async calls in handleRenameSubmit, handleDelete, and handleNewChat with try/catch (and finally where needed) to prevent unhandled rejections: in handleRenameSubmit call await history.renameConversation(...) inside try, setRenaming(null) in finally and show a user-facing error (e.g., toast or setError) in catch; in handleDelete ensure setContextMenu(null) is preserved but perform await history.deleteConversation(meta.id) inside try and surface errors in catch; in handleNewChat wrap await history.createConversation(backend) in try, only call params?.onOpenConversation?.(meta) on success, and show an error on failure—use the existing state mutators (setRenaming, setContextMenu) and the functions history.renameConversation, history.deleteConversation, history.createConversation to locate the changes.packages/main/src/agents/claudeNativeSessionWatcher.ts-401-433 (1)
401-433:⚠️ Potential issue | 🟡 Minorfs.watch callbacks lack error handling.
The
watch()calls at lines 407 and 421 pass callbacks that only handle normal events. Iffs.watchemits an error event (e.g.,ENOSPC,EMFILE), it won't be caught.🛡️ Suggested improvement
const tryProjectWatch = (): void => { try { this.dirWatcher?.close() - this.dirWatcher = watch(this.projectDir, () => this.scheduleRefresh()) + this.dirWatcher = watch(this.projectDir, (eventType, filename) => { + if (eventType === 'error') { + console.error('[ClaudeNativeSessionWatcher] Watch error on project dir') + return + } + this.scheduleRefresh() + }) + this.dirWatcher.on('error', (err) => { + console.error('[ClaudeNativeSessionWatcher] Watch error:', err) + }) } catch { this.dirWatcher = null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agents/claudeNativeSessionWatcher.ts` around lines 401 - 433, The fs.watch callbacks in attachDirWatch (including the inner tryProjectWatch and the parentWatcher callback) don't attach 'error' handlers, so runtime errors from the watchers (e.g., ENOSPC/EMFILE) are unhandled; update the logic where dirWatcher and parentWatcher are created to attach an explicit 'error' listener (e.g., watcher.on('error', err => { log or handle and close the watcher, set dirWatcher/parentWatcher to null })) and ensure you close/clear the corresponding watcher references inside those handlers and call scheduleRefresh or runDebouncedRefresh as appropriate; reference the functions/fields attachDirWatch, tryProjectWatch, dirWatcher, parentWatcher, scheduleRefresh, and runDebouncedRefresh when making changes.packages/renderer/src/hooks/useChat.ts-134-145 (1)
134-145:⚠️ Potential issue | 🟡 MinorNon-null assertion on
workspaceIdin effect callback may be stale.At line 136,
workspaceId!is used inside theonChatStreamEndcallback. IfworkspaceIdbecomesundefinedafter the effect runs but before the callback fires, this will passundefinedto the API.🛡️ Proposed guard
// Refresh session to get full message history with tool results - if (sessionIdRef.current) { + if (sessionIdRef.current && workspaceId) { - window.api.chatGetHistory(workspaceId!, end.sessionId).then((session: ChatSession | null) => { + window.api.chatGetHistory(workspaceId, end.sessionId).then((session: ChatSession | null) => { if (!session) return messagesRef.current = session.messages setStatus(session.status) tick() }) } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/hooks/useChat.ts` around lines 134 - 145, The onChatStreamEnd callback uses a non-null assertion on workspaceId (workspaceId!) which can become stale; capture the current workspaceId into a local const (e.g., const currentWorkspaceId = workspaceId) or explicitly guard before calling window.api.chatGetHistory, then call window.api.chatGetHistory(currentWorkspaceId, end.sessionId) only if currentWorkspaceId is defined; keep the rest of the logic (checking sessionIdRef.current, updating messagesRef.current, setStatus and tick) unchanged so you avoid passing undefined to window.api.chatGetHistory.packages/shared/src/index.ts-793-793 (1)
793-793:⚠️ Potential issue | 🟡 MinorCorrupted unicode characters in comment.
Line 793 contains corrupted unicode:
// ─── MCP ───────────────────────────────────────. This appears to be rendering issues with box-drawing characters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/index.ts` at line 793, The comment line containing " // ─── MCP ���───────────────────────────────���───── " has corrupted box-drawing characters; replace that commented divider with a stable ASCII or valid Unicode alternative (e.g., "// --- MCP ------------------------------------" or use proper em/en-dashes) so the file no longer contains replacement glyphs; update the comment near the existing "MCP" marker in packages/shared/src/index.ts to use the chosen clean divider string.packages/main/src/agentManager.ts-320-328 (1)
320-328:⚠️ Potential issue | 🟡 MinorLogging API key prefix in agent loop could be a security concern.
Line 327 logs
apiKeyPrefix: this.config.apiKey ? this.config.apiKey.slice(0, 8) + '...'. While only 8 characters are logged, this partial key exposure in logs could aid in key enumeration or brute-force attacks. Consider removing or reducing to just a booleanhasApiKey.🔒 Proposed fix to remove API key logging
console.log('[AgentManager] Starting agent loop', { sessionId: session.id, mode: session.mode, maxTurns: this.config.maxTurns, provider: this.config.provider, model: this.config.model, hasApiKey: !!this.config.apiKey, - apiKeyPrefix: this.config.apiKey ? this.config.apiKey.slice(0, 8) + '...' : '(empty)', })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentManager.ts` around lines 320 - 328, The console.log in AgentManager that prints the agent loop start currently includes a partial API key via this.config.apiKey.slice(0, 8) — remove the apiKeyPrefix property entirely and only log the boolean hasApiKey (keep sessionId, mode, maxTurns, provider, model) so no portion of the API key is written to logs; update the log invocation that builds the object (the console.log in the agent loop startup) to stop referencing this.config.apiKey.slice(...) and rely solely on !!this.config.apiKey for key presence.packages/main/src/agentManager.ts-591-593 (1)
591-593:⚠️ Potential issue | 🟡 MinorwaitForApproval creates a Promise that may never resolve.
If the user navigates away, closes the chat pane, or the session is otherwise abandoned without calling
approveToolCall,rejectToolCall, orstop, this Promise will remain pending indefinitely. Consider adding a timeout or cleanup mechanism.⏱️ Consider adding a timeout
private waitForApproval(toolCall: ToolCall): Promise<boolean> { return new Promise<boolean>((resolve) => { this.pendingApprovals.set(toolCall.id, { resolve }) + // Auto-reject after 10 minutes to prevent indefinite hangs + setTimeout(() => { + if (this.pendingApprovals.has(toolCall.id)) { + this.pendingApprovals.delete(toolCall.id) + resolve(false) + } + }, 10 * 60 * 1000) }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentManager.ts` around lines 591 - 593, waitForApproval currently inserts a Promise into pendingApprovals that can remain pending forever if approveToolCall, rejectToolCall, or stop are never invoked; add a timeout and cleanup so the Promise always settles: when creating the pending entry in waitForApproval, start a timer (e.g., configurable DEFAULT_APPROVAL_TIMEOUT) that rejects the Promise with a timeout error and deletes the pendingApprovals entry, and store the timer handle on the pending object; when approveToolCall, rejectToolCall, or stop resolve/reject the pending approval, clear the timeout and remove the entry from pendingApprovals to avoid leaks. Ensure symbols referenced are waitForApproval, this.pendingApprovals, approveToolCall, rejectToolCall, and stop.
🧹 Nitpick comments (24)
CLAUDE.md (1)
35-35: Clarify path semantics and phrasing in testing guideline.Use repo-relative
tests/(without leading/) and sentence-case wording to avoid ambiguity and keep contributor docs consistent.✏️ Suggested doc tweak
-- /tests is the home for all tests. make sure all new tests get placed in this folder. +- `tests/` is the home for all tests. Make sure all new tests are placed in this folder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` at line 35, Update the guideline sentence to use a repo-relative path and sentence-case: replace "/tests is the home for all tests. make sure all new tests get placed in this folder." with something like "tests/ is the home for all tests. Make sure all new tests are placed in this folder." — remove the leading "/" from the path and capitalize the sentence-start to follow repo-relative semantics and consistent phrasing.packages/renderer/src/styles/chat-history-pane.css (1)
211-219: Move the context-menu shadow into a theme token.
rgba(0, 0, 0, 0.3)is the one hard-coded color in this sheet, so the menu shadow will stay dark regardless of the active theme. Please route this through the shared CSS variable/token system instead.As per coding guidelines, "Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/chat-history-pane.css` around lines 211 - 219, The context menu CSS (.chat-history-pane__context-menu) uses a hard-coded shadow color (box-shadow: 0 4px 16px rgba(0, 0, 0, 0.3)); replace that literal with a theme token (e.g. var(--menu-shadow) or var(--shadow-elev-3)) and update the box-shadow declaration to use the token; then add the corresponding CSS variable definitions for light and dark themes via the project's data-theme token system so the shadow color adapts to theme changes.packages/renderer/src/styles/chat-pane.css (1)
137-150: Use a theme token for the working-set dropdown shadow.The hard-coded shadow color here bypasses the theme token system, so it will stay tuned for a dark surface even when the app is using a light theme. Please move this shadow color into a shared CSS variable.
As per coding guidelines, "Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/chat-pane.css` around lines 137 - 150, The box-shadow on .chat-working-set__dropdown uses a hard-coded color (rgba(0, 0, 0, 0.3)) which bypasses the app theme tokens; change it to use a CSS variable token (e.g., --shadow-elevation or --shadow-overlay) and update the styles to reference that variable for box-shadow so the dropdown follows data-theme light/dark modes; ensure the new token is defined in the shared theme variables and used here in the .chat-working-set__dropdown declaration (replace the hard-coded box-shadow value with a reference to the new CSS variable).tests/unit/agentTools.test.ts (1)
9-31: Usemkdtempfor safer isolated temp fixtures.Line 9’s timestamp-based directory is usually fine, but
mkdtempavoids collision/stale-state edge cases in parallel or retried runs.♻️ Suggested change
-import { writeFileSync, mkdirSync, rmSync } from 'fs' +import { writeFileSync, mkdirSync, rmSync, mkdtempSync } from 'fs' import { join } from 'path' import { tmpdir } from 'os' @@ -const TEST_DIR = join(tmpdir(), `agentTools-test-${Date.now()}`) +const TEST_DIR = mkdtempSync(join(tmpdir(), 'agentTools-test-'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/agentTools.test.ts` around lines 9 - 31, The timestamp-based TEST_DIR should be replaced with a safely-created temporary directory using mkdtemp (or mkdtempSync) to avoid collisions; update the TEST_DIR initialization to call fs.mkdtempSync(path.join(tmpdir(), 'agentTools-test-')) and ensure defaultContext.workspaceRoot references that variable, then move the file/directory creation into the beforeAll block as now and ensure afterAll still rmSync(TEST_DIR, { recursive: true, force: true }); also add the required fs import for mkdtempSync if missing and remove the old timestamp-based expression.packages/renderer/src/components/chat/AgentStatusDot.tsx (1)
19-22: Consider adding explicit accessibility labeling for the status indicator.Since this is a non-text status glyph, adding
aria-label(orrole="img"+ label) would improve screen-reader clarity beyondtitle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/chat/AgentStatusDot.tsx` around lines 19 - 22, The status glyph currently rendered in AgentStatusDot uses only title for UX; update the span to include an accessible label by adding aria-label={STATUS_LABELS[status]} (or role="img" plus aria-label={STATUS_LABELS[status]}) so screen readers announce the status; ensure to keep the existing className and title and use the STATUS_LABELS mapping and the status prop when constructing the aria-label.packages/renderer/src/components/WorktreePanel/WorktreeItem.tsx (1)
22-22: Update component docs for the new required prop.
onStartAgentis now part of the public props API, but the JSDoc param list still omits it. Please add it to keep the contract accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/WorktreePanel/WorktreeItem.tsx` at line 22, The JSDoc for the WorktreeItem component is missing the new required prop onStartAgent; update the component's doc comment above the WorktreeItem export so the `@param` list includes onStartAgent (describe its type/behavior consistently with other props), and ensure Props and the JSDoc stay in sync with the WorktreeItem({ worktree, onClick, onContextMenu, onOpenTerminal, onStartAgent, onMoreActions }: Props) signature.packages/renderer/src/styles/cli-agent-pane.css (1)
53-69: Consider using a CSS variable for button text color.The button text uses hardcoded
#fff. For consistency with the theming system, consider using a CSS variable (e.g.,var(--text-on-accent,#fff)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/cli-agent-pane.css` around lines 53 - 69, Replace the hardcoded white text color in the button styles with a theme variable: change color: `#fff` in .cli-agent-pane__btn--start and .cli-agent-pane__btn--stop to use a CSS variable such as color: var(--text-on-accent, `#fff`) so the buttons follow the theming system while keeping the fallback; no other behavior changes required.tests/unit/sseParser.test.ts (1)
110-118: Minor: Redundant.map((c) => c)call.
full.split('')already returnsstring[], so the.map((c) => c)is a no-op.🔧 Simplified version
it('handles many small chunks', async () => { const full = 'event: test\ndata: {"ok":true}\n\n' - const chunks = full.split('').map((c) => c) // one char per chunk + const chunks = full.split('') // one char per chunk const stream = createStream(chunks)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/sseParser.test.ts` around lines 110 - 118, The test "handles many small chunks" creates `chunks` with a redundant `.map((c) => c)` after `full.split('')`; remove the no-op mapping and set `chunks` directly from `full.split('')` (used in the test with `createStream` and `collectEvents`) so the test is simpler and functionally identical.tests/unit/llmClient.test.ts (1)
200-221: Make the env-var construction test deterministic and cleanup-safe.This promise-chaining pattern can be flaky, and
unstubAllEnvsonly runs on success. Preferasync/await+finally.✅ Suggested refactor
- it('resolves env vars in API key on construction', () => { - vi.stubEnv('MY_KEY', 'secret-123') - const client = new LlmClient({ ...defaultConfig, apiKey: '${env:MY_KEY}' }) + it('resolves env vars in API key on construction', async () => { + vi.stubEnv('MY_KEY', 'secret-123') + try { + const client = new LlmClient({ ...defaultConfig, apiKey: '${env:MY_KEY}' }) - // Verify by checking that the provider receives the resolved key - let receivedKey = '' - const spyProvider: LlmProvider = { - async *stream(_params, config) { - receivedKey = config.apiKey - }, - } - client.registerProvider('anthropic', spyProvider) + // Verify by checking that the provider receives the resolved key + let receivedKey = '' + const spyProvider: LlmProvider = { + async *stream(_params, config) { + receivedKey = config.apiKey + }, + } + client.registerProvider('anthropic', spyProvider) - // Drain the generator - const gen = client.stream({ requestId: 'r-env', messages: [{ role: 'user', content: [{ type: 'text', text: 'Hi' }] }] }) - gen.next().then(() => gen.return(undefined)) + await collectEvents( + client.stream({ + requestId: 'r-env', + messages: [{ role: 'user', content: [{ type: 'text', text: 'Hi' }] }], + }), + ) - // Check after microtask - return gen.next().then(() => { expect(receivedKey).toBe('secret-123') + } finally { vi.unstubAllEnvs() - }) + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/llmClient.test.ts` around lines 200 - 221, Convert the test "resolves env vars in API key on construction" to an async function, use try/finally to ensure vi.unstubAllEnvs() always runs, and await the generator operations to make the flow deterministic: call vi.stubEnv('MY_KEY', 'secret-123'), construct LlmClient, register the spyProvider (using LlmClient.registerProvider and the spy's async *stream to capture config.apiKey), then await the first gen.next() and await gen.return(undefined) in the finally block before asserting receivedKey === 'secret-123'; ensure all awaits target the generator returned by LlmClient.stream so the test deterministically resolves and always cleans up the env stub.packages/main/src/index.ts (2)
373-383: Consider removing or gating verbose diagnostic logging.The
console.loginloadLlmConfigincludes potentially sensitive metadata (API key length, whether it's an env reference). While useful for debugging, consider gating behind a debug flag or removing before release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/index.ts` around lines 373 - 383, The console.log in loadLlmConfig is emitting sensitive metadata (apiKey length and env-ref check); change this to either remove the verbose log or gate it behind a debug flag/logger level and stop printing sensitive fields—use an opt-in debug environment variable or the existing app logger (e.g., replace console.log with a debug-only logger check) and redact or omit config.apiKey, apiKeyLength and apiKeyIsEnvRef while keeping safe items (provider, model, baseUrl, storeHasEditorDefaults, storeKeys) for diagnostics; update loadLlmConfig and any callers to respect the debug flag.
571-599: Consider extracting the conversation ID resolution logic.The
CLI_AGENT_LOAD_MESSAGEShandler has multiple lookup strategies (cache by id, cache by claudeSessionId, prefix parsing, store fallback). This logic could be extracted into a helper function for testability and clarity.♻️ Suggested refactor
// Extract to a helper function: async function resolveAndLoadMessages( conversationId: string, nativeCache: ConversationMeta[], watcher: ClaudeNativeSessionWatcher | null, store: ConversationStore | null, ): Promise<CliAgentMessage[]> { // Native session by id or claudeSessionId const nativeMeta = nativeCache.find((c) => c.id === conversationId) ?? nativeCache.find((c) => c.claudeSessionId === conversationId) if (nativeMeta?.source === 'claude-native' && nativeMeta.claudeSessionId && watcher) { return watcher.loadMessages(nativeMeta.claudeSessionId) } // Native session by prefix const nativePrefix = 'claude-native:' if (conversationId.startsWith(nativePrefix) && watcher) { const rawId = conversationId.slice(nativePrefix.length) if (/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(rawId)) { return watcher.loadMessages(rawId) } } // Fallback to conversation store const stored = await store?.loadMessages(conversationId) if (!stored || typeof stored !== 'object') return [] const msgs = (stored as { messages?: unknown }).messages return Array.isArray(msgs) ? (msgs as CliAgentMessage[]) : [] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/index.ts` around lines 571 - 599, Extract the conversation ID resolution and load logic from the IpcChannels.CLI_AGENT_LOAD_MESSAGES handler into a helper (e.g., resolveAndLoadMessages) that accepts (conversationId, nativeSessionCache, nativeSessionWatcher, conversationStore) and returns Promise<CliAgentMessage[]>; move the native lookup by id/claudeSessionId (using nativeSessionCache.find), the 'claude-native:' prefix parsing/UUID check, and the conversationStore fallback (conversationStore.loadMessages and messages extraction) into that helper, then replace the inline logic in the ipcMain.handle callback to call resolveAndLoadMessages(conversationId, nativeSessionCache, nativeSessionWatcher, conversationStore) and return its result. Ensure the helper uses the same checks for nativeMeta.source === 'claude-native', presence of claudeSessionId, and the UUID regex before calling nativeSessionWatcher.loadMessages.packages/renderer/src/hooks/useCliAgent.ts (1)
253-254: Clarify thevoid renderTickpattern with a comment.The
void renderTickstatement forces the component to re-readmessagesRef.currentwhenrenderTickchanges. This is a clever pattern but non-obvious.📝 Suggested documentation
- // Force re-read of ref on renderTick change + // renderTick is intentionally referenced here (not just in the dependency array). + // This ensures the returned messagesRef.current reflects the latest state when tick() is called. void renderTick🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/hooks/useCliAgent.ts` around lines 253 - 254, Add an explanatory inline comment above the `void renderTick` line in useCliAgent.ts describing that the `void renderTick` statement is intentionally used to reference the `renderTick` variable so React/TypeScript re-evaluates the hook body and thus re-reads `messagesRef.current` when `renderTick` changes; reference the `renderTick` identifier and `messagesRef` so future readers understand this deliberate no-op side-effect and why it’s preferred here over other patterns.packages/renderer/src/components/panes/ChatHistoryPane.tsx (1)
158-176: Context menu may overflow viewport bounds.Using raw
clientX/clientYfor positioning can cause the menu to render partially off-screen when triggered near viewport edges.🔧 Consider clamping position
+ const menuWidth = 120 // approximate + const menuHeight = 80 // approximate + const x = Math.min(contextMenu.x, window.innerWidth - menuWidth) + const y = Math.min(contextMenu.y, window.innerHeight - menuHeight) <div className="chat-history-pane__context-menu" - style={{ left: contextMenu.x, top: contextMenu.y }} + style={{ left: x, top: y }} >packages/main/src/agentTools.ts (3)
36-38: Windows shell detection is included but project doesn't support Windows.Per learnings, aIDE does not support Windows. The
detectShellfunction includes a Windows code path (powershell.exe) that will never execute. This is harmless defensive coding but could be simplified. Based on learnings from this repository.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentTools.ts` around lines 36 - 38, The detectShell function contains a Windows-specific branch that isn't needed; remove the process.platform === 'win32' check and simplify detectShell (the function name) to return process.env.SHELL if set, otherwise return '/bin/zsh' when process.platform === 'darwin' and '/bin/bash' for all other platforms so the Windows-only 'powershell.exe' path is eliminated.
276-302: Ripgrep spawn handles maxResults correctly but stderr is not captured.The implementation kills the process when
maxResultsis reached, which is efficient. However,stderris connected to'pipe'but never consumed, which could cause issues if ripgrep writes significant error output. Consider either settingstderr: 'ignore'or consuming it for error reporting.♻️ Consider handling stderr
- const proc = spawn(rgPath, args, { stdio: ['ignore', 'pipe', 'pipe'] }) + const proc = spawn(rgPath, args, { stdio: ['ignore', 'pipe', 'ignore'] })Or alternatively, collect stderr for error reporting:
let stderrBuffer = '' proc.stderr?.on('data', (chunk: Buffer) => { stderrBuffer += chunk.toString() }) // ... use stderrBuffer in error handling🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentTools.ts` around lines 276 - 302, The ripgrep child process spawns with stderr piped but never consumed, which can hang or lose diagnostics; update the spawn/cleanup logic around the proc created with spawn(rgPath, args, { stdio: ['ignore','pipe','pipe'] }) to either set stderr to 'ignore' or consume it by attaching proc.stderr?.on('data', chunk => stderrBuffer += chunk.toString()) and capture that stderrBuffer for error reporting in the proc 'close'/'error' handling and when you call proc.kill(); ensure you reference proc and stderrBuffer (or change stdio to ignore) so stderr is not left unread.
188-207: Recursive directory walk has no depth limit.The
recursiveListfunction will traverse the entire directory tree without any depth limit. For very deep or large directory structures, this could be slow or cause stack issues (though async/await mitigates stack overflow). TheSKIPset provides some protection by excluding common large directories likenode_modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentTools.ts` around lines 188 - 207, The recursiveList implementation (function recursiveList and its inner walk) currently traverses without a depth limit; add a maxDepth parameter (default e.g. 10) to recursiveList and thread a depth counter into walk so you only recurse when currentDepth < maxDepth, skipping deeper directories while still collecting files at or above the limit; keep the SKIP set behavior and preserve the relative(rootPath, full) logic, and optionally return or annotate when the listing was truncated so callers can detect depth-limited results.packages/main/src/agentManager.ts (1)
643-648: Glob matching is basic and may not handle all patterns.The
globMatchfunction implements simple*wildcards only. Patterns with?,**, or character classes won't work as expected. This is documented implicitly by the simple implementation, but users may expect full glob syntax.📝 Consider using a glob library or documenting limitations
For more complete glob support, consider using a library like
minimatchorpicomatch. Alternatively, document in the UI that only simple*wildcards are supported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/agentManager.ts` around lines 643 - 648, The current globMatch function only supports '*' and will mis-handle '?', '**', character classes, etc.; update globMatch to use a well-tested glob library (e.g., import and call minimatch or picomatch) so patterns are evaluated correctly (replace the body of private globMatch(text: string, pattern: string) with the library call and add the import/dependency), or if you prefer to keep the simple implementation, explicitly document the limitation in the UI/docs where patterns are entered and in the function comment for globMatch so callers know only '*' is supported.packages/main/src/cliAgentManager.ts (4)
245-251: Stderr buffer truncation could lose important error context.The stderr buffer is truncated to 4KB by slicing from the end. If an important error message appears early and is followed by verbose output, it will be lost. Consider keeping both head and tail of the buffer.
♻️ Preserve both head and tail of stderr
proc.stderr?.on('data', (chunk: Buffer) => { const text = chunk.toString('utf-8') session.stderrBuffer += text - if (session.stderrBuffer.length > 4000) { - session.stderrBuffer = session.stderrBuffer.slice(-4000) + if (session.stderrBuffer.length > 8000) { + // Keep first 2KB and last 2KB with marker + session.stderrBuffer = + session.stderrBuffer.slice(0, 2000) + + '\n...[truncated]...\n' + + session.stderrBuffer.slice(-2000) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/cliAgentManager.ts` around lines 245 - 251, The current proc.stderr?.on('data', ...) handler truncates session.stderrBuffer by slicing only the tail which can drop important earlier error context; update the logic in that handler (the proc.stderr?.on callback and session.stderrBuffer usage) to preserve both a head and a tail when exceeding the max (e.g., keep the first N bytes and the last M bytes summing to MAX_STDERR_BUFFER) instead of keeping only the last chunk, and consider introducing constants like MAX_STDERR_BUFFER, HEAD_SIZE and TAIL_SIZE and concatenating preservedHead + '…' + preservedTail to maintain context.
139-144: Regex for UUID validation is correct but could use a constant.The UUID regex pattern is repeated inline. Consider extracting to a constant for clarity and reuse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/cliAgentManager.ts` around lines 139 - 144, Extract the UUID regex used inline in cliAgentManager.ts into a single constant (e.g., UUID_REGEX or UUID_V4_REGEX) at the top of the module and replace the inline pattern in the conversationId parsing block inside the function where existingClaudeSessionId is derived (the block checking conversationId.startsWith('claude-native:') and testing raw). Ensure the constant is exported or file-scoped as appropriate, update any other occurrences in the file to use it, and keep the behavior identical (case-insensitive, same pattern).
345-355: Destroy method uses SIGKILL without graceful shutdown.Unlike
stop(),destroy()immediately sends SIGKILL to all processes. Consider using the same graceful SIGTERM → SIGKILL pattern for consistency, unless immediate termination is required for app shutdown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/cliAgentManager.ts` around lines 345 - 355, The destroy() implementation currently calls session.process.kill('SIGKILL') immediately; change it to use the same graceful shutdown pattern as stop(): send SIGTERM first via session.process.kill('SIGTERM'), set a session.killTimer that will send SIGKILL after a short timeout (e.g., 5s) if the process hasn't exited, and listen for the process exit to clear the timer and avoid double-kill; ensure you still call await this.persistSession(session) only after handling/awaiting process termination and clearTimeout(session.killTimer) on exit; use the same session/process/killTimer handling code paths as stop() to keep behavior consistent.
232-236: Spawned process inherits full environment viaprocess.env.The spread
env: { ...process.env }passes the entire environment to the CLI process, which is generally expected for CLI tools. However, this could expose sensitive environment variables. Consider whether specific variables should be filtered or if this is the desired behavior for CLI agent integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/cliAgentManager.ts` around lines 232 - 236, The spawned process currently passes the entire environment via env: { ...process.env } in the spawn call (spawn(binaryPath, args, { cwd: session.worktreePath ?? this.workspaceRoot, stdio: [...], env: { ...process.env } })), which may leak sensitive variables; change this to construct a filteredEnv before calling spawn—either build a whitelist (e.g., PATH, HOME, NODE_ENV, any required CI vars) or clone process.env and remove known sensitive keys (e.g., AWS_*, GCP_*, SECRET_*, API_KEY_*), then pass env: filteredEnv to spawn; update the code around the spawn invocation to use the new filteredEnv variable so the CLI agent only receives intended environment variables.packages/main/src/providers/openAiCompatibleProvider.ts (2)
186-190: Deleting from Map while iterating may cause issues in some JS engines.The loop
for (const [idx, tc] of activeToolCalls)deletes entries during iteration. While this is generally safe in modern JSMapimplementations (iteration uses a snapshot), it's cleaner to collect and then clear.♻️ Cleaner Map cleanup
// Close any open tool calls - for (const [idx, tc] of activeToolCalls) { + for (const tc of activeToolCalls.values()) { yield { type: 'tool_use_end', id: tc.id } - activeToolCalls.delete(idx) } + activeToolCalls.clear()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/providers/openAiCompatibleProvider.ts` around lines 186 - 190, The current loop in openAiCompatibleProvider that iterates activeToolCalls and deletes entries during iteration can be unsafe; instead first collect the keys (or entries) to close, then iterate that snapshot yielding { type: 'tool_use_end', id: tc.id } for each and delete from activeToolCalls (or call activeToolCalls.clear() after yielding) to avoid mutating the Map while iterating; locate the loop that uses "for (const [idx, tc] of activeToolCalls)" and replace it with a two-step approach using a collected array of keys/entries and then perform the yield and deletion/clear.
147-155: SSE parsing errors are yielded but continue processing.When JSON parsing fails for an SSE data chunk, the code yields an error event but continues processing subsequent chunks. This is a reasonable approach for resilience, but the error event will interrupt the UI flow. Consider whether to log and skip instead, or accumulate errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/providers/openAiCompatibleProvider.ts` around lines 147 - 155, The SSE JSON parse catch currently yields an error event (yield { type: 'error', ... }) which interrupts UI flow; instead, change the handler inside the for-await loop over parseSseStream(response.body) so parsing failures are logged/recorded and skipped rather than yielded immediately: inside the inner catch around JSON.parse(sse.data) (related to OpenAiStreamChunk) call a logger (or push to an errors array) with the parsing failure and sse.data, then continue the loop without yielding an error event; if you need to surface errors once at the end, yield or return a single consolidated error after the stream completes.packages/main/src/preload.ts (1)
3-3: Very long import line could benefit from multi-line formatting.The import statement spans a single line with many types. Consider breaking into multiple lines for readability, though this is stylistic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/preload.ts` at line 3, The single-line type import in preload.ts importing ThemeName, FsWatchEvent, GitStatusResult, WorktreeInfo, WorktreeCreateOpts, SearchOpts, SearchFileResult, ReplaceOpts, ResolvedSettings, AideProjectSettings, AideInitResult, GitignoreAuditResult, AideTask, CompoundTask, TaskExecution, TaskInputRequest, TaskDiagnostic, WorkspaceEntry, AideLocalState, AideLocalTerminals, WindowApi, BrowserSessionMode, BrowserHostUpdate, BrowserDidNavigatePayload, BrowserPageTitlePayload, BrowserLoadingPayload, BrowserCanNavigatePayload, BrowserFocusPayload, ZoomCommandPayload, KeybindingRule, ChatMode, ChatSession, ChatStreamChunk, ChatStreamEnd, ChatToolCallPayload, McpServerStatus, ToolDefinition, AgentBackend, CliAgentStreamDelta, CliAgentMessage, CliAgentSession, CliAgentStatusPayload, CliAgentResultPayload, ConversationMeta, ConversationCreateOpts, ConversationListChangedPayload from '@aide/shared' should be wrapped across multiple lines for readability; refactor the import to a multi-line type import (keeping the leading "import type") and place each type on its own line or grouped logically with trailing commas so the list is easier to scan and maintain (refer to the import line in preload.ts to locate and update).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2dbc2f52-0996-46a9-a089-46f573c33258
📒 Files selected for processing (78)
CLAUDE.mddocs/IDE_BUILD_PLAN.mddocs/LSPPLAN.mddocs/WORKSPACE_PLAN.mddocs/agentmvp.mddocs/browserPanes.mddocs/skills/commit-push-pr.mddocs/skills/commit.mddocs/skills/frontend-design.mddocs/skills/pr-review-analyze.mdpackages/main/src/agentManager.tspackages/main/src/agentTools.tspackages/main/src/agents/claudeNativeSessionWatcher.tspackages/main/src/browserPaneManager.tspackages/main/src/cliAgentManager.tspackages/main/src/conversationStore.tspackages/main/src/gitStatus.tspackages/main/src/index.tspackages/main/src/llmClient.tspackages/main/src/preload.tspackages/main/src/providers/anthropicProvider.tspackages/main/src/providers/openAiCompatibleProvider.tspackages/main/src/providers/sseParser.tspackages/main/src/settingsResolver.tspackages/main/src/structuredOutputParser.tspackages/main/src/toolRegistry.tspackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/DockviewContainer.tsxpackages/renderer/src/components/WorktreePanel/WorktreeItem.tsxpackages/renderer/src/components/WorktreePanel/WorktreePanel.tsxpackages/renderer/src/components/chat/AgentStatusDot.tsxpackages/renderer/src/components/chat/ChatInput.tsxpackages/renderer/src/components/chat/MessageBubble.tsxpackages/renderer/src/components/chat/MessageList.tsxpackages/renderer/src/components/chat/ModeSelector.tsxpackages/renderer/src/components/chat/PermissionTierBadge.tsxpackages/renderer/src/components/chat/ToolCallCard.tsxpackages/renderer/src/components/chat/WorkingSetPicker.tsxpackages/renderer/src/components/panes/AgentTab.tsxpackages/renderer/src/components/panes/ChatHistoryPane.tsxpackages/renderer/src/components/panes/ChatPane.tsxpackages/renderer/src/components/panes/CliAgentPane.tsxpackages/renderer/src/components/panes/MarkdownPreviewPane.tsxpackages/renderer/src/components/settings/SettingControl.tsxpackages/renderer/src/components/settings/SettingsContent.tsxpackages/renderer/src/components/settings/ToolPermissionsEditor.tsxpackages/renderer/src/hooks/useChat.tspackages/renderer/src/hooks/useCliAgent.tspackages/renderer/src/hooks/useConversationHistory.tspackages/renderer/src/lib/defaultKeybindings.tspackages/renderer/src/lib/markdownRenderer.tspackages/renderer/src/lib/settingsSchema.tspackages/renderer/src/lib/workspaceSwitcher.tspackages/renderer/src/main.tsxpackages/renderer/src/styles/agent-status-dot.csspackages/renderer/src/styles/agent-tab.csspackages/renderer/src/styles/chat-history-pane.csspackages/renderer/src/styles/chat-pane.csspackages/renderer/src/styles/cli-agent-pane.csspackages/renderer/src/styles/settings-pane.csspackages/shared/src/agentTypes.tspackages/shared/src/cliAgentTypes.tspackages/shared/src/conversationTypes.tspackages/shared/src/index.tspackages/shared/src/llmTypes.tstests/unit/agentManager.test.tstests/unit/agentTools.test.tstests/unit/agentTypes.test.tstests/unit/anthropicProvider.test.tstests/unit/claudeNativeSessionWatcher.test.tstests/unit/llmClient.test.tstests/unit/openAiCompatibleProvider.test.tstests/unit/sharedIndex.test.tstests/unit/sseParser.test.tstests/unit/toolRegistry.test.tstests/unit/useChat.test.tsxtests/unit/useCliAgent.test.tsxvitest.config.ts
Previously stop() rejected all pending approvals across every session, which could incorrectly cancel tool approvals in other active agent tabs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…failures from non-zero exits The previous logic checked `'code' in error` which conflates Node.js ErrnoException string codes (e.g. ENOENT) with process exit codes, causing spawn failures to silently resolve instead of rejecting. Now uses `proc.exitCode === null` to detect spawn failures and simplifies exit code extraction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s-workspace data leaks getPageContent previously fell back to an arbitrary pane when no paneId was provided. Since BrowserPaneManager is a global singleton, this could return content from a different workspace. Now threads workspaceId through ToolContext so the fallback filters by the caller's workspace. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l failures Guard against invalid CSS selectors, null document.body before page load, and executeJavaScript promise rejections from crashed/destroyed renderers by degrading gracefully to empty string instead of throwing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…SetPicker Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ssionsEditor The Custom (✱) button was calling both onStateChange and onToggleExpand, causing the pattern editor to immediately collapse after opening. Also preserved existing pattern config when re-entering patterns state instead of overwriting with empty arrays. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ary values A committed .aide/settings.json could silently override API keys, endpoints, executable paths, permission tiers, and auto-approve rules — allowing an untrusted repo to exfiltrate credentials, run arbitrary binaries, or disable tool confirmations. Sensitive agent keys now source only from user-scoped (app-level) settings, with a server-side IPC guard and UI scope restrictions as defense-in-depth layers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, promise rejections from getResolvedSettings() and conversationCreate() in the worktree onStartAgent handler were silently swallowed, causing "Start Agent" to appear broken with no feedback. Now mirrors the agent.open fallback pattern so a chat pane always opens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tool-result messages were showing empty bubbles because MessageBubble rendered message.content which is always empty. Now derives display text from toolResults[].output, falling back to content when no results exist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WorkingSetPicker always received workspaceRoot, causing Edit mode to browse the parent workspace instead of the actual worktree directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace compile-time `as string` cast with runtime `typeof` check to prevent potential TypeError if a non-string value reaches PasswordControl. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tch wins Pass a generation-based cancellation predicate into createDefaultLayout() and check it after each await to prevent stale async IPC results from mutating Dockview for the wrong workspace during rapid switches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Streaming TextDecoder can buffer trailing multi-byte UTF-8 bytes. Without a final decoder.decode() call, the last non-ASCII character could be silently truncated when it spans the final chunk boundary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
This was referenced Apr 7, 2026
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
1 participant
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
fs.watch) with multi-tab session isolation and multi-worktree support20 commits across 71 files (+11,096 lines)
Built-in agent loop (Step 1–4), chat UI (Step 5), permission system (Step 6), CLI wrappers (Step 7), conversation history (Step 8), plus bug fixes for multi-tab isolation, chat history restoration, and worktree agents.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests