Upgrade Claude Agent SDK chat integration#637
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughBumps the Claude Agent SDK to ChangesClaude SDK Integration and UI Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arul28
changed the title
|
@copilot review but do not make fixes |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c3a7ef6bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0115d940a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3d55d842
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/chat/chatToolAppearance.test.ts (1)
90-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpand coverage for other newly added
getTargetmappings.Line 90 validates
EnterWorktree, but Lines 57-70 inchatToolAppearance.tsxadd many newgetTargetpaths. A small table-driven test here would catch argument-key regressions early.♻️ Suggested test pattern
+ it("extracts targets for newly added Claude tools", () => { + const cases: Array<[string, Record<string, unknown>, string | null]> = [ + ["TaskCreate", { subject: "ship SDK" }, "ship SDK"], + ["TaskGet", { taskId: "123" }, "123"], + ["TaskUpdate", { description: "update docs" }, "update docs"], + ["REPL", { description: "run snippet" }, "run snippet"], + ["Workflow", { name: "nightly-sync" }, "nightly-sync"], + ["CronCreate", { cron: "0 * * * *" }, "0 * * * *"], + ["CronDelete", { id: "cron-1" }, "cron-1"], + ["ScheduleWakeup", { reason: "retry later" }, "retry later"], + ["Monitor", { command: "tail -f logs" }, "tail -f logs"], + ["Artifact", { file_path: "/tmp/out.txt" }, "/tmp/out.txt"], + ["PushNotification", { message: "done" }, "done"], + ["RemoteTrigger", { action: "deploy" }, "deploy"], + ["EnterWorktree", { path: "/tmp/wt" }, "/tmp/wt"], + ]; + for (const [tool, args, expected] of cases) { + const meta = getToolMeta(tool); + expect(meta.getTarget).toBeDefined(); + expect(meta.getTarget!(args)).toBe(expected); + } + });🤖 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/chatToolAppearance.test.ts` around lines 90 - 97, The test for getTarget mappings in the chatToolAppearance.test.ts file only covers the EnterWorktree tool, but the chatToolAppearance.tsx file (lines 57-70) contains many other tools with getTarget path mappings that should also be validated. Expand the test coverage by implementing a table-driven test pattern that iterates through all the newly added getTarget mappings, testing each tool with its respective argument keys and expected return values to catch potential argument-key regressions early. This will ensure all tools with getTarget functions defined in the source file are properly covered.
🤖 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/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 3861-3868: When the chat-actions drawer is already open
(chatActionsOpen is true), check if the Agents tab is currently visible within
that drawer before returning early. If the Agents tab is visible, consume the
auto-open marker (burn the trackedActionCount flag) before returning, so that
closing and reopening the drawer doesn't immediately reopen it. Only skip the
marker consumption and return early if the Agents tab is not currently visible,
allowing the logic to retry when the drawer closes.
- Around line 5903-5923: The prompt_suggestion event handling block (which sets
setPromptSuggestionsBySession) is currently executing after the event has been
queued to pendingEventQueueRef.current.push(envelope), causing UI-only
suggestion data to pollute the transcript state cache. Move the entire
prompt_suggestion handling block (checking for envelope.event.type ===
"prompt_suggestion") to execute before the envelope is pushed to the queue, and
add a return statement after handling it so the suggestion event does not get
added to the transcript or eventsBySession cache.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/chat/chatToolAppearance.test.ts`:
- Around line 90-97: The test for getTarget mappings in the
chatToolAppearance.test.ts file only covers the EnterWorktree tool, but the
chatToolAppearance.tsx file (lines 57-70) contains many other tools with
getTarget path mappings that should also be validated. Expand the test coverage
by implementing a table-driven test pattern that iterates through all the newly
added getTarget mappings, testing each tool with its respective argument keys
and expected return values to catch potential argument-key regressions early.
This will ensure all tools with getTarget functions defined in the source file
are properly covered.
🪄 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: 09bbb2ba-9e89-4492-8158-c10daad0236d
⛔ Files ignored due to path filters (3)
apps/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/features/chat/README.mdis excluded by!docs/**
📒 Files selected for processing (14)
.agents/skills/ship/SKILL.mdapps/ade-cli/package.jsonapps/desktop/package.jsonapps/desktop/src/main/services/ai/tools/systemPrompt.test.tsapps/desktop/src/main/services/ai/tools/systemPrompt.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatTasksPanel.tsxapps/desktop/src/renderer/components/chat/chatToolAppearance.test.tsapps/desktop/src/renderer/components/chat/chatToolAppearance.tsx
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ccd4fda16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 255be31602
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18a16645f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 543f59c216
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0acc5f0d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43558326e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb796a309b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
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.test.tsx (1)
5396-5419: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winEnsure timer spy cleanup even on assertion failure.
At Line 5396 and Line 5419,
setTimeoutSpy.mockRestore()should run in afinally; otherwise a failure at Line 5418 can leak the spy into later tests.Suggested fix
- const setTimeoutSpy = vi.spyOn(window, "setTimeout"); - - act(() => { - emitChatEvent({ - sessionId: "session-1", - timestamp: "2026-03-24T06:00:00.000Z", - sequence: 1, - event: { - type: "prompt_suggestion", - suggestion: "Continue primary work", - }, - } as AgentChatEventEnvelope); - emitChatEvent({ - sessionId: "session-2", - timestamp: "2026-03-24T06:00:01.000Z", - sequence: 1, - event: { - type: "prompt_suggestion", - suggestion: "Continue background work", - }, - } as AgentChatEventEnvelope); - }); - expect(setTimeoutSpy.mock.calls.some(([, delay]) => delay === 16)).toBe(false); - setTimeoutSpy.mockRestore(); + const setTimeoutSpy = vi.spyOn(window, "setTimeout"); + try { + act(() => { + emitChatEvent({ + sessionId: "session-1", + timestamp: "2026-03-24T06:00:00.000Z", + sequence: 1, + event: { + type: "prompt_suggestion", + suggestion: "Continue primary work", + }, + } as AgentChatEventEnvelope); + emitChatEvent({ + sessionId: "session-2", + timestamp: "2026-03-24T06:00:01.000Z", + sequence: 1, + event: { + type: "prompt_suggestion", + suggestion: "Continue background work", + }, + } as AgentChatEventEnvelope); + }); + expect(setTimeoutSpy.mock.calls.some(([, delay]) => delay === 16)).toBe(false); + } finally { + setTimeoutSpy.mockRestore(); + }🤖 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.test.tsx` around lines 5396 - 5419, The setTimeoutSpy.mockRestore() cleanup at the end of the test will not execute if the preceding expect() assertion fails, causing the spy to leak into subsequent tests. Wrap the setTimeoutSpy variable declaration and the emitChatEvent act block with a try-finally structure, moving the setTimeoutSpy.mockRestore() call into the finally block to ensure the spy is always cleaned up regardless of whether the assertion passes or fails.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/chat/agentChatService.ts (1)
12769-12770: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRedundant
continue— confirm intended branch behavior.Both lines fall through to
continue, soif (resultSeen) continue;(Line 12769) is dead and identical to the unconditionalcontinue;on Line 12770. If theresultSeencheck was meant to alter behavior here (e.g. skip emitting the suggestion, orbreakthe drain), the edit looks incomplete; otherwise drop the redundant guard.♻️ Drop the redundant guard (if no behavior change intended)
} - if (resultSeen) continue; continue; }🤖 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/agentChatService.ts` around lines 12769 - 12770, Remove the redundant conditional `if (resultSeen) continue;` statement on line 12769 in the agentChatService.ts file, as it is dead code that unconditionally falls through to the identical `continue;` on the next line. If the resultSeen check was intended to trigger different behavior (such as breaking the loop or skipping specific logic), implement that intended behavior instead; otherwise, keep only the single unconditional continue statement.
🤖 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.
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx`:
- Around line 5396-5419: The setTimeoutSpy.mockRestore() cleanup at the end of
the test will not execute if the preceding expect() assertion fails, causing the
spy to leak into subsequent tests. Wrap the setTimeoutSpy variable declaration
and the emitChatEvent act block with a try-finally structure, moving the
setTimeoutSpy.mockRestore() call into the finally block to ensure the spy is
always cleaned up regardless of whether the assertion passes or fails.
---
Nitpick comments:
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 12769-12770: Remove the redundant conditional `if (resultSeen)
continue;` statement on line 12769 in the agentChatService.ts file, as it is
dead code that unconditionally falls through to the identical `continue;` on the
next line. If the resultSeen check was intended to trigger different behavior
(such as breaking the loop or skipping specific logic), implement that intended
behavior instead; otherwise, keep only the single unconditional continue
statement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca90abfa-fad4-4d56-ae2f-6dc942796da2
📒 Files selected for processing (7)
apps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.test.tsxapps/desktop/src/renderer/components/chat/ChatTasksPanel.tsxapps/desktop/src/renderer/components/chat/chatToolAppearance.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/components/chat/chatToolAppearance.test.ts
- apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
1 participant
Summary
@anthropic-ai/claude-agent-sdk@0.3.186for desktop and ADE CLI.runSessionTurndefault timeout.Follow-up ticket
https://linear.app/ade-linear/issue/ADE-103/add-long-lived-claude-sdk-wakecron-support-for-ade-chats
Validation
npm --prefix apps/desktop run typechecknpm --prefix apps/ade-cli run typecheckcd apps/desktop && npx vitest run src/main/services/chat/agentChatService.test.tscd apps/desktop && npx vitest run src/renderer/components/chat/AgentChatPane.test.tsx src/renderer/components/chat/AgentChatComposer.test.tsxcd apps/desktop && npx vitest run src/main/services/ai/tools/systemPrompt.test.ts src/renderer/components/chat/chatExecutionSummary.test.ts src/renderer/components/chat/chatTranscriptRows.test.tsnpm run test:desktop:shardednpm --prefix apps/desktop run buildnpm --prefix apps/desktop run lint(warnings only; existing baseline)npm --prefix apps/ade-cli run buildnpm --prefix apps/ade-cli run testnode scripts/validate-docs.mjsgit diff --checkSummary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores
Greptile Summary
This PR upgrades the Claude Agent SDK to 0.3.186 and wires its new event surfaces into ADE's chat pipeline, alongside a fix that prevents long-running background chat turns from being killed by the 5-minute
runSessionTurndefault RPC timeout.agentChatService.ts): introducesreadNextClaudeTurnMessage()withpendingPostResultNextcarry-over so the service can drain SDK-documented tail messages (prompt_suggestion,tool_use_summary, stale system events) that arrive after theresultmessage before moving on to the next user turn, without stalling indefinitely.api_retry,model_refusal_fallback,informational,notification,memory_recall,mirror_error,permission_denied, andworker_shutting_downare each translated intosystem_noticechat events; duplicate permission-denied UI is suppressed withemittedPermissionDeniedToolUseIds.AgentChatPane.tsx): replaces a singlepromptSuggestionscalar with apromptSuggestionsBySessionrecord so suggestions from background sessions never bleed into the visible composer; the composer shows them as placeholder text rather than a ghost overlay.Confidence Score: 5/5
Safe to merge; the new post-result drain and event-handler paths are well-scoped and the timeout fix for background launches is straightforward.
The core logic change — readNextClaudeTurnMessage with carry-over pendingPostResultNext — is complex but correctly isolated to the post-result tail phase and backed by focused tests. New system-event handlers all take a defensive typeof approach and fall back gracefully to empty strings. The per-session prompt-suggestion keying closes a real cross-tab leak. The only finding is dead code in the prompt_suggestion handler that has no runtime impact.
The readNextClaudeTurnMessage function in agentChatService.ts is the most complex addition and worth a careful read, but no blocking issues were found there.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant U as UserTurn participant R as readNextClaudeTurnMessage participant Q as sessionQuery participant L as MainLoop U->>R: "call with resultSeen=false" R->>R: drain pendingPostResultNext from prior turn R->>Q: sessionQuery.next() Q-->>R: tool/text message R-->>L: message L->>L: process tool/text handlers Q-->>R: result message R-->>L: result L->>L: "resultSeen=true continue" U->>R: "call with resultSeen=true 1s timeout" R->>Q: sessionQuery.next() stored as pendingPostResultNext alt message within 1s Q-->>R: prompt_suggestion or tool_use_summary R-->>L: message L->>L: stale tail discard or prompt_suggestion handler else timeout fires R-->>L: null L->>L: break post-result drain end Note over L: emit pendingWorkerShutdownReason if set Note over R: pendingPostResultNext carried into next turn%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant U as UserTurn participant R as readNextClaudeTurnMessage participant Q as sessionQuery participant L as MainLoop U->>R: "call with resultSeen=false" R->>R: drain pendingPostResultNext from prior turn R->>Q: sessionQuery.next() Q-->>R: tool/text message R-->>L: message L->>L: process tool/text handlers Q-->>R: result message R-->>L: result L->>L: "resultSeen=true continue" U->>R: "call with resultSeen=true 1s timeout" R->>Q: sessionQuery.next() stored as pendingPostResultNext alt message within 1s Q-->>R: prompt_suggestion or tool_use_summary R-->>L: message L->>L: stale tail discard or prompt_suggestion handler else timeout fires R-->>L: null L->>L: break post-result drain end Note over L: emit pendingWorkerShutdownReason if set Note over R: pendingPostResultNext carried into next turnReviews (10): Last reviewed commit: "Track settled Claude post-result drains" | Re-trigger Greptile