Update Claude agent runtime surfaces#287
Merged
arul28 merged 3 commits intoMay 12, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
📝 WalkthroughWalkthroughAdds structured Claude availability and bumps the Claude SDK. Implements desktop probe migration to SDK query, IPC/preload endpoints, MCP/plugin/output-style discovery, subprocess reaper, runtime-event mapping, renderer rewind/subagents, session-pointer persistence with DB migration, and extensive CLI/TUI provider-scoped features and tests. ChangesTypes & SDK bump
Desktop probe & integration
IPC / Preload / Renderer
MCP, plugins, output styles, subprocess reaper
Runtime events & input pump
Renderer features: subagents, rewind, message list, handoff
Session pointers & DB migration
CLI / TUI features
🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested labels ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const event = envelope.event as Record<string, unknown>; | ||
| if (event.type === "status" && event.turnStatus === "started") setStreaming(true); | ||
| if (event.type === "done" || (event.type === "status" && event.turnStatus === "completed")) setStreaming(false); | ||
| if (event.type === "subagent_started" || event.type === "subagent.started") { |
There was a problem hiding this comment.
Dedup key in TUI omits event payload
High Severity
The TUI event dedup key at line 2023 uses sequence:timestamp:event.type without including the event payload. This PR adds new streaming event types (subagent events, system_notice, tokens with cache fields) that can emit multiple distinct events with the same type within the same millisecond. The desktop AgentChatPane and agentChatService both use timestamp#type#JSON.stringify(event) as the dedup key. Without the payload, the TUI silently drops distinct streaming fragments that share a millisecond timestamp and type, truncating replies after idle_ttl teardown + rehydration.
Triggered by learned rule: Chat envelope dedup keys must include event payload
Reviewed by Cursor Bugbot for commit a92be4f. Configure here.
| return { value: JSON.parse(fs.readFileSync(filePath, "utf8")), error: null }; | ||
| } catch (error) { | ||
| return { value: null, error: error instanceof Error ? error.message : String(error) }; | ||
| } |
There was a problem hiding this comment.
Duplicate readJsonFile functions across new modules
Low Severity
Both new modules keybindings/index.ts and statusline/index.ts introduce identical readJsonFile helper functions with the same signature and implementation. This duplication means bug fixes or behavior changes to JSON reading would need to be applied in both places.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a92be4f. Configure here.
| const event = envelope.event as Record<string, unknown>; | ||
| if (event.type === "status" && event.turnStatus === "started") setStreaming(true); | ||
| if (event.type === "done" || (event.type === "status" && event.turnStatus === "completed")) setStreaming(false); | ||
| if (event.type === "subagent_started" || event.type === "subagent.started") { |
There was a problem hiding this comment.
TUI dedup key omits event payload risking dropped fragments
High Severity
The TUI event dedup key uses sequence:timestamp:event.type without the event payload. The desktop equivalents in agentChatService.ts and AgentChatPane.tsx correctly use timestamp#type#JSON.stringify(event). This PR adds new streaming event types (subagent events, system_notice, cache-enriched tokens) that can produce multiple distinct events with the same type within a millisecond. Without payload in the key, distinct fragments are silently dropped, truncating replies during rehydration.
Triggered by learned rule: Chat envelope dedup keys must include event payload
Reviewed by Cursor Bugbot for commit a92be4f. Configure here.
| return { value: JSON.parse(fs.readFileSync(filePath, "utf8")), error: null }; | ||
| } catch (error) { | ||
| return { value: null, error: error instanceof Error ? error.message : String(error) }; | ||
| } |
There was a problem hiding this comment.
Duplicate readJsonFile utility in two new modules
Low Severity
Both new modules keybindings/index.ts and statusline/index.ts introduce identical readJsonFile helper functions with the same signature and implementation. This duplication means bug fixes or behavior changes to JSON file reading need to be applied in both places.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a92be4f. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
3766
to
3770
| useEffect(() => { | ||
| if (!selectedSessionId) { | ||
| setProofDrawerOpen(false); | ||
| setSubagentPaneOpen(false); | ||
| } |
There was a problem hiding this comment.
rewindConfirmDialog state is never cleared when selectedSessionId changes. If a user switches sessions while the dialog is open and then presses "Revert", rewindFilesFromMessage executes the actual rewind against the original session (the selectedSessionId captured in the closure), while the user is looking at a completely different session. The result: a destructive file revert is applied to the wrong project silently. The useEffect that runs on selectedSessionId changes only resets setSubagentPaneOpen/setProofDrawerOpen — it should also call closeRewindConfirmDialog() to cancel any in-flight rewind confirmation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
Line: 3766-3770
Comment:
**Rewind dialog persists across session changes**
`rewindConfirmDialog` state is never cleared when `selectedSessionId` changes. If a user switches sessions while the dialog is open and then presses "Revert", `rewindFilesFromMessage` executes the actual rewind against the **original session** (the `selectedSessionId` captured in the closure), while the user is looking at a completely different session. The result: a destructive file revert is applied to the wrong project silently. The `useEffect` that runs on `selectedSessionId` changes only resets `setSubagentPaneOpen`/`setProofDrawerOpen` — it should also call `closeRewindConfirmDialog()` to cancel any in-flight rewind confirmation.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+311
to
+316
| const CLAUDE_AGENT_SDK_VERSION = "0.2.139"; | ||
| const CLAUDE_AGENT_SDK_API = "v1_query"; | ||
| const CLAUDE_AGENT_SDK_TELEMETRY_TAGS = { | ||
| "claude_sdk.version": CLAUDE_AGENT_SDK_VERSION, | ||
| "claude_sdk.api": CLAUDE_AGENT_SDK_API, | ||
| } as const; |
There was a problem hiding this comment.
CLAUDE_AGENT_SDK_VERSION is pinned to "0.2.139". Telemetry emitted with this tag will misreport the version as soon as the dependency is bumped. The installed version should be read from the resolved package (e.g. via require("@anthropic-ai/claude-agent-sdk/package.json").version) so the tag always matches what's actually running.
Suggested change
| const CLAUDE_AGENT_SDK_VERSION = "0.2.139"; | |
| const CLAUDE_AGENT_SDK_API = "v1_query"; | |
| const CLAUDE_AGENT_SDK_TELEMETRY_TAGS = { | |
| "claude_sdk.version": CLAUDE_AGENT_SDK_VERSION, | |
| "claude_sdk.api": CLAUDE_AGENT_SDK_API, | |
| } as const; | |
| // eslint-disable-next-line @typescript-eslint/no-require-imports | |
| const CLAUDE_AGENT_SDK_VERSION: string = (require("@anthropic-ai/claude-agent-sdk/package.json") as { version: string }).version ?? "unknown"; | |
| const CLAUDE_AGENT_SDK_API = "v1_query"; | |
| const CLAUDE_AGENT_SDK_TELEMETRY_TAGS = { | |
| "claude_sdk.version": CLAUDE_AGENT_SDK_VERSION, | |
| "claude_sdk.api": CLAUDE_AGENT_SDK_API, | |
| } as const; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/services/chat/agentChatService.ts
Line: 311-316
Comment:
**Hardcoded SDK version will silently drift from the installed package**
`CLAUDE_AGENT_SDK_VERSION` is pinned to `"0.2.139"`. Telemetry emitted with this tag will misreport the version as soon as the dependency is bumped. The installed version should be read from the resolved package (e.g. via `require("@anthropic-ai/claude-agent-sdk/package.json").version`) so the tag always matches what's actually running.
```suggestion
// eslint-disable-next-line @typescript-eslint/no-require-imports
const CLAUDE_AGENT_SDK_VERSION: string = (require("@anthropic-ai/claude-agent-sdk/package.json") as { version: string }).version ?? "unknown";
const CLAUDE_AGENT_SDK_API = "v1_query";
const CLAUDE_AGENT_SDK_TELEMETRY_TAGS = {
"claude_sdk.version": CLAUDE_AGENT_SDK_VERSION,
"claude_sdk.api": CLAUDE_AGENT_SDK_API,
} as const;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+148
to
+156
| ...(process.env.PATH ? { PATH: process.env.PATH } : {}), | ||
| }, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export function buildClaudeMcpServers(args: BuildAdeClaudeMcpServersArgs): McpServerMap { | ||
| return buildAdeClaudeMcpServers(args); | ||
| } |
There was a problem hiding this comment.
discoverProjectClaudeMcpServers is exported and tested but never called from buildClaudeMcpServers
discoverProjectClaudeMcpServers reads project-level .mcp.json / .claude/.mcp.json files, but buildClaudeMcpServers only returns the ade built-in server. If the startup() SDK call does not internally scan these config files, any project-scoped MCP tools will be silently omitted at runtime. Does the startup() call in the Claude Agent SDK automatically discover project .mcp.json files from cwd, or is discoverProjectClaudeMcpServers meant to be merged into the McpServerMap passed to buildClaudeMcpServers?
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/services/chat/claudeMcpServers.ts
Line: 148-156
Comment:
**`discoverProjectClaudeMcpServers` is exported and tested but never called from `buildClaudeMcpServers`**
`discoverProjectClaudeMcpServers` reads project-level `.mcp.json` / `.claude/.mcp.json` files, but `buildClaudeMcpServers` only returns the `ade` built-in server. If the `startup()` SDK call does not internally scan these config files, any project-scoped MCP tools will be silently omitted at runtime. Does the `startup()` call in the Claude Agent SDK automatically discover project `.mcp.json` files from `cwd`, or is `discoverProjectClaudeMcpServers` meant to be merged into the `McpServerMap` passed to `buildClaudeMcpServers`?
How can I resolve this? If you propose a fix, please make it concise.
Copilot stopped work on behalf of
arul28 due to an error
May 12, 2026 09:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (1)
5415-5421:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep right-side panes mutually exclusive in every open path.
These two paths leave another pane open (
cursorCloudPaneOpenin the App Control header toggle,appControlOpenin the Cursor Cloud bring-to-local path). Since the render path mounts every pane whose flag is true, users can end up with both panels rendered into the same split area.Suggested fix
setAppControlOpen((current) => { const next = !current; if (next) { setProofDrawerOpen(false); setIosSimulatorOpen(false); + setCursorCloudPaneOpen(false); setSubagentPaneOpen(false); } return next; });onOpenCloudBringToLocal={() => { setCursorCloudLaunchModeOpen(false); setProofDrawerOpen(false); setIosSimulatorOpen(false); + setAppControlOpen(false); setSubagentPaneOpen(false); setCursorCloudPaneOpen(true); }}Also applies to: 6133-6139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 5415 - 5421, The App Control toggle and the Cursor Cloud "bring-to-local" path currently leave the other pane's flag unchanged, allowing both to be true; update the handlers (the setAppControlOpen(...) block and the Cursor Cloud handler at the other path) to explicitly close the opposing pane by calling setCursorCloudPaneOpen(false) when opening App Control and calling setAppControlOpen(false) when opening Cursor Cloud, and also ensure both paths mirror the same mutual-exclusive closures for proof/ios/subagent flags (setProofDrawerOpen(false), setIosSimulatorOpen(false), setSubagentPaneOpen(false)) so only one right-side pane flag is true at a time.
🧹 Nitpick comments (6)
apps/desktop/src/main/services/chat/claudeInputPump.test.ts (1)
17-51: ⚡ Quick winAdd coverage for iterator
return()close path.The branch at Line 45 in
claudeInputPump.tsis currently untested; a focused test would protect close-on-return behavior from regressions.Proposed test addition
describe("ClaudeInputPump", () => { @@ it("rejects pushes after close", () => { @@ expect(() => pump.push(userMessage("too late"))).toThrow(/closed/i); }); + + it("closes when iterator.return() is called", async () => { + const pump = new ClaudeInputPump(); + const iterator = pump[Symbol.asyncIterator](); + + await expect(iterator.return?.()).resolves.toEqual({ done: true, value: undefined }); + expect(() => pump.push(userMessage("after return"))).toThrow(/closed/i); + await expect(iterator.next()).resolves.toEqual({ done: true, value: undefined }); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/chat/claudeInputPump.test.ts` around lines 17 - 51, Add a test to cover the iterator.return() close path for ClaudeInputPump: create a ClaudeInputPump, get its async iterator via pump[Symbol.asyncIterator](), call iterator.next() to create a pending read, then call iterator.return() and assert the pending read resolves to { done: true, value: undefined } and subsequent iterator.next() also resolves to done true; additionally verify that pump.push(...) after return/close throws (or behaves like close). Use the existing test patterns (pump.push, pump.close, expect(...).resolves/toThrow) and reference ClaudeInputPump, pump[Symbol.asyncIterator](), iterator.next(), and iterator.return() to locate the code to change.apps/ade-cli/src/tuiClient/statusline/index.ts (2)
202-202: ⚡ Quick winConsider adding error handling for stdin write.
Line 202 writes to
stdinwithout error handling. If the subprocess closes stdin early or there's a write error, this could cause an unhandled error event.🛡️ Proposed fix to add stdin error handling
- child.stdin.end(`${JSON.stringify(payload)}\n`); + child.stdin.on("error", () => { + // Ignore stdin errors; the process error/close handlers will report the issue. + }); + child.stdin.end(`${JSON.stringify(payload)}\n`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/tuiClient/statusline/index.ts` at line 202, The call to child.stdin.end(`${JSON.stringify(payload)}\n`) in index.ts can throw if the subprocess closes stdin or a write fails; wrap this write in error handling by checking child.stdin.writable before writing and attach an 'error' listener to child.stdin (and/or handle the callback from end if provided) to log or handle the error, e.g., reference the child.stdin.end call and the payload variable and ensure any error is caught and passed to your existing logger or cleanup routine so an unhandled exception cannot occur.
165-170: ⚡ Quick winSecurity consideration:
shell: truewith user-controlled command.The function spawns a subprocess with
shell: trueusing a command from user configuration. While the command comes from the user's own config files (making it acceptable in this context), consider documenting this security assumption in a comment.The implementation is appropriate for a CLI tool where users control their environment, but the pattern should not be replicated in contexts where commands come from untrusted sources.
📝 Suggested documentation addition
export async function runClaudeStatusLineCommand( config: ClaudeStatusLineConfig, payload: ClaudeStatusLinePayload, options: { timeoutMs?: number } = {}, ): Promise<{ ok: true; text: string } | { ok: false; error: string }> { const timeoutMs = options.timeoutMs ?? 1_000; return await new Promise((resolve) => { + // Security note: shell: true is acceptable here because config.command + // comes from the user's own settings files. Do not use this pattern + // with commands from untrusted sources. const child = spawn(config.command, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/tuiClient/statusline/index.ts` around lines 165 - 170, Add a short inline comment at the spawn call (where `child` is created using `spawn(config.command, { shell: true, ... })`) stating that `shell: true` is used intentionally because the command originates from the user's local configuration (i.e., trusted user-controlled input) and warning that this pattern must not be used with untrusted inputs; reference `config.command` and `child` so reviewers can find the location easily and include a brief security note about not reusing `shell: true` in server-side or multi-user contexts.apps/desktop/src/renderer/components/chat/rewindFilesPreview.test.ts (1)
52-79: ⚡ Quick winStrengthen the aggregation assertion with non-identical turn counts.
Right now both summaries use the same
8/3, so this test won’t catch overwrite behavior. Make the second turn use different counts and assert cumulative totals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/rewindFilesPreview.test.ts` around lines 52 - 79, The test uses identical per-turn counts so it won't detect overwrite vs aggregation; update the second summary call (the one created by summary("turn-2", "src/auth.ts", "mid", "head")) to use different additions/deletions than the first summary created by summary("turn-1", "src/auth.ts", "base", "mid") and then update the expected object for buildRewindPreviewFiles to assert the cumulative additions and deletions (sum of both turns), while keeping beforeSha from the first summary ("base") and afterSha from the last summary ("head") and diffAvailable true for that file.apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts (1)
87-90: ⚡ Quick winAdd a regression assertion for shifted printable keys.
Please add a
keypressToChordcase for a shifted letter (for exampleK) so shifted-chord behavior is locked in.Suggested test addition
it("converts Ink keypresses to chords", () => { expect(keypressToChord("", { pageDown: true })).toBe("pagedown"); expect(keypressToChord("k", { ctrl: true })).toBe("ctrl+k"); + expect(keypressToChord("K", { shift: true })).toBe("shift+k"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts` around lines 87 - 90, Add a regression assertion for shifted printable keys by extending the existing "converts Ink keypresses to chords" test: call keypressToChord with an uppercase character (e.g., keypressToChord("K", {})) and assert the chord includes the shift modifier (expect(...).toBe("shift+k")), so shifted-chord behavior for the keypressToChord function is locked in.apps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsx (1)
311-315: 💤 Low valueMinor style inconsistency in filter predicates.
The
tabCounts.backgroundfilter usess.background(truthy check) whilevisibleSnapshotsat line 345 usessnapshot.background === true(strict equality). Consider using consistent predicate style to avoid potential divergence ifbackgroundis ever set to a truthy non-boolean value.🔧 Suggested consistency fix
tabCounts: { subagents: snapshots.filter((s) => !s.background).length, teammates: 0, - background: snapshots.filter((s) => s.background).length, + background: snapshots.filter((s) => s.background === true).length, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsx` around lines 311 - 315, In ChatSubagentsPanel, the predicates for computing tabCounts (using snapshots.filter((s) => !s.background) and snapshots.filter((s) => s.background)) are inconsistent with the strict check used in visibleSnapshots (snapshot.background === true); update the tabCounts calculations to use the same strict boolean checks (e.g., snapshots.filter((s) => s.background === true) and snapshots.filter((s) => s.background !== true) or snapshots.filter((s) => s.background === false)) so tabCounts, visibleSnapshots and the snapshots variable use a consistent predicate style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/services/sync/syncRemoteCommandService.ts`:
- Line 307: The spread currently omits claudeOutputStyle when it's explicitly
null because it uses a truthy check; change the conditional to include the
property whenever it exists (including null) by checking for undefined rather
than falsiness. Replace the spread ...(session.claudeOutputStyle ? {
claudeOutputStyle: session.claudeOutputStyle } : {}) with a check like
...(session.claudeOutputStyle !== undefined ? { claudeOutputStyle:
session.claudeOutputStyle } : {}) (or use
Object.prototype.hasOwnProperty.call(session, 'claudeOutputStyle')) in the code
that builds the fallback summary payload in syncRemoteCommandService / the
function that constructs the create args so the nullable contract is preserved.
In `@apps/ade-cli/src/tuiClient/app.tsx`:
- Around line 3253-3271: openHistorySearch currently puts only truncated display
strings into the pane state so accept/execute handlers (historySearch:accept /
historySearch:execute) cannot restore the full prompt; change setRightPane usage
in openHistorySearch to store rows as objects with both the full prompt (e.g.,
full) and its truncated display (e.g., label or display) — keep
promptHistoryRef, promptRef and chatDraftRef as the sources of truth — and
update the pane handlers historySearch:accept and historySearch:execute to read
the selected row.full and set it back into the composer (write into
promptRef.current or chatDraftRef.current as appropriate), then close the right
pane (setRightOpen(false)) and focus the composer (setPaneFocus or focusChat) so
the chosen prompt is restored and ready to edit/execute.
- Around line 2030-2034: The code unconditionally calls setPaneFocus("details")
on subagent start which steals keyboard focus; change this so the pane opens but
focus is preserved unless the user already had the details pane focused.
Specifically, in the handler that calls setRightOpen and setRightPane (for event
types "subagent_started"/"subagent.started"), remove the unconditional
setPaneFocus call and instead conditionally call setPaneFocus("details") only
when the current focus state indicates the details pane is already active (e.g.,
check the paneFocus state variable or the current right-pane focus before
invoking setPaneFocus), leaving focus untouched for other cases.
- Around line 3470-3476: The "tabs:previous" branch incorrectly calls
cyclePaneFocus() like the "next" branch so it moves forward; change the handler
so previous triggers a backward move instead—either add a direction parameter to
cyclePaneFocus (e.g., cyclePaneFocus("next") vs cyclePaneFocus("previous")) and
call cyclePaneFocus("previous") in the "tabs:previous" / "footer:previous"
branch, or implement a separate function (e.g., cyclePaneFocusPrevious or
movePaneFocusBackward) and call that from the previous-action branch; update any
related callers to match the new API and ensure the two action strings
("tabs:next"/"footer:next" vs "tabs:previous"/"footer:previous") invoke opposite
directions.
In `@apps/ade-cli/src/tuiClient/commands.ts`:
- Around line 204-208: The loop that registers user commands currently skips any
user command whose key is in ADE_OWNED_CLAUDE_PARITY_COMMANDS unconditionally;
change that logic so commands in ADE_OWNED_CLAUDE_PARITY_COMMANDS are only
suppressed when the ADE builtin is actually present for the current provider
(i.e., respect the earlier provider-based filtering that may have removed the
ADE builtin), by augmenting the condition that checks
ADE_OWNED_CLAUDE_PARITY_COMMANDS to also verify the ADE builtin is
enabled/available for this provider before continuing; update the loop that
calls slashCommandKey(command.name) and sets byName.set(key, command) so valid
runtime/user `/agents`-style commands are not hidden for non-Claude providers.
In `@apps/ade-cli/src/tuiClient/keybindings/index.ts`:
- Around line 329-350: The code currently pushes a warning when
RESERVED_KEYS.has(normalizedKey) but continues to accept and add the binding;
change the parsing logic so reserved keys are not added or dispatchable by
returning early for that entry: after pushing the warning for
RESERVED_KEYS.has(normalizedKey) skip further processing for that key (i.e., do
not push into bindings or mark implemented). Update the block that references
RESERVED_KEYS, normalizedKey, warnings, bindings, and implemented so reserved
keys immediately continue/return from that iteration before any checks that add
to bindings.
- Around line 294-299: The current combo-building logic drops "shift" for
printable letters because the condition uses key.shift && named !==
input.toLowerCase(), so chords like "shift+k" never emit; change the shift
inclusion to depend only on key.shift (i.e., use key.shift ? "shift" : null) so
the mods array always contains "shift" when the Shift key is pressed; update the
mods construction around the variables mods, key, named, and input and ensure
normalizeKeyChord([...mods, named].join("+")) still receives the correct chord
string.
In `@apps/desktop/src/main/services/chat/agentChatService.test.ts`:
- Around line 704-718: The prompt pump currently runs as a fire-and-forget IIFE
that calls send(...) and always calls markFirstInput() in finally, which can
swallow rejections from session.send() and make next() succeed incorrectly;
change the flow so pump failures propagate to next() by awaiting the pump
promise instead of ignoring it (or returning/rethrowing its rejection), and only
call markFirstInput() after the pump completes successfully (or still call it
but rethrow the original error), updating the logic around prompt, send, and
markFirstInput to ensure any rejection from send/session.send bubbles up to
next().
In `@apps/desktop/src/main/services/chat/claudeMcpServers.ts`:
- Around line 20-22: The current fileExists(filePath: string | undefined | null)
only checks existence, allowing directories; change it to validate that the path
exists and is a regular file (e.g., using fs.statSync or fs.lstatSync and
.isFile()) and update the places that rely on it (the fileExists function itself
and the resolution logic that handles ADE_CLI_PATH/ADE_CLI_BIN_DIR around the
code referenced at lines ~91-98) so environment overrides that point to
directories are rejected and fallback logic executes instead.
In `@apps/desktop/src/main/services/sessions/sessionService.ts`:
- Around line 363-382: The two separate db.run statements that clear existing
chat_session_id mappings and then upsert the claude_sessions row must be
executed atomically to avoid losing the mapping if the upsert fails; wrap both
operations in a single transaction (BEGIN/COMMIT with rollback on error) so the
"update claude_sessions set chat_session_id = null ..." and the subsequent
"insert ... on conflict" execute as one unit; locate the block using
chatSessionId, sessionId and the db.run calls and ensure you begin a transaction
before the first db.run, commit after the upsert, and rollback on any error.
In `@apps/desktop/src/main/services/state/kvDb.ts`:
- Around line 1038-1045: The foreign key on chat_session_id currently references
terminal_sessions(id) (foreign key(chat_session_id) references
terminal_sessions(id) on delete set null) which is incorrect; update the
migration to either reference the correct parent table (e.g., change
terminal_sessions(id) to chat_sessions(id) if a chat_sessions table is the
canonical parent) or remove the FK constraint entirely if no canonical
chat-session table exists in this DB, ensuring the change is made where the
table DDL defines chat_session_id in kvDb.ts.
In `@apps/desktop/src/preload/preload.ts`:
- Around line 5109-5114: rewindFiles is a chat mutation but doesn't touch the
agentChatSummaryCache, causing stale summaries; update the rewindFiles
implementation (the async property that uses callProjectRuntimeActionOr and
IPC.agentChatRewindFiles) to invalidate/clear agentChatSummaryCache before the
mutation and refresh/recompute it after a successful call (or at minimum clear
it on success) so callers won't observe stale summaries.
In `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx`:
- Around line 2482-2483: Validate and coerce the incoming usage payload before
computing/displaying percentages: ensure usage is an object, set const
categories = Array.isArray(usage?.categories) ? usage.categories : []; coerce
the percentage with const rawPct = Number(usage?.percentage); const percent =
Number.isFinite(rawPct) ? Math.max(0, Math.min(100, rawPct)) : 0; and use this
safe percent everywhere you currently reference usage.percentage (including the
other percent usage around the block that corresponds to lines ~2499-2500) so
you never produce NaN or crash on malformed payloads.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 4923-4927: The dialog is opened using live props but not bound to
the session that produced the preview—capture the current session id at the
moment you call openRewindConfirmDialog (e.g., const sessionIdAtOpen =
selectedSessionId or session.id) and pass that sessionId into the dialog call so
the dialog's diff loading and confirm path use the same session; update the
openRewindConfirmDialog invocation(s) (the one around openRewindConfirmDialog
with request/preview/files and the similar call at the other location) and the
dialog API to accept and use this sessionId (or a stable session token) instead
of reading the live selectedSessionId.
In `@apps/desktop/src/renderer/components/chat/chatExecutionSummary.ts`:
- Around line 54-93: The subagent snapshot updates for the "subagent_progress"
and "subagent_result" branches drop or ignore authoritative metadata: preserve
event.finalSummary and event.background when present so snapshots don't show
stale summary or wrong background; in the "subagent_progress" handler (where
snapshots.set is called) include finalSummary: event.finalSummary?.trim() ??
existing?.finalSummary ?? null and set background: event.background ??
existing?.background ?? false, and in the "subagent_result" handler ensure
finalSummary prefers event.finalSummary?.trim() then event.summary?.trim() then
existing?.finalSummary, and set background: event.background ??
existing?.background ?? false so both handlers consistently carry
forward/override these fields.
In `@apps/desktop/src/renderer/components/chat/RewindFilesConfirmDialog.tsx`:
- Around line 344-349: The RewindFilesConfirmDialog currently sets autoFocus on
the destructive "Revert" button (the element with onClick={onConfirm}), which
can cause an accidental Enter to confirm; remove the autoFocus from that button
and instead set focus on a neutral control (e.g., the "Cancel" button) or rely
on no autofocus so the safe default remains; update the JSX in
RewindFilesConfirmDialog.tsx to remove the autoFocus prop from the button tied
to onConfirm and, if desired, add autoFocus to the cancel button or manage
initial focus programmatically in the component.
In `@apps/desktop/src/renderer/components/chat/rewindFilesPreview.ts`:
- Around line 69-83: The aggregation loop over summaries (iterating summaries ->
summary.files) currently overwrites prior counts in the byPath map; update the
object set on byPath (key derived via pathsMatch(file.path)) to accumulate
additions and deletions by summing existing?.additions + file.additions and
existing?.deletions + file.deletions, preserve existing?.beforeSha
(existing?.beforeSha ?? summary.beforeSha) and keep afterSha as either the
existing?.afterSha or summary.afterSha depending on desired semantics (e.g.,
existing?.afterSha ?? summary.afterSha), and ensure diffAvailable remains true
while choosing a consistent status (e.g., existing?.status ?? file.status) so
earlier turns for the same file are not lost.
In `@apps/ios/ADE/Resources/DatabaseBootstrap.sql`:
- Around line 189-196: The foreign key on chat_session_id is referencing
terminal_sessions(id) incorrectly; update the constraint in the table definition
that contains chat_session_id so it references the correct parent table
chat_sessions(id) (keeping the on delete set null behavior if intended) and
remove or correct the existing foreign key(line with foreign
key(chat_session_id) references terminal_sessions(id) on delete set null) to
prevent FK validation failures during Claude pointer upserts.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 5415-5421: The App Control toggle and the Cursor Cloud
"bring-to-local" path currently leave the other pane's flag unchanged, allowing
both to be true; update the handlers (the setAppControlOpen(...) block and the
Cursor Cloud handler at the other path) to explicitly close the opposing pane by
calling setCursorCloudPaneOpen(false) when opening App Control and calling
setAppControlOpen(false) when opening Cursor Cloud, and also ensure both paths
mirror the same mutual-exclusive closures for proof/ios/subagent flags
(setProofDrawerOpen(false), setIosSimulatorOpen(false),
setSubagentPaneOpen(false)) so only one right-side pane flag is true at a time.
---
Nitpick comments:
In `@apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts`:
- Around line 87-90: Add a regression assertion for shifted printable keys by
extending the existing "converts Ink keypresses to chords" test: call
keypressToChord with an uppercase character (e.g., keypressToChord("K", {})) and
assert the chord includes the shift modifier (expect(...).toBe("shift+k")), so
shifted-chord behavior for the keypressToChord function is locked in.
In `@apps/ade-cli/src/tuiClient/statusline/index.ts`:
- Line 202: The call to child.stdin.end(`${JSON.stringify(payload)}\n`) in
index.ts can throw if the subprocess closes stdin or a write fails; wrap this
write in error handling by checking child.stdin.writable before writing and
attach an 'error' listener to child.stdin (and/or handle the callback from end
if provided) to log or handle the error, e.g., reference the child.stdin.end
call and the payload variable and ensure any error is caught and passed to your
existing logger or cleanup routine so an unhandled exception cannot occur.
- Around line 165-170: Add a short inline comment at the spawn call (where
`child` is created using `spawn(config.command, { shell: true, ... })`) stating
that `shell: true` is used intentionally because the command originates from the
user's local configuration (i.e., trusted user-controlled input) and warning
that this pattern must not be used with untrusted inputs; reference
`config.command` and `child` so reviewers can find the location easily and
include a brief security note about not reusing `shell: true` in server-side or
multi-user contexts.
In `@apps/desktop/src/main/services/chat/claudeInputPump.test.ts`:
- Around line 17-51: Add a test to cover the iterator.return() close path for
ClaudeInputPump: create a ClaudeInputPump, get its async iterator via
pump[Symbol.asyncIterator](), call iterator.next() to create a pending read,
then call iterator.return() and assert the pending read resolves to { done:
true, value: undefined } and subsequent iterator.next() also resolves to done
true; additionally verify that pump.push(...) after return/close throws (or
behaves like close). Use the existing test patterns (pump.push, pump.close,
expect(...).resolves/toThrow) and reference ClaudeInputPump,
pump[Symbol.asyncIterator](), iterator.next(), and iterator.return() to locate
the code to change.
In `@apps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsx`:
- Around line 311-315: In ChatSubagentsPanel, the predicates for computing
tabCounts (using snapshots.filter((s) => !s.background) and snapshots.filter((s)
=> s.background)) are inconsistent with the strict check used in
visibleSnapshots (snapshot.background === true); update the tabCounts
calculations to use the same strict boolean checks (e.g., snapshots.filter((s)
=> s.background === true) and snapshots.filter((s) => s.background !== true) or
snapshots.filter((s) => s.background === false)) so tabCounts, visibleSnapshots
and the snapshots variable use a consistent predicate style.
In `@apps/desktop/src/renderer/components/chat/rewindFilesPreview.test.ts`:
- Around line 52-79: The test uses identical per-turn counts so it won't detect
overwrite vs aggregation; update the second summary call (the one created by
summary("turn-2", "src/auth.ts", "mid", "head")) to use different
additions/deletions than the first summary created by summary("turn-1",
"src/auth.ts", "base", "mid") and then update the expected object for
buildRewindPreviewFiles to assert the cumulative additions and deletions (sum of
both turns), while keeping beforeSha from the first summary ("base") and
afterSha from the last summary ("head") and diffAvailable true for that file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 630cff58-0104-47ad-8c7c-96d0f3a64e4e
⛔ Files ignored due to path filters (17)
README.mdis excluded by!*.mdapps/ade-cli/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonapps/desktop/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/examples/statuslines/README.mdis excluded by!docs/**docs/examples/statuslines/compact.jsis excluded by!docs/**docs/examples/statuslines/git-context.shis excluded by!docs/**docs/features/ade-code/README.mdis excluded by!docs/**docs/features/agents/tool-registration.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/agent-routing.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/chat/tool-system.mdis excluded by!docs/**docs/features/chat/transcript-and-turns.mdis excluded by!docs/**docs/features/sync-and-multi-device/remote-commands.mdis excluded by!docs/**docs/plans/ade-31-claude-agent-sdk-rewrite.mdis excluded by!docs/**docs/playbooks/ship-lane.mdis excluded by!docs/**
📒 Files selected for processing (81)
apps/ade-cli/package.jsonapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/tuiClient/__tests__/RightPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/ade-cli/src/tuiClient/__tests__/commands.test.tsapps/ade-cli/src/tuiClient/__tests__/keybindings.test.tsapps/ade-cli/src/tuiClient/__tests__/statusline.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/commands.tsapps/ade-cli/src/tuiClient/components/ModelStatus.tsxapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/ade-cli/src/tuiClient/components/SlashPalette.tsxapps/ade-cli/src/tuiClient/keybindings/index.tsapps/ade-cli/src/tuiClient/statusline/index.tsapps/ade-cli/src/tuiClient/types.tsapps/desktop/package.jsonapps/desktop/src/main/packagedRuntimeSmoke.test.tsapps/desktop/src/main/packagedRuntimeSmoke.tsapps/desktop/src/main/packagedRuntimeSmokeShared.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/ai/aiIntegrationService.tsapps/desktop/src/main/services/ai/aiSettingsStatus.tsapps/desktop/src/main/services/ai/claudeRuntimeProbe.test.tsapps/desktop/src/main/services/ai/claudeRuntimeProbe.tsapps/desktop/src/main/services/ai/tools/systemPrompt.test.tsapps/desktop/src/main/services/ai/tools/systemPrompt.tsapps/desktop/src/main/services/chat/agentChatService.suggestLaneName.test.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/buildClaudeV2Message.tsapps/desktop/src/main/services/chat/claudeInputPump.test.tsapps/desktop/src/main/services/chat/claudeInputPump.tsapps/desktop/src/main/services/chat/claudeMcpServers.test.tsapps/desktop/src/main/services/chat/claudeMcpServers.tsapps/desktop/src/main/services/chat/claudeOutputStyles.test.tsapps/desktop/src/main/services/chat/claudeOutputStyles.tsapps/desktop/src/main/services/chat/claudeSubprocessReaper.test.tsapps/desktop/src/main/services/chat/claudeSubprocessReaper.tsapps/desktop/src/main/services/chat/runtimeEvents.test.tsapps/desktop/src/main/services/chat/runtimeEvents.tsapps/desktop/src/main/services/computerUse/syntheticToolResult.tsapps/desktop/src/main/services/config/projectConfigService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.tsapps/desktop/src/main/services/sessions/sessionService.test.tsapps/desktop/src/main/services/sessions/sessionService.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.test.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/automations/adeActionSchemas.tsapps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatMessageList.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsxapps/desktop/src/renderer/components/chat/RewindFilesConfirmDialog.tsxapps/desktop/src/renderer/components/chat/chatExecutionSummary.test.tsapps/desktop/src/renderer/components/chat/chatExecutionSummary.tsapps/desktop/src/renderer/components/chat/rewindFilesPreview.test.tsapps/desktop/src/renderer/components/chat/rewindFilesPreview.tsapps/desktop/src/renderer/components/settings/ProvidersSection.test.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.tsxapps/desktop/src/renderer/components/shared/permissionOptions.test.tsapps/desktop/src/renderer/components/shared/permissionOptions.tsapps/desktop/src/renderer/components/usage/UsageQuotaPanel.test.tsxapps/desktop/src/renderer/lib/aiDiscoveryCache.test.tsapps/desktop/src/renderer/lib/modelOptions.test.tsapps/desktop/src/renderer/state/appStore.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/chat.tsapps/desktop/src/shared/types/config.tsapps/desktop/src/shared/types/sessions.tsapps/ios/ADE/Resources/DatabaseBootstrap.sql
💤 Files with no reviewable changes (2)
- apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts
- apps/desktop/src/main/services/ai/claudeRuntimeProbe.ts
| ...(session.permissionMode ? { permissionMode: session.permissionMode } : {}), | ||
| ...(session.interactionMode !== undefined ? { interactionMode: session.interactionMode } : {}), | ||
| ...(session.claudePermissionMode ? { claudePermissionMode: session.claudePermissionMode } : {}), | ||
| ...(session.claudeOutputStyle ? { claudeOutputStyle: session.claudeOutputStyle } : {}), |
There was a problem hiding this comment.
Preserve explicit null for claudeOutputStyle in fallback summaries.
At Line 307, the truthy check omits claudeOutputStyle when it is explicitly null, which can drift from the nullable contract shape used by create args and shared types.
Suggested fix
- ...(session.claudeOutputStyle ? { claudeOutputStyle: session.claudeOutputStyle } : {}),
+ ...(session.claudeOutputStyle !== undefined ? { claudeOutputStyle: session.claudeOutputStyle } : {}),As per coding guidelines, "Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes in TypeScript".
📝 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
| ...(session.claudeOutputStyle ? { claudeOutputStyle: session.claudeOutputStyle } : {}), | |
| ...(session.claudeOutputStyle !== undefined ? { claudeOutputStyle: session.claudeOutputStyle } : {}), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/services/sync/syncRemoteCommandService.ts` at line 307, The
spread currently omits claudeOutputStyle when it's explicitly null because it
uses a truthy check; change the conditional to include the property whenever it
exists (including null) by checking for undefined rather than falsiness. Replace
the spread ...(session.claudeOutputStyle ? { claudeOutputStyle:
session.claudeOutputStyle } : {}) with a check like
...(session.claudeOutputStyle !== undefined ? { claudeOutputStyle:
session.claudeOutputStyle } : {}) (or use
Object.prototype.hasOwnProperty.call(session, 'claudeOutputStyle')) in the code
that builds the fallback summary payload in syncRemoteCommandService / the
function that constructs the create args so the nullable contract is preserved.
Comment on lines
+2030
to
+2034
| if (event.type === "subagent_started" || event.type === "subagent.started") { | ||
| setRightOpen(true); | ||
| setRightPane((prev) => prev.kind === "subagents" ? prev : { kind: "subagents", tab: "subagents", snapshots: [] }); | ||
| setPaneFocus("details"); | ||
| } |
There was a problem hiding this comment.
Don't steal focus on background subagent events.
Opening the subagent pane is fine, but forcing setPaneFocus("details") here will yank keyboard focus away from the composer every time Claude starts a subagent. That means background work can interrupt typing or shortcut handling mid-prompt. Please update the pane without changing focus, or only autofocus when the user is already on the details pane.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/app.tsx` around lines 2030 - 2034, The code
unconditionally calls setPaneFocus("details") on subagent start which steals
keyboard focus; change this so the pane opens but focus is preserved unless the
user already had the details pane focused. Specifically, in the handler that
calls setRightOpen and setRightPane (for event types
"subagent_started"/"subagent.started"), remove the unconditional setPaneFocus
call and instead conditionally call setPaneFocus("details") only when the
current focus state indicates the details pane is already active (e.g., check
the paneFocus state variable or the current right-pane focus before invoking
setPaneFocus), leaving focus untouched for other cases.
Comment on lines
+3253
to
+3271
| const openHistorySearch = useCallback(() => { | ||
| const query = (promptRef.current || chatDraftRef.current).trim().toLowerCase(); | ||
| const rows = [...promptHistoryRef.current] | ||
| .reverse() | ||
| .filter((entry) => !query || entry.toLowerCase().includes(query)) | ||
| .slice(0, 20) | ||
| .map((entry) => { | ||
| const compact = entry.replace(/\s+/g, " "); | ||
| return compact.length > 34 ? `${compact.slice(0, 33)}…` : compact; | ||
| }); | ||
| setRightPane({ | ||
| kind: "list", | ||
| title: "History search", | ||
| rows, | ||
| emptyText: query ? `No prompt history matched "${query}".` : "No prompt history in this chat yet.", | ||
| }); | ||
| setRightOpen(true); | ||
| setPaneFocus("details"); | ||
| }, [setPaneFocus]); |
There was a problem hiding this comment.
History search never reapplies the selected prompt.
This pane only stores truncated display rows, and the historySearch:accept / historySearch:execute handlers just call focusChat(). As written, the new history-search flow can browse entries but can never put the chosen prompt back into the composer. Please keep the full prompt payload in pane state and wire accept/execute to restore the selected entry.
Also applies to: 3420-3423
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/app.tsx` around lines 3253 - 3271,
openHistorySearch currently puts only truncated display strings into the pane
state so accept/execute handlers (historySearch:accept / historySearch:execute)
cannot restore the full prompt; change setRightPane usage in openHistorySearch
to store rows as objects with both the full prompt (e.g., full) and its
truncated display (e.g., label or display) — keep promptHistoryRef, promptRef
and chatDraftRef as the sources of truth — and update the pane handlers
historySearch:accept and historySearch:execute to read the selected row.full and
set it back into the composer (write into promptRef.current or
chatDraftRef.current as appropriate), then close the right pane
(setRightOpen(false)) and focus the composer (setPaneFocus or focusChat) so the
chosen prompt is restored and ready to edit/execute.
Comment on lines
+3470
to
+3476
| if (action === "tabs:next" || action === "footer:next") { | ||
| cyclePaneFocus(); | ||
| return true; | ||
| } | ||
| if (action === "tabs:previous" || action === "footer:previous") { | ||
| cyclePaneFocus(); | ||
| return true; |
There was a problem hiding this comment.
tabs:previous currently moves forward.
Both branches call cyclePaneFocus(), so the "previous" binding can't move backward. This makes the reverse shortcut behave identically to "next".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/app.tsx` around lines 3470 - 3476, The
"tabs:previous" branch incorrectly calls cyclePaneFocus() like the "next" branch
so it moves forward; change the handler so previous triggers a backward move
instead—either add a direction parameter to cyclePaneFocus (e.g.,
cyclePaneFocus("next") vs cyclePaneFocus("previous")) and call
cyclePaneFocus("previous") in the "tabs:previous" / "footer:previous" branch, or
implement a separate function (e.g., cyclePaneFocusPrevious or
movePaneFocusBackward) and call that from the previous-action branch; update any
related callers to match the new API and ensure the two action strings
("tabs:next"/"footer:next" vs "tabs:previous"/"footer:previous") invoke opposite
directions.
Comment on lines
204
to
208
| for (const command of users) { | ||
| const key = slashCommandKey(command.name); | ||
| if (ADE_OWNED_SINGLE_WORD_COMMANDS.has(key)) continue; | ||
| if (ADE_OWNED_CLAUDE_PARITY_COMMANDS.has(key)) continue; | ||
| byName.set(key, command); |
There was a problem hiding this comment.
Don’t unconditionally suppress user /agents-style commands outside Claude.
Line 207 skips user commands in ADE_OWNED_CLAUDE_PARITY_COMMANDS even when the ADE builtin was filtered out by provider (Line 187). That can hide valid runtime/user commands for non-Claude providers.
Suggested fix
for (const command of users) {
const key = slashCommandKey(command.name);
if (ADE_OWNED_SINGLE_WORD_COMMANDS.has(key)) continue;
- if (ADE_OWNED_CLAUDE_PARITY_COMMANDS.has(key)) continue;
+ if (ADE_OWNED_CLAUDE_PARITY_COMMANDS.has(key) && byName.has(key)) continue;
byName.set(key, command);
}📝 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
| for (const command of users) { | |
| const key = slashCommandKey(command.name); | |
| if (ADE_OWNED_SINGLE_WORD_COMMANDS.has(key)) continue; | |
| if (ADE_OWNED_CLAUDE_PARITY_COMMANDS.has(key)) continue; | |
| byName.set(key, command); | |
| for (const command of users) { | |
| const key = slashCommandKey(command.name); | |
| if (ADE_OWNED_SINGLE_WORD_COMMANDS.has(key)) continue; | |
| if (ADE_OWNED_CLAUDE_PARITY_COMMANDS.has(key) && byName.has(key)) continue; | |
| byName.set(key, command); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/commands.ts` around lines 204 - 208, The loop that
registers user commands currently skips any user command whose key is in
ADE_OWNED_CLAUDE_PARITY_COMMANDS unconditionally; change that logic so commands
in ADE_OWNED_CLAUDE_PARITY_COMMANDS are only suppressed when the ADE builtin is
actually present for the current provider (i.e., respect the earlier
provider-based filtering that may have removed the ADE builtin), by augmenting
the condition that checks ADE_OWNED_CLAUDE_PARITY_COMMANDS to also verify the
ADE builtin is enabled/available for this provider before continuing; update the
loop that calls slashCommandKey(command.name) and sets byName.set(key, command)
so valid runtime/user `/agents`-style commands are not hidden for non-Claude
providers.
Comment on lines
+4923
to
+4927
| const confirmed = await openRewindConfirmDialog({ | ||
| request, | ||
| preview, | ||
| files, | ||
| }); |
There was a problem hiding this comment.
Bind the rewind dialog to the session that produced the preview.
This dialog state is created from the current session, but the rendered dialog receives the live selectedSessionId. If the user switches chats before confirming, any dialog-side diff loading will point at the new chat while the confirm path still rewinds the old one.
Suggested fix
- const [rewindConfirmDialog, setRewindConfirmDialog] = useState<RewindFilesConfirmDialogState | null>(null);
+ const [rewindConfirmDialog, setRewindConfirmDialog] = useState<
+ (RewindFilesConfirmDialogState & { sessionId: string }) | null
+ >(null);
const rewindFilesFromMessage = useCallback(async (request: { messageId: string; timestamp: string; text: string }) => {
if (!selectedSessionId) return;
+ const sessionId = selectedSessionId;
setBusy(true);
setError(null);
try {
const preview = await window.ade.agentChat.rewindFiles({
- sessionId: selectedSessionId,
+ sessionId,
userMessageId: request.messageId,
dryRun: true,
});
...
const confirmed = await openRewindConfirmDialog({
+ sessionId,
request,
preview,
files,
});
if (!confirmed) return;
setBusy(true);
const result = await window.ade.agentChat.rewindFiles({
- sessionId: selectedSessionId,
+ sessionId,
userMessageId: request.messageId,
dryRun: false,
}); <RewindFilesConfirmDialog
state={rewindConfirmDialog}
- sessionId={selectedSessionId}
+ sessionId={rewindConfirmDialog?.sessionId ?? null}
onCancel={closeRewindConfirmDialog}
onConfirm={confirmRewindDialog}
/>Also applies to: 6544-6548
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines
4923 - 4927, The dialog is opened using live props but not bound to the session
that produced the preview—capture the current session id at the moment you call
openRewindConfirmDialog (e.g., const sessionIdAtOpen = selectedSessionId or
session.id) and pass that sessionId into the dialog call so the dialog's diff
loading and confirm path use the same session; update the
openRewindConfirmDialog invocation(s) (the one around openRewindConfirmDialog
with request/preview/files and the similar call at the other location) and the
dialog API to accept and use this sessionId (or a stable session token) instead
of reading the live selectedSessionId.
Comment on lines
54
to
93
| if (event.type === "subagent_progress") { | ||
| const existing = snapshots.get(event.taskId); | ||
| snapshots.set(event.taskId, { | ||
| taskId: event.taskId, | ||
| const key = event.agentId ?? event.taskId; | ||
| const existing = snapshots.get(key) ?? snapshots.get(event.taskId); | ||
| if (key !== event.taskId) snapshots.delete(event.taskId); | ||
| snapshots.set(key, { | ||
| taskId: existing?.taskId ?? event.taskId, | ||
| agentId: event.agentId ?? existing?.agentId, | ||
| agentType: event.agentType ?? existing?.agentType, | ||
| parentToolUseId: event.parentToolUseId ?? existing?.parentToolUseId ?? null, | ||
| description: event.description?.trim() || existing?.description || "Subagent task", | ||
| status: "running", | ||
| startedAt: existing?.startedAt ?? envelope.timestamp, | ||
| updatedAt: envelope.timestamp, | ||
| summary: event.summary?.trim() || existing?.summary || null, | ||
| finalSummary: existing?.finalSummary ?? null, | ||
| lastToolName: event.lastToolName ?? existing?.lastToolName, | ||
| background: existing?.background ?? false, | ||
| usage: event.usage ? { ...(existing?.usage ?? {}), ...event.usage } : existing?.usage, | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| if (event.type === "subagent_result") { | ||
| const existing = snapshots.get(event.taskId); | ||
| snapshots.set(event.taskId, { | ||
| const key = event.agentId ?? event.taskId; | ||
| const existing = snapshots.get(key) ?? snapshots.get(event.taskId); | ||
| if (key !== event.taskId) snapshots.delete(event.taskId); | ||
| snapshots.set(key, { | ||
| taskId: event.taskId, | ||
| agentId: event.agentId ?? existing?.agentId, | ||
| agentType: event.agentType ?? existing?.agentType, | ||
| parentToolUseId: event.parentToolUseId ?? existing?.parentToolUseId ?? null, | ||
| description: existing?.description ?? "Subagent task", | ||
| status: event.status, | ||
| startedAt: existing?.startedAt ?? envelope.timestamp, | ||
| updatedAt: envelope.timestamp, | ||
| summary: event.summary?.trim() || existing?.summary || null, | ||
| finalSummary: event.finalSummary?.trim() || event.summary?.trim() || existing?.finalSummary || null, | ||
| lastToolName: existing?.lastToolName, | ||
| background: existing?.background ?? false, | ||
| usage: event.usage ? { ...(existing?.usage ?? {}), ...event.usage } : existing?.usage, |
There was a problem hiding this comment.
Preserve progress/result metadata when updating subagent snapshots.
subagent_progress drops event.finalSummary, and both subagent_progress / subagent_result ignore event.background. That leaves the panel with stale summary text or the wrong foreground/background classification whenever the started event is missing or later events carry the authoritative metadata.
Suggested fix
if (event.type === "subagent_progress") {
const key = event.agentId ?? event.taskId;
const existing = snapshots.get(key) ?? snapshots.get(event.taskId);
if (key !== event.taskId) snapshots.delete(event.taskId);
snapshots.set(key, {
taskId: existing?.taskId ?? event.taskId,
agentId: event.agentId ?? existing?.agentId,
agentType: event.agentType ?? existing?.agentType,
parentToolUseId: event.parentToolUseId ?? existing?.parentToolUseId ?? null,
description: event.description?.trim() || existing?.description || "Subagent task",
status: "running",
startedAt: existing?.startedAt ?? envelope.timestamp,
updatedAt: envelope.timestamp,
summary: event.summary?.trim() || existing?.summary || null,
- finalSummary: existing?.finalSummary ?? null,
+ finalSummary: event.finalSummary?.trim() || existing?.finalSummary || null,
lastToolName: event.lastToolName ?? existing?.lastToolName,
- background: existing?.background ?? false,
+ background: event.background ?? existing?.background ?? false,
usage: event.usage ? { ...(existing?.usage ?? {}), ...event.usage } : existing?.usage,
});
continue;
}
if (event.type === "subagent_result") {
const key = event.agentId ?? event.taskId;
const existing = snapshots.get(key) ?? snapshots.get(event.taskId);
if (key !== event.taskId) snapshots.delete(event.taskId);
snapshots.set(key, {
taskId: event.taskId,
agentId: event.agentId ?? existing?.agentId,
agentType: event.agentType ?? existing?.agentType,
parentToolUseId: event.parentToolUseId ?? existing?.parentToolUseId ?? null,
description: existing?.description ?? "Subagent task",
status: event.status,
startedAt: existing?.startedAt ?? envelope.timestamp,
updatedAt: envelope.timestamp,
summary: event.summary?.trim() || existing?.summary || null,
finalSummary: event.finalSummary?.trim() || event.summary?.trim() || existing?.finalSummary || null,
lastToolName: existing?.lastToolName,
- background: existing?.background ?? false,
+ background: event.background ?? existing?.background ?? false,
usage: event.usage ? { ...(existing?.usage ?? {}), ...event.usage } : existing?.usage,
});
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/chat/chatExecutionSummary.ts` around
lines 54 - 93, The subagent snapshot updates for the "subagent_progress" and
"subagent_result" branches drop or ignore authoritative metadata: preserve
event.finalSummary and event.background when present so snapshots don't show
stale summary or wrong background; in the "subagent_progress" handler (where
snapshots.set is called) include finalSummary: event.finalSummary?.trim() ??
existing?.finalSummary ?? null and set background: event.background ??
existing?.background ?? false, and in the "subagent_result" handler ensure
finalSummary prefers event.finalSummary?.trim() then event.summary?.trim() then
existing?.finalSummary, and set background: event.background ??
existing?.background ?? false so both handlers consistently carry
forward/override these fields.
Comment on lines
+344
to
+349
| <button | ||
| type="button" | ||
| className="rounded-md border border-red-300/20 bg-red-500/18 px-3 py-1.5 text-[12px] font-semibold text-red-50/90 transition-colors hover:bg-red-500/24" | ||
| onClick={onConfirm} | ||
| autoFocus | ||
| > |
There was a problem hiding this comment.
Don't default focus to the destructive action.
autoFocus on Revert means an immediate Enter keypress confirms the rollback as soon as the dialog opens. In a destructive confirm flow, default focus should land on Cancel or another neutral control instead.
Suggested fix
<button
type="button"
className="rounded-md border border-white/[0.12] bg-white/[0.035] px-3 py-1.5 text-[12px] font-medium text-fg/70 transition-colors hover:bg-white/[0.06]"
onClick={onCancel}
+ autoFocus
>
Cancel
</button>
<button
type="button"
className="rounded-md border border-red-300/20 bg-red-500/18 px-3 py-1.5 text-[12px] font-semibold text-red-50/90 transition-colors hover:bg-red-500/24"
onClick={onConfirm}
- autoFocus
>
Revert {filesCount} file{filesCount === 1 ? "" : "s"}
</button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/chat/RewindFilesConfirmDialog.tsx`
around lines 344 - 349, The RewindFilesConfirmDialog currently sets autoFocus on
the destructive "Revert" button (the element with onClick={onConfirm}), which
can cause an accidental Enter to confirm; remove the autoFocus from that button
and instead set focus on a neutral control (e.g., the "Cancel" button) or rely
on no autofocus so the safe default remains; update the JSX in
RewindFilesConfirmDialog.tsx to remove the autoFocus prop from the button tied
to onConfirm and, if desired, add autoFocus to the cancel button or manage
initial focus programmatically in the component.
Comment on lines
+69
to
+83
| for (const summary of summaries) { | ||
| for (const file of summary.files) { | ||
| const existingKey = [...byPath.keys()].find((key) => pathsMatch(key, file.path)); | ||
| const key = existingKey ?? file.path; | ||
| const existing = byPath.get(key); | ||
| byPath.set(key, { | ||
| path: existing?.path ?? file.path, | ||
| additions: file.additions, | ||
| deletions: file.deletions, | ||
| status: file.status, | ||
| beforeSha: existing?.beforeSha ?? summary.beforeSha, | ||
| afterSha: summary.afterSha, | ||
| diffAvailable: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Accumulate counts when the same file appears in multiple summaries.
Line 76 and Line 77 overwrite prior values, so earlier turns for the same file are lost in the preview totals.
Proposed fix
byPath.set(key, {
path: existing?.path ?? file.path,
- additions: file.additions,
- deletions: file.deletions,
+ additions: (existing?.additions ?? 0) + file.additions,
+ deletions: (existing?.deletions ?? 0) + file.deletions,
status: file.status,
beforeSha: existing?.beforeSha ?? summary.beforeSha,
afterSha: summary.afterSha,
diffAvailable: 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
| for (const summary of summaries) { | |
| for (const file of summary.files) { | |
| const existingKey = [...byPath.keys()].find((key) => pathsMatch(key, file.path)); | |
| const key = existingKey ?? file.path; | |
| const existing = byPath.get(key); | |
| byPath.set(key, { | |
| path: existing?.path ?? file.path, | |
| additions: file.additions, | |
| deletions: file.deletions, | |
| status: file.status, | |
| beforeSha: existing?.beforeSha ?? summary.beforeSha, | |
| afterSha: summary.afterSha, | |
| diffAvailable: true, | |
| }); | |
| } | |
| for (const summary of summaries) { | |
| for (const file of summary.files) { | |
| const existingKey = [...byPath.keys()].find((key) => pathsMatch(key, file.path)); | |
| const key = existingKey ?? file.path; | |
| const existing = byPath.get(key); | |
| byPath.set(key, { | |
| path: existing?.path ?? file.path, | |
| additions: (existing?.additions ?? 0) + file.additions, | |
| deletions: (existing?.deletions ?? 0) + file.deletions, | |
| status: file.status, | |
| beforeSha: existing?.beforeSha ?? summary.beforeSha, | |
| afterSha: summary.afterSha, | |
| diffAvailable: true, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/chat/rewindFilesPreview.ts` around lines
69 - 83, The aggregation loop over summaries (iterating summaries ->
summary.files) currently overwrites prior counts in the byPath map; update the
object set on byPath (key derived via pathsMatch(file.path)) to accumulate
additions and deletions by summing existing?.additions + file.additions and
existing?.deletions + file.deletions, preserve existing?.beforeSha
(existing?.beforeSha ?? summary.beforeSha) and keep afterSha as either the
existing?.afterSha or summary.afterSha depending on desired semantics (e.g.,
existing?.afterSha ?? summary.afterSha), and ensure diffAvailable remains true
while choosing a consistent status (e.g., existing?.status ?? file.status) so
earlier turns for the same file are not lost.
Comment on lines
+189
to
+196
| chat_session_id text unique, | ||
| title text, | ||
| tags_json text, | ||
| created_at text not null, | ||
| updated_at text not null, | ||
| foreign key(lane_id) references lanes(id), | ||
| foreign key(chat_session_id) references terminal_sessions(id) on delete set null | ||
| ); |
There was a problem hiding this comment.
chat_session_id foreign key appears bound to the wrong parent table.
Line 189 stores a chat session identifier, but Line 195 constrains it to terminal_sessions(id). If FK checks are enabled, this can reject legitimate chat-session values and break Claude pointer upserts.
Suggested fix direction
create table if not exists claude_sessions (
session_id text primary key,
lane_id text not null,
chat_session_id text unique,
title text,
tags_json text,
created_at text not null,
updated_at text not null,
foreign key(lane_id) references lanes(id),
- foreign key(chat_session_id) references terminal_sessions(id) on delete set null
+ -- chat_session_id should reference the canonical chat sessions table,
+ -- or remain unconstrained if no such table exists in this DB.
+ -- foreign key(chat_session_id) references chat_sessions(id) on delete set null
);📝 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
| chat_session_id text unique, | |
| title text, | |
| tags_json text, | |
| created_at text not null, | |
| updated_at text not null, | |
| foreign key(lane_id) references lanes(id), | |
| foreign key(chat_session_id) references terminal_sessions(id) on delete set null | |
| ); | |
| chat_session_id text unique, | |
| title text, | |
| tags_json text, | |
| created_at text not null, | |
| updated_at text not null, | |
| foreign key(lane_id) references lanes(id), | |
| -- chat_session_id should reference the canonical chat sessions table, | |
| -- or remain unconstrained if no such table exists in this DB. | |
| -- foreign key(chat_session_id) references chat_sessions(id) on delete set null | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ios/ADE/Resources/DatabaseBootstrap.sql` around lines 189 - 196, The
foreign key on chat_session_id is referencing terminal_sessions(id) incorrectly;
update the constraint in the table definition that contains chat_session_id so
it references the correct parent table chat_sessions(id) (keeping the on delete
set null behavior if intended) and remove or correct the existing foreign
key(line with foreign key(chat_session_id) references terminal_sessions(id) on
delete set null) to prevent FK validation failures during Claude pointer
upserts.
Owner
Author
| ...(session.interactionMode !== undefined ? { interactionMode: session.interactionMode } : {}), | ||
| ...(session.claudePermissionMode ? { claudePermissionMode: session.claudePermissionMode } : {}), | ||
| ...(session.claudeOutputStyle ? { claudeOutputStyle: session.claudeOutputStyle } : {}), | ||
| ...(session.codexApprovalPolicy ? { codexApprovalPolicy: session.codexApprovalPolicy } : {}), |
There was a problem hiding this comment.
Mixed tab and space indentation in session summary
Low Severity
Lines 304–308 use hard-tab indentation (\t) while the rest of the file and surrounding lines use spaces. This inconsistency was introduced when the claudeOutputStyle spread was added between the existing claudePermissionMode and codexApprovalPolicy lines.
Reviewed by Cursor Bugbot for commit 03e46d5. Configure here.
Copilot stopped work on behalf of
arul28 due to an error
May 12, 2026 09:35
Owner
Author
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1dd5c3a. Configure here.
| .replace(/opt/gi, "alt") | ||
| .replace(/command/gi, "cmd") | ||
| .replace(/super/gi, "cmd") | ||
| .replace(/win/gi, "cmd"); |
There was a problem hiding this comment.
Keybinding normalizer corrupts keys containing "win" substring
Low Severity
In normalizeSingleKeystroke, the replacement .replace(/win/gi, "cmd") matches any substring containing "win", not just the standalone modifier token. A key name like "window" or "rewind" would be corrupted to "cmdow" or "recmdd". The same issue affects .replace(/opt/gi, "alt"), which would corrupt a key named "option" into "altionaln" (first option → alt via the earlier replace, then opt in the result gets hit again — though that's partially mitigated by order). Using word-boundary-aware patterns or exact-match logic would prevent false matches.
Reviewed by Cursor Bugbot for commit 1dd5c3a. Configure here.
Copilot stopped work on behalf of
arul28 due to an error
May 12, 2026 10:03
arul28
deleted the
ade-31-look-into-updating-claude-agent-sdk-and-new-features
branch
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
Validation
Rebase note
Rebased onto current origin/main before opening. Resolved the localRuntimeConnectionPool.test.ts overlap by preserving both sides: the runtime receives the temp project root and assertions compare against fs.realpathSync.native(projectRoot).
Note
Medium Risk
Touches Claude integration surfaces across both the
ade codeTUI and desktop runtime status/packaging, so regressions could affect provider availability detection, command routing, and session UX. Changes are mostly additive but broad and cross-cutting (new commands, keybindings, statusline runner, and updated SDK packaging).Overview
Updates Claude integration and UX surfaces across the CLI TUI and desktop.
In
ade code, adds Claude-parity controls including provider-scoped slash commands (e.g./context,/mcp,/plugin,/output-style,/agents,/skills,/init,/tag, plus SDK chat commands like/compact//usage//insights//fast//goal), a new subagents right-pane with tabbed views, richer token stats (cache tokens, context window, rate-limit notices), and Claudeautopermission mode support.Introduces Claude-compatible keybindings (
~/.claude/keybindings.json) with validation, multi-chord dispatch, live reload, and maps many actions into TUI behaviors (history search, external editor, clipboard image attach, pane toggles, scrolling). Adds Claude statusline support that runs a configured shell command with a JSON payload and renders up to 3 lines inModelStatus, plus optional vim-mode indicator driven by Claude settings.On the desktop side, bumps
@anthropic-ai/claude-agent-sdkto0.2.139, expands packaging/unpack + pruning logic for platform-specific native SDK artifacts, refines runtime smoke probing and error classification for missing binaries vs auth/runtime failures, and changes AI provider status to report structured Claude availability (binary+auth) instead of a boolean.Reviewed by Cursor Bugbot for commit 1dd5c3a. Configure here.
Summary by CodeRabbit
New Features
Improvements
Greptile Summary
This PR expands the Claude agent runtime integration — migrating from the deprecated
unstable_v2_createSessionAPI to the newstartup/querySDK surface — and adds a broad set of companion features across the desktop app andadeCLI.ClaudeInputPump(async-iterable message queue),ClaudeSubprocessReaper(SIGTERM\u2192SIGKILL lifecycle manager),runtimeEvents.ts(canonical event vocabulary with forward/reverse conversion helpers), output-style discovery, plugin management, MCP server controls, andrewindFiles/context-usage query support.ade codeUX additions: configurable keybinding system with chord dispatch and vim-mode awareness, a statusline command runner that streams JSON context to user scripts, a subagents side panel, and a per-file diff preview replacing the old flat-text rewind dialog.claudeboolean inAiIntegrationStatus.availableProvidersis replaced by a structuredAiClaudeAvailabilityobject that separately tracks binary presence and auth readiness, enabling richer UI feedback on partial states.Confidence Score: 5/5
Safe to merge; all findings are display or style nits with no impact on correctness of the runtime lifecycle, event routing, or data persistence.
The core SDK migration (startup/query API), subprocess lifecycle management, and session persistence changes are well-structured and covered by tests. The three flagged issues are limited to a misleading line-count badge in the rewind dialog, dead code in a normalization helper, and fragile string matching in an availability heuristic — none affect the happy-path behavior or data integrity.
apps/desktop/src/renderer/components/chat/rewindFilesPreview.ts (accumulated additions/deletions display) and apps/desktop/src/main/services/ai/aiIntegrationService.ts (binaryOnlyBlockers heuristic).
Important Files Changed
unstable_v2_createSessionto the newstartup/querySDK API; adds rewindFiles, context usage, output styles, plugin management, and MCP controls; internal state shape updated accordingly.subagent_*and newsubagent.*event shapes; logic is internally consistent and well-tested.reapStaleRegistryat construction time is intentional for startup cleanup of orphaned pids..mdfiles from ancestor.claudedirectories and installed plugins; usesMAX_ANCESTOR_DEPTHandMAX_PLUGIN_DEPTHcaps to bound filesystem traversal.discoverProjectClaudeMcpServersis exported for future use but intentionally not wired intobuildClaudeMcpServers(noted in prior review thread).beforeSha/afterShatracking is correct but per-fileadditions/deletionsare overwritten with each summary instead of being accumulated, giving misleading counts in the rewind dialog.claudeavailability flag with a structuredAiClaudeAvailabilityobject including binary presence and auth readiness;binaryOnlyBlockersrelies on fragile substring matching of probe error messages to determine readiness.claude_sessionstable for tracking SDK session pointers with title/tag metadata; upsert logic correctly handles chatSessionId uniqueness enforcement across sessions.normalizeSingleKeystroke(redundant.toLowerCase()and unreachable space mapping).shell: trueis intentional for user-defined script commands.Sequence Diagram
sequenceDiagram participant UI as Renderer / ade CLI participant ACS as agentChatService participant SDK as claude-agent-sdk participant Reaper as ClaudeSubprocessReaper participant Pump as ClaudeInputPump UI->>ACS: createSession / resumeSession ACS->>Pump: new ClaudeInputPump() ACS->>SDK: "startup({ mcpServers, spawnProcess })" SDK->>Reaper: spawnClaudeCodeProcess(options, metadata) Reaper-->>SDK: ChildProcess (registered + persisted) SDK-->>ACS: WarmQuery handle UI->>ACS: sendMessage(text) ACS->>Pump: pump.push(SDKUserMessage) ACS->>SDK: query(inputPump, options) loop Stream events SDK-->>ACS: SDKMessage ACS->>ACS: buildCanonicalAgentChatRuntimeEvent() ACS-->>UI: emit AgentChatEvent end UI->>ACS: rewindFiles(userMessageId) ACS->>SDK: sessionQuery.rewindFiles(messageId) SDK-->>ACS: ClaudeRewindFilesResult ACS-->>UI: AgentChatRewindFilesResult UI->>ACS: interrupt / teardown ACS->>Pump: pump.close() ACS->>Reaper: reapAll(teardown) Reaper->>Reaper: SIGTERM then SIGKILL after graceComments Outside Diff (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx, line 4625-4690 (link)/contextand/output-stylecommand handlersThe two new slash-command intercept blocks use hard-tab indentation (
\t) while the rest of the file uses spaces. The indentation should be normalised to match the surrounding code.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix: keep packaged Claude native runtime" | Re-trigger Greptile