feat(studio): Add a pop out window for the Studio Code Agent#422
Open
htolentino-nvidia wants to merge 1 commit into
Open
feat(studio): Add a pop out window for the Studio Code Agent#422htolentino-nvidia wants to merge 1 commit into
htolentino-nvidia wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughIntroduces a shared ChangesClaude Code Shared Runtime and Top-Bar Pop-out
Sequence Diagram(s)sequenceDiagram
participant User
participant GlobalNav
participant TopBarChat as ClaudeCodeTopBarChat
participant Provider as ClaudeCodeChatProvider
participant Runtime as useClaudeCodeChatRuntime
participant Storage as activeSessionStorage
participant API as fetchSessionHistory
rect rgba(100, 149, 237, 0.5)
note over GlobalNav,TopBarChat: App load / route change
GlobalNav->>GlobalNav: matchPath → shouldMountClaudeCodeTopBarChat
GlobalNav->>TopBarChat: render (when not dashboard/full-chat route)
end
rect rgba(144, 238, 144, 0.5)
note over Provider,Storage: Provider mount
Provider->>Storage: readStoredActiveSessionId(workspace)
Storage-->>Provider: sessionId
Provider->>API: fetchSessionHistory(sessionId)
API-->>Provider: history
Provider->>Runtime: loadSession(sessionId, artifacts, messages)
end
rect rgba(255, 165, 0, 0.5)
note over User,Provider: User opens top-bar pop-out and starts new chat
User->>TopBarChat: click open
User->>TopBarChat: click New chat
TopBarChat->>Provider: startNewChat()
Provider->>Runtime: handleReset()
Runtime->>Storage: writeStoredActiveSessionId(workspace, null)
end
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsx (1)
200-208: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for session-scoped full-chat paths
Current cases only check the base Claude route. Add one case for a session URL to prevent regressions in top-bar mount gating.
🤖 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 `@web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsx` around lines 200 - 208, Add a new test case after the existing test to verify that GlobalNav does not mount on session-scoped full-chat paths. Create a similar test structure to the current one in the renderGlobalNav function, but instead of using just the base ROUTES.workspace.claudeCodeChat route, pass a session-scoped full-chat path (such as one that includes a session ID in the URL parameters) to ensure the top-bar chat element is not mounted for session-specific chat routes as well.web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.test.tsx (1)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression assertion that
onNewChatis passed toClaudeCodeLayout.Current tests won’t catch the missing
onNewChatwiring from the route to layout.Suggested test hardening
vi.mock('`@studio/routes/agents/ClaudeCodeChatRoute/ClaudeCodeLayout`', () => ({ ClaudeCodeLayout: ({ activeSessionId, + onNewChat, children, }: { activeSessionId?: string; + onNewChat?: () => void; children: ReactNode; }) => ( - <div data-active-session-id={activeSessionId ?? ''} data-testid="chat-layout"> + <div + data-active-session-id={activeSessionId ?? ''} + data-has-on-new-chat={String(Boolean(onNewChat))} + data-testid="chat-layout" + > {children} </div> ), }));it('renders the chat for the active session once loaded', () => { mocks.chat.sessionId = 'session-existing'; renderClaudeCodeChatRoute({ search: '?session=session-existing' }); expect(screen.getByTestId('chat-thread')).toBeInTheDocument(); expect(screen.getByTestId('chat-layout')).toHaveAttribute( 'data-active-session-id', 'session-existing' ); + expect(screen.getByTestId('chat-layout')).toHaveAttribute('data-has-on-new-chat', 'true'); });Also applies to: 107-117
🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.test.tsx` around lines 44 - 56, The ClaudeCodeLayout mock function does not capture or verify the onNewChat prop, which means tests cannot catch if this prop is missing from the route implementation. Update the mock for ClaudeCodeLayout to accept onNewChat as part of the destructured props (along with activeSessionId and children), and then add test assertions to verify that onNewChat is actually being passed to the ClaudeCodeLayout component when it's rendered. This ensures that any future changes that remove or forget to wire onNewChat from the route will be caught by the test suite.web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.test.tsx (1)
64-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for “load then reset” race.
Current tests don’t assert that an in-flight
loadSessioncannot apply afterstartNewChat. This is the critical stale-request edge for this provider.🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.test.tsx` around lines 64 - 97, Add a new test case in the ClaudeCodeChatProvider.test.tsx file that validates the race condition between loadSession and startNewChat. The test should: set up mocks for getClaudeCodeSessionHistory to simulate an in-flight request, initiate a loadSession action, immediately call startNewChat before the loadSession promise resolves, then verify that applySession is never called or is called with the expected new session state (not the stale loaded session). This ensures that stale responses from loadSession cannot overwrite state after startNewChat has been invoked.
🤖 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 `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx`:
- Around line 32-34: The matchPath call on line 33 with end: true performs an
exact match on the pathname, which fails when the Claude code chat URL includes
additional segments like session IDs, causing line 34 to incorrectly set
shouldMountClaudeCodeTopBarChat to true on full chat pages. Remove the end: true
parameter from the matchPath call for ROUTES.workspace.claudeCodeChat so it
matches any pathname that starts with the Claude code chat route, regardless of
additional path segments that follow it.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsx`:
- Around line 58-59: The requestedSessionIdRef is not being cleared when
startNewChat executes, which allows pending loadSession requests from the
previous session to complete and unexpectedly rehydrate the old session after a
reset. Clear or invalidate requestedSessionIdRef (set to null or undefined) at
the beginning of the startNewChat function to cancel any pending requests, and
apply the same fix to the other reset/new chat scenario referenced at lines
95-97 to ensure consistency across all places where a new chat session is
initiated.
In `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsx`:
- Around line 103-107: The ClaudeCodeLayout component at line 103 is missing the
onNewChat callback prop, which prevents the "New Chat" button in the history
panel from resetting the chat state. Add the onNewChat prop to the
ClaudeCodeLayout component and wire it to the appropriate reset handler (likely
startNewChat or handleChatReset) so that clicking "New Chat" properly resets the
shared chat runtime instead of only navigating.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts`:
- Around line 565-576: The `loadSession` callback function does not cancel any
active/in-flight runs before loading a new session. This allows old stream
events to corrupt the newly loaded session state if a user loads history while a
run is still executing. Add a call to cancel active runs (such as a `cancelRun`
or similar method that stops in-flight streams) at the very beginning of the
`loadSession` callback body, before any state updates like `setSessionId`,
`setArtifacts`, or `replaceMessages` are called.
In `@web/packages/studio/src/routes/PageLayout/index.tsx`:
- Around line 50-52: The ClaudeCodeChatProvider component is being reused when
the workspace changes, which allows internal state to persist across workspace
boundaries. Add a key prop to the ClaudeCodeChatProvider component that uniquely
identifies each workspace (such as workspace.id or workspace.name) to force
React to destroy and recreate the provider instance whenever the workspace value
changes, preventing state from bleeding between workspaces.
---
Nitpick comments:
In `@web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsx`:
- Around line 200-208: Add a new test case after the existing test to verify
that GlobalNav does not mount on session-scoped full-chat paths. Create a
similar test structure to the current one in the renderGlobalNav function, but
instead of using just the base ROUTES.workspace.claudeCodeChat route, pass a
session-scoped full-chat path (such as one that includes a session ID in the URL
parameters) to ensure the top-bar chat element is not mounted for
session-specific chat routes as well.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.test.tsx`:
- Around line 64-97: Add a new test case in the ClaudeCodeChatProvider.test.tsx
file that validates the race condition between loadSession and startNewChat. The
test should: set up mocks for getClaudeCodeSessionHistory to simulate an
in-flight request, initiate a loadSession action, immediately call startNewChat
before the loadSession promise resolves, then verify that applySession is never
called or is called with the expected new session state (not the stale loaded
session). This ensures that stale responses from loadSession cannot overwrite
state after startNewChat has been invoked.
In `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.test.tsx`:
- Around line 44-56: The ClaudeCodeLayout mock function does not capture or
verify the onNewChat prop, which means tests cannot catch if this prop is
missing from the route implementation. Update the mock for ClaudeCodeLayout to
accept onNewChat as part of the destructured props (along with activeSessionId
and children), and then add test assertions to verify that onNewChat is actually
being passed to the ClaudeCodeLayout component when it's rendered. This ensures
that any future changes that remove or forget to wire onNewChat from the route
will be caught by the test suite.
🪄 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: Enterprise
Run ID: 48b00c48-b875-4635-8c05-378062f630d5
📒 Files selected for processing (19)
web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsxweb/packages/studio/src/components/Layouts/GlobalNav/index.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/PageLayout/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeChatThread.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeLayout.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeTopBarChat.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/activeSessionStorage.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/useClaudeCodeChatContext.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useCustomAssistantChatRuntime.tsweb/packages/studio/src/util/localStorage.ts
Comment on lines
+32
to
+34
| const isClaudeCodeChatRoute = | ||
| matchPath({ path: ROUTES.workspace.claudeCodeChat, end: true }, location.pathname) !== null; | ||
| const shouldMountClaudeCodeTopBarChat = !isDashboardRoute && !isClaudeCodeChatRoute; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
matchPath(..., end: true) can misclassify full chat subroutes
If the full chat URL includes a session segment, Line 33 won’t match and Line 34 will mount the compact pop-out on full-chat pages. Use a non-exact match for the Claude route family.
Suggested fix
- const isClaudeCodeChatRoute =
- matchPath({ path: ROUTES.workspace.claudeCodeChat, end: true }, location.pathname) !== null;
+ const isClaudeCodeChatRoute =
+ matchPath({ path: ROUTES.workspace.claudeCodeChat, end: false }, location.pathname) !== 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
| const isClaudeCodeChatRoute = | |
| matchPath({ path: ROUTES.workspace.claudeCodeChat, end: true }, location.pathname) !== null; | |
| const shouldMountClaudeCodeTopBarChat = !isDashboardRoute && !isClaudeCodeChatRoute; | |
| const isClaudeCodeChatRoute = | |
| matchPath({ path: ROUTES.workspace.claudeCodeChat, end: false }, location.pathname) !== null; | |
| const shouldMountClaudeCodeTopBarChat = !isDashboardRoute && !isClaudeCodeChatRoute; |
🤖 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 `@web/packages/studio/src/components/Layouts/GlobalNav/index.tsx` around lines
32 - 34, The matchPath call on line 33 with end: true performs an exact match on
the pathname, which fails when the Claude code chat URL includes additional
segments like session IDs, causing line 34 to incorrectly set
shouldMountClaudeCodeTopBarChat to true on full chat pages. Remove the end: true
parameter from the matchPath call for ROUTES.workspace.claudeCodeChat so it
matches any pathname that starts with the Claude code chat route, regardless of
additional path segments that follow it.
Comment on lines
+58
to
+59
| requestedSessionIdRef.current = trimmedSessionId; | ||
| setLoadStatus('loading'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate pending loadSession requests on reset/new chat.
requestedSessionIdRef is not cleared when startNewChat runs. A slow history fetch can complete after reset and rehydrate an old session unexpectedly.
Suggested fix
+ const startNewChat = useCallback(() => {
+ requestedSessionIdRef.current = null;
+ setLoadStatus('idle');
+ handleReset();
+ }, [handleReset]);
+
const value = useMemo(
- () => ({ chat, loadStatus, loadSession, startNewChat: handleReset }),
- [chat, handleReset, loadSession, loadStatus]
+ () => ({ chat, loadStatus, loadSession, startNewChat }),
+ [chat, loadStatus, loadSession, startNewChat]
);Also applies to: 95-97
🤖 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
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsx`
around lines 58 - 59, The requestedSessionIdRef is not being cleared when
startNewChat executes, which allows pending loadSession requests from the
previous session to complete and unexpectedly rehydrate the old session after a
reset. Clear or invalidate requestedSessionIdRef (set to null or undefined) at
the beginning of the startNewChat function to cancel any pending requests, and
apply the same fix to the other reset/new chat scenario referenced at lines
95-97 to ensure consistency across all places where a new chat session is
initiated.
Comment on lines
+103
to
+107
| <ClaudeCodeLayout activeSessionId={sessionId ?? undefined} artifacts={artifacts}> | ||
| <AccessibleTitle title={`Code Agent chat for ${workspace}`}> | ||
| <Stack className="h-full w-full py-density-lg"> | ||
| <Stack className="min-h-0 w-full flex-1"> | ||
| <AssistantRuntimeProvider runtime={runtime}> | ||
| <AssistantChatThread | ||
| contentClassName="mx-auto w-full max-w-180 px-density-2xl" | ||
| composerContainerClassName="mx-auto w-full max-w-180 px-density-2xl" | ||
| viewportClassName={CHAT_VIEWPORT_SCROLLBAR_CLASS} | ||
| hideAssistantMessageActions | ||
| toolCallPartComponent={ClaudeCodeToolCallPart} | ||
| attributes={{ | ||
| ThreadViewport: { | ||
| ref: chatViewportRef, | ||
| }, | ||
| }} | ||
| placeholder="Ask Claude Code to work in this workspace" | ||
| onReset={handleChatReset} | ||
| showRunningIndicator={!decisionRequest && !inputRequest} | ||
| messageContentProps={{ markdownLinkComponent: ClaudeCodeStudioLink }} | ||
| emptyState={{ | ||
| slotHeading: 'Start a Claude Code session', | ||
| slotSubheading: 'Ask Claude Code to work in this workspace.', | ||
| }} | ||
| composerOverride={ | ||
| decisionRequest ? ( | ||
| <AgentDecisionInput | ||
| request={decisionRequest} | ||
| choices={decisionChoices} | ||
| defaultChoiceId={decisionChoices[0]?.id} | ||
| status={decisionStatus} | ||
| onSubmit={resolveDecisionRequest} | ||
| onSkip={skipDecisionRequest} | ||
| /> | ||
| ) : inputRequest ? ( | ||
| <BlockingInputComposer | ||
| inputRequest={inputRequest} | ||
| inputStatus={inputStatus} | ||
| workspace={workspace} | ||
| onSubmit={handleInputSubmit} | ||
| onSkip={skipInputRequest} | ||
| /> | ||
| ) : undefined | ||
| } | ||
| /> | ||
| </AssistantRuntimeProvider> | ||
| <ClaudeCodeChatThread chat={chat} onReset={handleChatReset} /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wire startNewChat into ClaudeCodeLayout to make “New Chat” actually reset state.
At Line 103, ClaudeCodeLayout is rendered without onNewChat. In ClaudeCodeLayout, the history-panel “New Chat” path only invokes reset logic through that callback, so current behavior navigates without resetting the shared chat runtime.
Suggested fix
- return (
- <ClaudeCodeLayout activeSessionId={sessionId ?? undefined} artifacts={artifacts}>
+ return (
+ <ClaudeCodeLayout
+ activeSessionId={sessionId ?? undefined}
+ artifacts={artifacts}
+ onNewChat={startNewChat}
+ >📝 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
| <ClaudeCodeLayout activeSessionId={sessionId ?? undefined} artifacts={artifacts}> | |
| <AccessibleTitle title={`Code Agent chat for ${workspace}`}> | |
| <Stack className="h-full w-full py-density-lg"> | |
| <Stack className="min-h-0 w-full flex-1"> | |
| <AssistantRuntimeProvider runtime={runtime}> | |
| <AssistantChatThread | |
| contentClassName="mx-auto w-full max-w-180 px-density-2xl" | |
| composerContainerClassName="mx-auto w-full max-w-180 px-density-2xl" | |
| viewportClassName={CHAT_VIEWPORT_SCROLLBAR_CLASS} | |
| hideAssistantMessageActions | |
| toolCallPartComponent={ClaudeCodeToolCallPart} | |
| attributes={{ | |
| ThreadViewport: { | |
| ref: chatViewportRef, | |
| }, | |
| }} | |
| placeholder="Ask Claude Code to work in this workspace" | |
| onReset={handleChatReset} | |
| showRunningIndicator={!decisionRequest && !inputRequest} | |
| messageContentProps={{ markdownLinkComponent: ClaudeCodeStudioLink }} | |
| emptyState={{ | |
| slotHeading: 'Start a Claude Code session', | |
| slotSubheading: 'Ask Claude Code to work in this workspace.', | |
| }} | |
| composerOverride={ | |
| decisionRequest ? ( | |
| <AgentDecisionInput | |
| request={decisionRequest} | |
| choices={decisionChoices} | |
| defaultChoiceId={decisionChoices[0]?.id} | |
| status={decisionStatus} | |
| onSubmit={resolveDecisionRequest} | |
| onSkip={skipDecisionRequest} | |
| /> | |
| ) : inputRequest ? ( | |
| <BlockingInputComposer | |
| inputRequest={inputRequest} | |
| inputStatus={inputStatus} | |
| workspace={workspace} | |
| onSubmit={handleInputSubmit} | |
| onSkip={skipInputRequest} | |
| /> | |
| ) : undefined | |
| } | |
| /> | |
| </AssistantRuntimeProvider> | |
| <ClaudeCodeChatThread chat={chat} onReset={handleChatReset} /> | |
| <ClaudeCodeLayout | |
| activeSessionId={sessionId ?? undefined} | |
| artifacts={artifacts} | |
| onNewChat={startNewChat} | |
| > | |
| <AccessibleTitle title={`Code Agent chat for ${workspace}`}> | |
| <Stack className="h-full w-full py-density-lg"> | |
| <Stack className="min-h-0 w-full flex-1"> | |
| <ClaudeCodeChatThread chat={chat} onReset={handleChatReset} /> |
🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/index.tsx` around
lines 103 - 107, The ClaudeCodeLayout component at line 103 is missing the
onNewChat callback prop, which prevents the "New Chat" button in the history
panel from resetting the chat state. Add the onNewChat prop to the
ClaudeCodeLayout component and wire it to the appropriate reset handler (likely
startNewChat or handleChatReset) so that clicking "New Chat" properly resets the
shared chat runtime instead of only navigating.
Comment on lines
+565
to
+576
| const loadSession = useCallback( | ||
| ({ artifacts: nextArtifacts, messages, sessionId: nextSessionId }: LoadClaudeCodeSessionOptions) => { | ||
| sessionIdRef.current = nextSessionId; | ||
| setSessionId(nextSessionId); | ||
| onSessionIdChange?.(nextSessionId); | ||
| setArtifacts(createWorkspaceArtifacts(nextArtifacts, workspace)); | ||
| clearPermissionRequest(); | ||
| clearInputRequest(); | ||
| replaceMessages(messages); | ||
| }, | ||
| [clearInputRequest, clearPermissionRequest, onSessionIdChange, replaceMessages, workspace] | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cancel active runs before loadSession hydration.
Lines 565-574 hydrate sessionId/messages/artifacts without stopping an in-flight stream. If a user loads history while running, old-stream events can append into the newly loaded chat and corrupt session state.
Suggested fix
const loadSession = useCallback(
({ artifacts: nextArtifacts, messages, sessionId: nextSessionId }: LoadClaudeCodeSessionOptions) => {
+ // Abort any in-flight run before hydrating a different session.
+ resetThread();
sessionIdRef.current = nextSessionId;
setSessionId(nextSessionId);
onSessionIdChange?.(nextSessionId);
setArtifacts(createWorkspaceArtifacts(nextArtifacts, workspace));
clearPermissionRequest();
clearInputRequest();
replaceMessages(messages);
},
- [clearInputRequest, clearPermissionRequest, onSessionIdChange, replaceMessages, workspace]
+ [
+ clearInputRequest,
+ clearPermissionRequest,
+ onSessionIdChange,
+ replaceMessages,
+ resetThread,
+ workspace,
+ ]
);🤖 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
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts`
around lines 565 - 576, The `loadSession` callback function does not cancel any
active/in-flight runs before loading a new session. This allows old stream
events to corrupt the newly loaded session state if a user loads history while a
run is still executing. Add a call to cancel active runs (such as a `cancelRun`
or similar method that stops in-flight streams) at the very beginning of the
`loadSession` callback body, before any state updates like `setSessionId`,
`setArtifacts`, or `replaceMessages` are called.
Comment on lines
+50
to
+52
| {CODING_AGENT_STUDIO_ENABLED && workspace ? ( | ||
| <ClaudeCodeChatProvider workspace={workspace}>{layout}</ClaudeCodeChatProvider> | ||
| ) : ( |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Key ClaudeCodeChatProvider by workspace to prevent cross-workspace state bleed.
Lines 50-52 reuse one provider instance as workspace changes. That can carry session/runtime state across workspace boundaries.
Suggested fix
{CODING_AGENT_STUDIO_ENABLED && workspace ? (
- <ClaudeCodeChatProvider workspace={workspace}>{layout}</ClaudeCodeChatProvider>
+ <ClaudeCodeChatProvider key={workspace} workspace={workspace}>
+ {layout}
+ </ClaudeCodeChatProvider>
) : (
layout
)}📝 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
| {CODING_AGENT_STUDIO_ENABLED && workspace ? ( | |
| <ClaudeCodeChatProvider workspace={workspace}>{layout}</ClaudeCodeChatProvider> | |
| ) : ( | |
| {CODING_AGENT_STUDIO_ENABLED && workspace ? ( | |
| <ClaudeCodeChatProvider key={workspace} workspace={workspace}> | |
| {layout} | |
| </ClaudeCodeChatProvider> | |
| ) : ( |
🤖 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 `@web/packages/studio/src/routes/PageLayout/index.tsx` around lines 50 - 52,
The ClaudeCodeChatProvider component is being reused when the workspace changes,
which allows internal state to persist across workspace boundaries. Add a key
prop to the ClaudeCodeChatProvider component that uniquely identifies each
workspace (such as workspace.id or workspace.name) to force React to destroy and
recreate the provider instance whenever the workspace value changes, preventing
state from bleeding between workspaces.
Contributor
|
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 by CodeRabbit
Release Notes