Repository navigation
Conversation
The v2 workspace chat (WorkspaceChatInterface) read a persisted draft on mount but never saved one on unmount, so switching tabs lost whatever was typed. The deeper cause: packages/panes renders only the active Tab and unkeyed, so two same-kind panes at the same layout slot reconcile into one React instance across tab switches — the PromptInputProvider's initial state never resets, bleeding draft text between chat panes (or, with differing sessions, wiping it via the session-change clear). Fixes both by: - Porting v1's DraftSaver pattern into v2: save the textarea draft to the pane's launchConfig.draftInput on unmount (guarded by isSendingRef), clear it on send and on session change. - Keying the chat pane by ctx.pane.id in usePaneRegistry so switching between chat tabs cleanly remounts the subtree instead of reusing one instance. Scoped to chat to avoid remounting terminals/browsers, which intentionally persist.
|
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. |
📝 WalkthroughWalkthroughChat panes now persist unsent drafts across tab switches. A new ChangesChat Draft Persistence
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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 |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 2fda2df on Jun 2, 2026 8:58am UTC. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx (1)
491-517: Remount-on-switch is the right fix — confirm conversation reload is seamless.Keying the chat pane by
ctx.pane.idcorrectly fixes draft bleed and enables the unmount save, and scoping it to chat (not terminals/browsers) is sensible. The tradeoff is that every chat-tab switch fully remounts the chat subtree, re-runninguseChatDisplay/queries and resetting scroll position. With react-query caching this should be instant, but please verify there's no visible flash or scroll jump when toggling between two active chat tabs with long histories.🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx around lines 491 - 517, Keying ChatPane by ctx.pane.id fixes draft bleed but forces remounts which can cause visible flashes or scroll jumps when switching; reproduce toggling between two active chat tabs with long histories and verify useChatDisplay queries are served from react-query cache (stable cache keys) and that there is no UI flash. If you observe flashes/scroll jumps, preserve and restore scroll position inside ChatPane (capture scroll on unmount and reset on mount by sessionId or pane id) or use react-query initialData/staleTime to avoid refetch flashes; also ensure useChatDisplay and any query keys are stable so switching relies on cached data. Check renderPane, ChatPane props (workspaceId, sessionId, initialLaunchConfig) and ctx.actions.updateData to implement restoring state on remount.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx:
- Around line 30-35: The current DraftSaver component mutates refs
(textRef.current, onSaveDraftRef.current, isSendingRef.current) during render;
move those assignments into a useLayoutEffect to avoid updating ref.current in
the render phase while preserving synchronous semantics for unmount
cleanup—inside DraftSaver, replace the inline render updates with a
useLayoutEffect that reads textInput.value and sets textRef.current and
onSaveDraftRef.current, and if needed resets isSendingRef.current when
textInput.value.length === 0; alternatively ensure the unmount cleanup reads
textInput.value directly instead of relying on ref values if you prefer a
different approach.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 491-517: Keying ChatPane by ctx.pane.id fixes draft bleed but
forces remounts which can cause visible flashes or scroll jumps when switching;
reproduce toggling between two active chat tabs with long histories and verify
useChatDisplay queries are served from react-query cache (stable cache keys) and
that there is no UI flash. If you observe flashes/scroll jumps, preserve and
restore scroll position inside ChatPane (capture scroll on unmount and reset on
mount by sessionId or pane id) or use react-query initialData/staleTime to avoid
refetch flashes; also ensure useChatDisplay and any query keys are stable so
switching relies on cached data. Check renderPane, ChatPane props (workspaceId,
sessionId, initialLaunchConfig) and ctx.actions.updateData to implement
restoring state on remount.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07bf9727-1e23-44e2-8ee7-4551ed285fc0
📒 Files selected for processing (7)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/ChatPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types.ts
| // Synchronous ref updates so the unmount cleanup always reads the latest values | ||
| textRef.current = textInput.value; | ||
| onSaveDraftRef.current = onSaveDraft; | ||
| if (isSendingRef.current && textInput.value.length === 0) { | ||
| isSendingRef.current = false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does React 19 documentation discourage writing to ref.current during render, and does the React Compiler assume render functions are pure with respect to ref writes?
💡 Result:
Yes, the React 19 documentation explicitly discourages writing to (or reading from) ref.current during rendering [1]. It classifies this practice as an anti-pattern that makes component behavior unpredictable, as React does not track ref changes for rendering purposes [1]. The only recognized exception to this rule is the initialization pattern, such as if (!ref.current) ref.current = new Thing();, which is permitted during the first render [1][2]. Regarding the React Compiler, it does indeed enforce these rules. The compiler is designed to validate that components adhere to the "Rules of React," which include keeping render functions pure and avoiding side effects—such as mutating refs—during the render phase [3][4]. The compiler performs static analysis to detect violations, and it will often issue errors or warnings, such as "InvalidReact: Ref values (the current property) may not be accessed during render," when it identifies improper ref usage [5][6][7]. While early versions of the compiler faced some issues with false positives in complex cases (e.g., when ref-accessing functions were stored in objects), the React team has actively updated the compiler to improve its ref validation logic and handle these edge cases more robustly [5][8][9]. If a component violates these purity rules, the compiler will generally skip memoizing that component rather than producing potentially broken output [4].
Citations:
- 1: https://react.dev/learn/referencing-values-with-refs
- 2: https://stackoverflow.com/questions/68025789/is-it-safe-to-change-a-refs-value-during-render-instead-of-in-useeffect
- 3: [Compiler Bug]: Compiler assumes all functions returned from hooks are pure react/react#32107
- 4: https://dev.to/grimicorn/react-compiler-v10-automatic-memoization-without-the-headache-4n62
- 5: [Compiler Bug]: InvalidReact: Ref values (the
currentproperty) may not be accessed during render. False Positive react/react#32320 - 6: [Compiler]: Ref values (the
currentproperty) may not be accessed during render. (eslint) react/react#31330 - 7: [Compiler Bug]:
ref.current === undefined,!ref.currentare not allowed react/react#34278 - 8: [Compiler Bug]: Compiler doesn't catch ref access in render react/react#31406
- 9: react/react@04a7a61
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx"
# Print only the relevant region around the cited lines (about 1 screen)
sed -n '1,120p' "$FILE" | nl -baRepository: superset-sh/superset
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx'
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p'Repository: superset-sh/superset
Length of output: 107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx'
cat -n "$FILE" | sed -n '1,220p'Repository: superset-sh/superset
Length of output: 2112
Avoid mutating ref.current during render in DraftSaver
React discourages reading/writing ref.current during render (except one-time initialization) and the React Compiler assumes render is pure; violations can make behavior unpredictable and trigger compiler diagnostics/optimization skips.
In DraftSaver.tsx, textRef.current / onSaveDraftRef.current (and isSendingRef.current reset) are currently updated in the render phase, and the unmount cleanup relies on those latest ref values. If moving this work out of render, don’t switch to passive useEffect in a way that could let unmount run before the final ref sync—prefer useLayoutEffect for the ref synchronization (or have the cleanup read textInput.value directly) to preserve the “save latest draft on unmount” behavior.
🤖 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/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx
around lines 30 - 35, The current DraftSaver component mutates refs
(textRef.current, onSaveDraftRef.current, isSendingRef.current) during render;
move those assignments into a useLayoutEffect to avoid updating ref.current in
the render phase while preserving synchronous semantics for unmount
cleanup—inside DraftSaver, replace the inline render updates with a
useLayoutEffect that reads textInput.value and sets textRef.current and
onSaveDraftRef.current, and if needed resets isSendingRef.current when
textInput.value.length === 0; alternatively ensure the unmount cleanup reads
textInput.value directly instead of relying on ref values if you prefer a
different approach.
Greptile SummaryThis PR fixes two related draft-preservation bugs in the v2 workspace chat: drafts were lost on tab switch (no save-on-unmount logic), and two chat panes at the same layout slot bled state into each other (React reconciled them into one instance without a
Confidence Score: 3/5The core draft-preservation logic is sound, but the placement of The
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx | Adds isSendingRef set before async send (not after), clearDraftInStore on session change and on success, and renders DraftSaver inside PromptInputProvider — the pre-send placement of isSendingRef.current=true introduces a regression where tab-switch during an in-flight send can permanently lose user text if the send later fails |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx | New component that saves textarea draft to pane store on unmount; correctly uses refs to capture latest values in cleanup, but the isSendingRef guard could prevent saving during in-flight sends that fail after a tab switch |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx | Adds key={ctx.pane.id} to ChatPane to fix draft bleeding, and threads onSaveDraft down; the updateData spread preserves other launchConfig fields which could inadvertently re-trigger auto-launch on return |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/ChatPane.tsx | Simple prop-threading of onSaveDraft through to ChatPaneInterface; no issues |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types.ts | Adds draftInput field to ChatPaneData.launchConfig inline type; correct and minimal |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/types.ts | Adds onSaveDraft to ChatPaneInterfaceProps with clear JSDoc; no issues |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx:583-617
**Draft lost when send fails after tab switch**
`isSendingRef.current` is set to `true` before the first `await` in the try block. The new `key={ctx.pane.id}` added to `ChatPane` now unmounts the entire subtree when the user switches tabs. If the user switches tabs while a send is in-flight, `DraftSaver`'s unmount cleanup sees `isSendingRef.current === true` and skips saving the draft. If the send subsequently fails (network error, server error), the user's typed text is gone — this scenario didn't occur before the key was added because tab switches reused the same component instance rather than remounting.
The fix is to set `isSendingRef.current = true` only after the try/catch completes successfully (i.e., after the `} catch` closing brace, just before `clearDraftInStore()`). During an in-flight send `isSendingRef.current` stays `false`, so if the component unmounts, `DraftSaver` saves the current textarea contents as a recovery draft. After success, setting it `true` right before `clearDraftInStore()` still prevents saving the just-sent text on any subsequent same-render unmount.
### Issue 2 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:509-514
**Stale `launchConfig` fields preserved alongside `draftInput`**
`onSaveDraft` spreads `data.launchConfig` to build the updated config, which means fields like `initialPrompt`, `initialFiles`, or `model` captured in the render closure are written back every time a draft is saved. If `initialPrompt` was not yet consumed when the user types and switches tabs, the returned `launchConfig` will carry both `initialPrompt` and the new `draftInput`. On the next remount the auto-launch logic sees a live `initialPrompt` and may re-fire the prompt even though the user was in the middle of composing their own message.
Consider narrowing the spread to only carry fields that are safe to roundtrip, or explicitly excluding `initialPrompt`/`initialFiles`/`model` when saving a draft.
### Issue 3 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx:624
**`isSendingRef` not reset in success path**
`isSendingRef.current` is never explicitly reset to `false` after a successful send — only the `catch` block resets it. The only path back to `false` in the success case is `DraftSaver`'s render-body side-effect (`if (isSendingRef.current && textInput.value.length === 0) isSendingRef.current = false`). Adding an explicit `isSendingRef.current = false` on the line before `clearDraftInStore()` would make the success and error paths symmetric and remove the dependency on a child component's render cycle for correctness.
Reviews (1): Last reviewed commit: "fix(desktop): preserve chat drafts per-p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -598,13 +613,16 @@ export function ChatPaneInterface({ | |||
| onUserMessageSubmitted?.(content); | |||
| } | |||
| } catch (error) { | |||
| isSendingRef.current = false; | |||
| const sendErrorMessage = toSendFailureMessage(error); | |||
There was a problem hiding this comment.
isSendingRef.current is set to true before the first await in the try block. The new key={ctx.pane.id} added to ChatPane now unmounts the entire subtree when the user switches tabs. If the user switches tabs while a send is in-flight, DraftSaver's unmount cleanup sees isSendingRef.current === true and skips saving the draft. If the send subsequently fails (network error, server error), the user's typed text is gone — this scenario didn't occur before the key was added because tab switches reused the same component instance rather than remounting.
The fix is to set isSendingRef.current = true only after the try/catch completes successfully (i.e., after the } catch closing brace, just before clearDraftInStore()). During an in-flight send isSendingRef.current stays false, so if the component unmounts, DraftSaver saves the current textarea contents as a recovery draft. After success, setting it true right before clearDraftInStore() still prevents saving the just-sent text on any subsequent same-render unmount.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx
Line: 583-617
Comment:
**Draft lost when send fails after tab switch**
`isSendingRef.current` is set to `true` before the first `await` in the try block. The new `key={ctx.pane.id}` added to `ChatPane` now unmounts the entire subtree when the user switches tabs. If the user switches tabs while a send is in-flight, `DraftSaver`'s unmount cleanup sees `isSendingRef.current === true` and skips saving the draft. If the send subsequently fails (network error, server error), the user's typed text is gone — this scenario didn't occur before the key was added because tab switches reused the same component instance rather than remounting.
The fix is to set `isSendingRef.current = true` only after the try/catch completes successfully (i.e., after the `} catch` closing brace, just before `clearDraftInStore()`). During an in-flight send `isSendingRef.current` stays `false`, so if the component unmounts, `DraftSaver` saves the current textarea contents as a recovery draft. After success, setting it `true` right before `clearDraftInStore()` still prevents saving the just-sent text on any subsequent same-render unmount.
How can I resolve this? If you propose a fix, please make it concise.| onSaveDraft={(draftInput) => | ||
| ctx.actions.updateData({ | ||
| ...data, | ||
| launchConfig: { ...(data.launchConfig ?? {}), draftInput }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
launchConfig fields preserved alongside draftInput
onSaveDraft spreads data.launchConfig to build the updated config, which means fields like initialPrompt, initialFiles, or model captured in the render closure are written back every time a draft is saved. If initialPrompt was not yet consumed when the user types and switches tabs, the returned launchConfig will carry both initialPrompt and the new draftInput. On the next remount the auto-launch logic sees a live initialPrompt and may re-fire the prompt even though the user was in the middle of composing their own message.
Consider narrowing the spread to only carry fields that are safe to roundtrip, or explicitly excluding initialPrompt/initialFiles/model when saving a draft.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
Line: 509-514
Comment:
**Stale `launchConfig` fields preserved alongside `draftInput`**
`onSaveDraft` spreads `data.launchConfig` to build the updated config, which means fields like `initialPrompt`, `initialFiles`, or `model` captured in the render closure are written back every time a draft is saved. If `initialPrompt` was not yet consumed when the user types and switches tabs, the returned `launchConfig` will carry both `initialPrompt` and the new `draftInput`. On the next remount the auto-launch logic sees a live `initialPrompt` and may re-fire the prompt even though the user was in the middle of composing their own message.
Consider narrowing the spread to only carry fields that are safe to roundtrip, or explicitly excluding `initialPrompt`/`initialFiles`/`model` when saving a draft.
How can I resolve this? If you propose a fix, please make it concise.| throw new Error(sendErrorMessage); | ||
| } | ||
|
|
||
| clearDraftInStore(); |
There was a problem hiding this comment.
isSendingRef not reset in success path
isSendingRef.current is never explicitly reset to false after a successful send — only the catch block resets it. The only path back to false in the success case is DraftSaver's render-body side-effect (if (isSendingRef.current && textInput.value.length === 0) isSendingRef.current = false). Adding an explicit isSendingRef.current = false on the line before clearDraftInStore() would make the success and error paths symmetric and remove the dependency on a child component's render cycle for correctness.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/ChatPaneInterface.tsx
Line: 624
Comment:
**`isSendingRef` not reset in success path**
`isSendingRef.current` is never explicitly reset to `false` after a successful send — only the `catch` block resets it. The only path back to `false` in the success case is `DraftSaver`'s render-body side-effect (`if (isSendingRef.current && textInput.value.length === 0) isSendingRef.current = false`). Adding an explicit `isSendingRef.current = false` on the line before `clearDraftInStore()` would make the success and error paths symmetric and remove the dependency on a child component's render cycle for correctness.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx:20">
P2: Draft persistence lifecycle logic is duplicated across v1 and v2 `DraftSaver` components instead of being shared.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx:31">
P2: Mutating `ref.current` during render violates React's purity rules. `textRef.current`, `onSaveDraftRef.current`, and `isSendingRef.current` are all written to in the render body, which the React Compiler treats as a violation (it may skip memoization or emit diagnostics). Move these ref synchronizations into a `useLayoutEffect` so they still complete before any unmount cleanup fires but don't run during the render phase itself:
```tsx
useLayoutEffect(() => {
textRef.current = textInput.value;
onSaveDraftRef.current = onSaveDraft;
if (isSendingRef.current && textInput.value.length === 0) {
isSendingRef.current = false;
}
});
useLayoutEffect (not useEffect) is important here because it fires synchronously after commit but before any sibling unmount effects, ensuring the cleanup in the unmount effect always reads the latest values.
</details>
<details>
<summary>Architecture diagram</summary>
```mermaid
sequenceDiagram
participant Tab as Workspace Tab Bar
participant PaneReg as Pane Registry
participant ChatPane as ChatPane Component
participant ChatUI as ChatPaneInterface
participant DraftSaver as DraftSaver Component
participant PromptInput as PromptInputProvider
participant PaneData as Pane Data Store
Note over Tab,PaneData: Per-Pane Draft Persistence
Tab->>PaneReg: render active tab (no key)
alt First chat pane rendered
PaneReg->>ChatPane: create with key={pane.id}
ChatPane->>ChatUI: mount with onSaveDraft callback
ChatUI->>PromptInput: initialInput={launchConfig.draftInput}
ChatUI->>DraftSaver: mount inside PromptInputProvider
DraftSaver->>PromptInput: read textInput.value via controller
Note over DraftSaver: Stores text in ref for unmount cleanup
else Switch to different chat pane
Tab->>PaneReg: render different active tab
PaneReg->>ChatPane: create NEW instance (key changed)
alt Old pane unmounts
ChatUI->>DraftSaver: unmount cleanup
DraftSaver->>DraftSaver: isSendingRef.current? skip
DraftSaver->>ChatUI: onSaveDraft(textRef.current)
ChatUI->>PaneData: updateData({ draftInput })
end
alt New pane mounts
PaneReg->>ChatPane: key={newPane.id}
ChatUI->>PromptInput: initialInput={saved draftInput}
end
end
Note over ChatUI,PaneData: Save on Send & Session Change
ChatUI->>ChatUI: user sends message
ChatUI->>ChatUI: isSendingRef = true
ChatUI->>ChatUI: clearDraftInStore()
ChatUI->>PaneData: onSaveDraft(undefined)
ChatUI->>ChatUI: sessionId changes
ChatUI->>ChatUI: clearDraftInStore()
ChatUI->>PaneData: onSaveDraft(undefined)
ChatUI->>PromptInput: textInput.clear() via DraftSaver
Note over PaneData: draftInput survives tab switches in persisted pane data
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const previousSessionIdRef = useRef(sessionId); | ||
|
|
||
| // Synchronous ref updates so the unmount cleanup always reads the latest values | ||
| textRef.current = textInput.value; |
There was a problem hiding this comment.
P2: Mutating ref.current during render violates React's purity rules. textRef.current, onSaveDraftRef.current, and isSendingRef.current are all written to in the render body, which the React Compiler treats as a violation (it may skip memoization or emit diagnostics). Move these ref synchronizations into a useLayoutEffect so they still complete before any unmount cleanup fires but don't run during the render phase itself:
useLayoutEffect(() => {
textRef.current = textInput.value;
onSaveDraftRef.current = onSaveDraft;
if (isSendingRef.current && textInput.value.length === 0) {
isSendingRef.current = false;
}
});useLayoutEffect (not useEffect) is important here because it fires synchronously after commit but before any sibling unmount effects, ensuring the cleanup in the unmount effect always reads the latest values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx, line 31:
<comment>Mutating `ref.current` during render violates React's purity rules. `textRef.current`, `onSaveDraftRef.current`, and `isSendingRef.current` are all written to in the render body, which the React Compiler treats as a violation (it may skip memoization or emit diagnostics). Move these ref synchronizations into a `useLayoutEffect` so they still complete before any unmount cleanup fires but don't run during the render phase itself:
```tsx
useLayoutEffect(() => {
textRef.current = textInput.value;
onSaveDraftRef.current = onSaveDraft;
if (isSendingRef.current && textInput.value.length === 0) {
isSendingRef.current = false;
}
});
useLayoutEffect (not useEffect) is important here because it fires synchronously after commit but before any sibling unmount effects, ensuring the cleanup in the unmount effect always reads the latest values.
| * state (and latest `onSaveDraft` closure) without re-registering the effect on | ||
| * every render. | ||
| */ | ||
| export function DraftSaver({ |
There was a problem hiding this comment.
P2: Draft persistence lifecycle logic is duplicated across v1 and v2 DraftSaver components instead of being shared.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/DraftSaver/DraftSaver.tsx, line 20:
<comment>Draft persistence lifecycle logic is duplicated across v1 and v2 `DraftSaver` components instead of being shared.</comment>
<file context>
@@ -0,0 +1,52 @@
+ * state (and latest `onSaveDraft` closure) without re-registering the effect on
+ * every render.
+ */
+export function DraftSaver({
+ sessionId,
+ isSendingRef,
</file context>
Problem
Two related bugs in the v2 workspace chat:
Root cause
The v2
WorkspaceChatInterfaceread a persisted draft on mount (initialInput={initialLaunchConfig?.draftInput}) but had no save-on-unmount logic — the v1DraftSaver(PR #2631) was never ported.The deeper cause is in
packages/panes:Workspace.tsxrenders only the active tab as<Tab tab={activeTab}>with nokey. When two tabs each hold a chat pane at the same layout slot, React reconciles them into the sameChatPaneInterface→PromptInputProviderinstance instead of remounting. SinceuseState(initialInput)only runs on first mount, the editor text never resets:Because the instance is reused, unmount/remount-based persistence never engaged.
Fix
DraftSaverpattern into v2: save the textarea draft to the pane'slaunchConfig.draftInputon unmount (guarded byisSendingRefso an in-flight send isn't re-persisted), and clear it on send and on session change. Threaded a newonSaveDraftcallback throughusePaneRegistry→ChatPane→ChatPaneInterface, wired to the pane registry'supdateData.ctx.pane.idinusePaneRegistryso switching between chat tabs cleanly remounts the chat subtree instead of reusing one instance. Scoped to chat on purpose — keying<Tab>itself would remount terminals/browsers, which intentionally persist.draftInputto v2'sChatPaneData.launchConfigtype (its inline type lacked it).Testing
bun run typecheck→ passbun run lint→ passSummary by cubic
Preserves chat drafts per pane in the v2 workspace so drafts aren’t lost on tab switch or shared across chat panes. Users can switch tabs without losing what they typed.
launchConfig.draftInput, clear on send and on session change (guarded byisSendingRef).pane.idin the registry to remount the chat subtree when switching tabs, preventing state bleed across panes; scoped to chat only.draftInputtoChatPaneData.launchConfigand threadedonSaveDraftthrough the registry →ChatPane→ChatPaneInterfaceto persist drafts.Written for commit 2fda2df. Summary will update on new commits.
Summary by CodeRabbit