Repository navigation
fix(native-chat): an unsent draft survives a reload or quit - #24905
Conversation
The composer's draft text, its editor document and settled image refs lived only in module memory, so a reload, quit or crash lost whatever was typed, including text a Stop had just given back. They are now saved per pane scope in localStorage: typing is debounced and flushed when the window hides or closes, text given back to the composer is saved at once, an emptied or sent draft is removed at once, and only the newest drafts within the composer caches' existing bound are kept.
The draft's text and its image chips were saved by two writers that each rebuilt the stored record from what storage held, so a draft that once went over the size cap, or one refused write, lost its images on the next save. One store now holds each composer scope's whole draft in memory and writes storage from it: typing is coalesced, returned text, chips and clears land at once, a refused write stays pending for the next flush, and all drafts together keep to a budget so the outbox can still save a send. Closing a tab drops its panes' drafts.
…ss reloads A message given back from the outbox could be larger than one draft was allowed to store, so the saved draft was deleted and the returned text was gone after a reload. One draft may now use the whole draft budget, which holds the largest message a send accepts. An adopted launch link is also parked in the agent's input line. Restoring it after a reload, with no seed left to replace that copy, typed it twice, so the untouched copy is shown but not saved. A composer reused for another pane now shows that pane's images, so adding one never saves the previous pane's over them.
…worktree A pasted image lives in a temp file Orca wrote, and the permission to show it is held in memory only, so after a restart a restored pasted chip showed a generic icon, and once the temp file was cleaned it looked fine but the send failed after the composer cleared. Pasted images now stay in the draft for the current run only; images the user attached from their own files are still saved. Removing a worktree now deletes its terminal tabs' drafts, as closing the tab does. A paste that finished late in a composer that had already been replaced wrote that composer's old images back over a newer send. Dropping a placeholder chip no longer writes the saved draft. The pane-change reload in the images hook is removed: the composer is keyed by pane, so a pane change always mounts a new one.
…ping a copy A composer that had been replaced, for example by a question prompt, could still finish an attach or a send later and write its own old copy of the draft over the store, so a sent message or image came back. The composer now reads text and images from the draft store and changes the draft as it is now: an added image or file reference joins what is there, and a send that settles after a round trip, or after its composer was replaced, clears the draft only if it is still what was sent. Only chips still being written and an IME composition in progress stay local to the composer. Closing a tab now deletes drafts by the exact tab id: a second chat for one session has an id that extends the first one's, so a prefix match deleted its drafts too.
…mposers An image pasted while /compact or /clear was still on its way to the host vanished when the command was accepted: the clear that followed also wiped the chip still being saved. A command that settles now clears only the draft it sent, and an image pasted meanwhile stays. A composer on screen could lose its pasted images, and its unsaved launch text, once 128 other drafts pushed its record out of memory, because those are never stored. A draft that a composer is showing is no longer evicted.
A write to a pane while every other cached draft was on screen could evict the record it had just written, losing it. Also adds the test that a send settling after its composer unmounted clears the saved draft, through the real send hook.
…fts-survive-reload # Conflicts: # src/renderer/src/store/slices/tabs/tabs-close-actions.ts
Review summaryThe problem. The native chat composer kept an unsent draft (text, skill and file chips, attached images) only in memory. A window reload, quit, update restart or crash threw it away. That included text Orca itself puts back in the composer: Stop returns not-yet-sent messages there, and editing a queued message moves its text there. That text existed nowhere else, so a reload before you re-sent it lost the message for good. What changes for you.
What the review changed. The first version saved text and images through two separate writers, each patching what storage held. The review proved this could silently drop a draft's images after a reload. It also found:
The change now has one store that owns each composer's whole draft. Storage is always written whole from it, and the composer reads straight from the store. Every change applies to the draft as it is now. A send clears the draft only if it still equals what was sent. Total size is bounded so a send can always be saved. Drafts are deleted when you close their tab or remove their worktree in Orca. Pasted images are not saved (see below). These are the same mechanisms the common pattern uses, apart from the labelled differences in the PR body. Fixed in review (all with tests that fail without the fix):
Deferred (each labelled in the PR body):
What was verified.
Not verified.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (33)
🚧 Files skipped from review as they are similar to previous changes (6)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds persistent, scoped native-chat drafts with IndexedDB storage, recovery journaling, hydration, and cross-window updates. It adds application-data storage and restoration checks for pasted images, plus unavailable-image indicators and send blocking. Composer scopes and send handling now preserve draft content that changes while a send is in progress. Tab and workspace teardown capture and remove drafts for the affected scopes. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Draft persistence is broadly in place. A few known follow-ups remain open: a small memory leak from image previews, Windows test fragility around symlinks, and draft saves that can keep failing for the rest of a session after another window upgrades or deletes the database. These are bounded and suitable for follow-up, but the owner should be aware of them before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Image recovery preserves existing access checks and limits restoration to application-owned files. Saved drafts introduce a data-retention obligation: failed cleanup followed by an abrupt exit can leave previously cleared content in storage. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 48.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 174 functions across 80 files. (6 skipped: 6 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1ef4bdb1-2882-4c74-a27f-b34ee936ffc2
📒 Files selected for processing (21)
src/renderer/src/components/native-chat/NativeChatComposer.tsxsrc/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsxsrc/renderer/src/components/native-chat/native-chat-composer-draft-storage.tssrc/renderer/src/components/native-chat/native-chat-composer-draft-store.test.tssrc/renderer/src/components/native-chat/native-chat-composer-draft-store.tssrc/renderer/src/components/native-chat/native-chat-composer-scope-cache.test.tssrc/renderer/src/components/native-chat/native-chat-composer-scope-cache.tssrc/renderer/src/components/native-chat/native-chat-draft-cache.tssrc/renderer/src/components/native-chat/structured-session-takeover-report.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-attachments.tssrc/renderer/src/components/native-chat/use-native-chat-composer-submit.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-submit.tssrc/renderer/src/components/native-chat/use-native-chat-draft.tssrc/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.tssrc/renderer/src/components/native-chat/use-native-chat-structured-composer-send.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-structured-composer-send.tssrc/renderer/src/store/slices/native-chat-launch-draft-teardown.test.tssrc/renderer/src/store/slices/tabs/tabs-close-actions.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.tssrc/renderer/src/store/terminals/terminal-tab-close.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| // Chips still being written, and the clipboard previews this composer minted, are its own. | ||
| const [local, setLocal] = useState<LocalAttachments>(NO_LOCAL_ATTACHMENTS) | ||
| // Read by callbacks between renders; only they change it, always together with the state. | ||
| const localRef = useRef(local) | ||
| const updateLocal = useCallback((next: LocalAttachments) => { | ||
| localRef.current = next | ||
| setLocal(next) | ||
| }, []) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
git show HEAD~1:src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts 2>/dev/null | rg -n -C3 'revokeObjectURL|useEffect' || true
rg -n -C3 'revokeObjectURL' src/renderer/src/components/native-chatRepository: stablyai/orca
Length of output: 5863
🏁 Script executed:
#!/bin/bash
set -e
git diff --no-ext-diff --unified=35 3284b4c70c901402831bb4ccc5576ea083d2e5ae 407ce4ac30ce305e5e8b505681b263ea620577f5 -- src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts
printf '\\n--- current attachment hook ---\\n'
nl -ba src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts | sed -n '1,330p'
printf '\\n--- paste hook cleanup and async path ---\\n'
nl -ba src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts | sed -n '1,245p'
printf '\\n--- attachment hook bindings ---\\n'
rg -n -C 3 'useNativeChatComposerAttachments|useNativeChatComposerPaste|resolvePendingImageAttachment|forgetLocalAttachment|dropPendingRef' src/renderer/src/components/native-chatRepository: stablyai/orca
Length of output: 42783
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\\n' '--- paste hook lifecycle and resolution ---'
nl -ba src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts | sed -n '47,160p'
printf '%s\\n' '--- attachment hook call sites ---'
rg -n -C 5 'useNativeChatComposerAttachments\\(' src/renderer/srcRepository: stablyai/orca
Length of output: 5183
🏁 Script executed:
#!/bin/bash
printf '%s\\n' '--- async paste continuation ---'
nl -ba src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts | sed -n '157,245p'
printf '%s\\n' '--- attachment hook callers ---'
rg -F -n -C 5 'useNativeChatComposerAttachments(' src/renderer/src || trueRepository: stablyai/orca
Length of output: 13378
🏁 Script executed:
#!/bin/bash
set -e
rg -l -F '<NativeChatComposer' src/renderer/src --glob '*.tsx' | while IFS= read -r file; do
printf '\\n--- %s ---\\n' "$file"
rg -n -C 18 -F '<NativeChatComposer' "$file"
doneRepository: stablyai/orca
Length of output: 42454
Release composer-owned blob previews on unmount.
When a clipboard paste settles, resolvePendingImageAttachment moves its preview URL into local.previews. The paste hook removes that ID from its pending set before resolving it, so its cleanup cannot revoke the settled URL. When a question prompt replaces the composer, the preview can remain alive without an owner. Add cleanup for both local collections.
♻️ Suggested fix
const updateLocal = useCallback((next: LocalAttachments) => {
localRef.current = next
setLocal(next)
}, [])
+ useEffect(() => {
+ return () => {
+ const current = localRef.current
+ current.pending.forEach((attachment) => releasePreviewUrl(attachment.previewUrl))
+ current.previews.forEach(releasePreviewUrl)
+ localRef.current = NO_LOCAL_ATTACHMENTS
+ }
+ }, [])📝 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.
| // Chips still being written, and the clipboard previews this composer minted, are its own. | |
| const [local, setLocal] = useState<LocalAttachments>(NO_LOCAL_ATTACHMENTS) | |
| // Read by callbacks between renders; only they change it, always together with the state. | |
| const localRef = useRef(local) | |
| const updateLocal = useCallback((next: LocalAttachments) => { | |
| localRef.current = next | |
| setLocal(next) | |
| }, []) | |
| // Chips still being written, and the clipboard previews this composer minted, are its own. | |
| const [local, setLocal] = useState<LocalAttachments>(NO_LOCAL_ATTACHMENTS) | |
| // Read by callbacks between renders; only they change it, always together with the state. | |
| const localRef = useRef(local) | |
| const updateLocal = useCallback((next: LocalAttachments) => { | |
| localRef.current = next | |
| setLocal(next) | |
| }, []) | |
| useEffect(() => { | |
| return () => { | |
| const current = localRef.current | |
| current.pending.forEach((attachment) => releasePreviewUrl(attachment.previewUrl)) | |
| current.previews.forEach(releasePreviewUrl) | |
| localRef.current = NO_LOCAL_ATTACHMENTS | |
| } | |
| }, []) |
Source: Learnings
There was a problem hiding this comment.
ℹ️ No critical issues — one behavior worth a look inline, plus a test-name nit.
Reviewed changes
- One store owns each pane's draft.
native-chat-composer-draft-store.tskeeps the text, editor document, and image refs per scope in memory and writes one localStorage record whole, with a 250 ms debounce and a flush onpagehide/beforeunload/hiddenvisibilitychange. - Storage and bounds.
native-chat-composer-draft-storage.tscaps drafts at 128 records / 1,000,000 chars (oldest first), drops the editor document before the text, and excludes pasted images and unsaved launch text. - Hooks re-plumbed.
use-native-chat-draftreads viauseSyncExternalStorewith an IME composition hold; attachments render settled images from the store plus local pending/preview chips. - Sends clear by scope. The structured and goal send hooks take
draftScopeKeyand clear throughclearNativeChatComposerDraftIfUnchanged, with amountedref for a send that settles after its composer unmounted. - Teardown deletes drafts by tab. Terminal user close, structured tab close, and worktree removal each delete that tab's drafts;
setBoundedScopeCacheEntrygained eviction hooks that skip in-use entries.
ℹ️ Nitpicks
native-chat-composer-draft-lifecycle.test.tsx:177— the first test is named "clears the saved draft when a send settles after the composer unmounted", but it never callssend; it directly callsheld.setDraft('')/held.attachments.clearImageAttachments(). The next test (:217) is the one that actually exercises a settling send. Rename the first to describe the replaced-composer clear it tests.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| sent: NativeChatComposerDraft | ||
| ): boolean { | ||
| const current = readNativeChatComposerDraft(scopeKey) | ||
| if (current.text !== sent.text || !sameImages(current.images, sent.images)) { |
There was a problem hiding this comment.
This all-or-nothing check means that if an image pasted during a host command's round trip finishes saving before the command settles, images no longer matches and the sent command text is neither cleared nor prevented from persisting. Because this PR now persists drafts, an accepted /compact or /clear whose text was stranded this way reappears in the composer after a reload, inviting a duplicate invocation.
Technical details
# Host-command text can survive the clear and resurrect on reload
## Affected sites
- `src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts:264-274` — `clearNativeChatComposerDraftIfUnchanged` returns `false` when either text or images differ, clearing neither.
- `src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts:84-89` — the host-command branch clears only through this helper, so the `false` path leaves the text to be persisted.
- `src/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsx:271-300` — the only test of this path settles the command *before* the image resolves, so the resolved-first ordering is unexercised.
## Required outcome
- A host command (`/compact`, `/clear`, `/goal`) whose text is unchanged since the send must have that text cleared even when the user attached an image during the round trip; only the image added since the send should remain. Today the text alone persists, and the PR's own test at `:271` asserts the intended outcome (`draft` empty, image kept) for the pending ordering only.
## Suggested approach (optional)
- Compare the two fields independently: clear the text whenever `current.text === sent.text`, and remove just the images present in `sent` (for host commands, none) from `current.images` rather than bailing out on any image difference. Keep the `!mounted.current` case conservative if the changed-text behavior there is deliberate.
## Open questions for the human
- The PR notes the in-session "command text stays too" matches main; is the durable resurrection after reload acceptable, or should the clear split text and images?…ach again A pasted screenshot in an unsent draft vanished after a restart with no notice, and a restored image whose file had been deleted looked fine until the send failed. Both now come back as a placeholder chip that names the image and says it wasn't saved with the draft. Send stays disabled, and the send button says to attach the image again or remove it. Attaching a file with the same name takes the placeholder's place. Two browser tabs of the web client on one chat now follow each other: when one sends or edits the draft, the other drops its copy and shows the new one, unless it has an edit of its own not yet saved.
A pasted screenshot lived in the OS temp folder with a read permission held in memory, so a draft could only bring it back as a placeholder. Local pastes are now written to Orca's own native-chat-pastes folder under the app's user data, and a draft keeps them as real images. On restore the composer asks the main process to re-grant each one; main grants a read only when the file's real path, symlinks and junctions resolved, is inside that folder, and reports anything else as not kept, which becomes the placeholder. Pastes older than 30 days are deleted at startup, far past the host's 24 hour window for a resent message; the sweep never follows a link and never blocks startup. SSH pastes and older temp-folder pastes still come back as placeholders.
… attach replaces one A placeholder now shows the image's name and a short hint on the chip itself, "Attach again" for a file and "Not kept" for a pasted image, with the longer explanation in its tooltip. The copy no longer says the image wasn't saved: it says it couldn't be brought back, and for a pasted image it asks only for removal, since a new paste can't match it. The send button now says to remove the image to send. Only an image the user attaches (picking, dropping or pasting) takes the place of a placeholder with its file name. An image Stop gives back is added beside it, so a different file with the same name no longer hides the reminder.
…ore grants only real paths Every local clipboard image save had moved into the paste folder, whose macOS path has a space, so a screenshot pasted into a terminal or sent to a terminal-backed agent no longer attached. Only a native-chat composer paste goes there now, through an optional field on the existing save call; terminal, editor and phone pastes stay in the system temp folder as before. An image path sent to an agent's input is escaped the way a dropped image is. The restore re-grant now grants only the file's real path, and only when both the real path and the path as written are inside the paste folder, which must not itself be a link. The sweep deletes only Orca's paste files and skips a linked folder.
…stored Restore granted only a paste's real path, but the composer reads the preview by the path its draft stored, so with user data reached through a link the restored chip showed a generic icon. The stored spelling is granted too; it is already proven to sit inside the paste folder. A test also pins that a composer paste asks for the paste folder.
…the same file A grant also covers its path's own real path, so a stored spelling that is itself a link inside the paste folder could have granted an outside file while the path as restored reached a real paste. The stored spelling is now granted only when it resolves to the same file; otherwise only the real path is, and the preview falls back to the generic icon.
Follow-up: images in a restored draftThe problem. A user-side check of the earlier head found that a screenshot pasted into an unsent draft silently disappeared after a restart, while its text came back. The text could then refer to an image the agent never got. A restored image whose file had since been deleted looked normal, but sending it failed after the composer had already cleared. In the web client, a second browser tab kept a stale copy of a draft another tab had already sent. What changes for you.
Fixed in review of this round:
Deferred (labelled in the PR body):
Verified.
Not verified:
|
…nsaction's complete, and false on abort, for an append before the load
… only what preceded it - A removal is never capped in the journal (it is tiny, and it is what keeps a sent draft from coming back); only whole drafts count against their 256,000 characters, and additions keep their own 800,000. - A load replays the journal as it stood when the load began reading, so an addition a live window journals later, whose own write is ordered after that read, isn't replayed over its newer typing. - A lifecycle test no longer says the paste is re-granted.
#24917's empty-chat check read the composer draft under the pane key, while this branch keeps a structured chat's draft under its conversation key, so a chat holding typed or given-back text, or only an image, read as empty: a new chat could reuse it and another request's text could go into it. It reads the conversation's draft now, as C2 adapted it (d4e469b), with the tests moved to that key. While the startup load of saved drafts is still running, no chat reads as empty: one may hold a saved draft memory doesn't have yet. The cost is only that a new chat opens instead of reusing one during that moment.
…fts-survive-reload
Follow-up: up to date with main, given-back text is safe through a crash, and reuse checks the right draftMain moved twice under this PR.
What this means for you.
Fixed in review this round:
Deferred (labelled in the PR body):
Verified.
Not verified:
|
There was a problem hiding this comment.
Important
A previous run's journaled whole draft is silently lost when the next run appends to the same conversation before its startup load begins reading. The append-before-load path writes a partial record with a fresh savedAt over a storage base that never absorbed the journal entry, so the entry is pruned and replayed as stale. Reproduced with a focused test (below).
Reviewed changes
Delta against the prior pullfrog review (393eb7f), the six commits since:
- Additions are durable at append time. A new
native-chat-composer-draft-addition.tsand a second journal entry kind letappendNativeChatComposerDraftjournal text/images given back the moment they are added; it returns whether the addition was durable, and a queued card's Edit deletes only once storage confirms the scope's write. - Per-scope write confirm.
native-chat-composer-draft-persistence.tstracks writes per scope (scopeWrites), sonativeChatComposerDraftWriteSettledincludes an append made before the load landed and returns false when storage refused. - Journal caps split. Whole drafts stop at 256k, additions at 800k, and removals are never capped; a load snapshots the journal when it begins reading, so a live window's later addition is not replayed over its newer typing.
- Paste restore rule.
restoreNativeChatPasteskeeps a paste by its real path only (the stored-spellingsameFilecheck is gone); the preview read stays safe because chat-image access reads only image files. - Empty-chat reuse.
isStructuredLaunchChatEmptynow reads the conversation's draft and treats every chat as non-empty while saved drafts are still loading, so a draft-holding chat is never reused or taken over.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| continue | ||
| } | ||
| const stored = drafts.get(entry.scopeKey) | ||
| if (stored && stored.savedAt >= entry.at) { |
There was a problem hiding this comment.
The previous run's journaled whole draft can be dropped here. When the next run appends to the same scope before its startup load begins reading, appendBeforeLoad read-modify-writes a partial record onto storage (its base never absorbed the journaled draft) and stamps a fresh savedAt; this comparison then treats that partial record as superseding the whole-draft journal entry and skips it, and withAppends will not re-apply the append (sequence == readAtSequence), so the earlier text is gone.
Technical details
# A journaled whole draft is lost when the next run appends before its load starts
## Affected sites
- `native-chat-composer-draft-journal.ts:127` — `stored.savedAt >= entry.at` treats the append-before-load partial record as newer than the whole-draft journal entry, so the entry is dropped and its text lost.
- `native-chat-composer-draft-persistence.ts:175,188` — `appendedAt` is the new record's `savedAt`, so `pruneNativeChatComposerDraftJournal(scopeKey, appendedAt)` also removes the older whole-draft entry from localStorage (it intended to remove only this run's addition entry).
- `native-chat-composer-draft-load.ts:45` — `!entry.committed || entry.sequence > readAtSequence` skips re-applying the append when it was made before `readAtSequence` was captured, even though the loaded draft never contained the journaled draft.
## Required outcome
- A run that starts with a journaled (unconfirmed) whole draft and appends to the same scope before its load lands must end with the journaled text plus the addition, not just the addition.
- Add a test: run A types a draft whose write never commits and quits (journal holds it); run B appends to the same `agent-session:<id>` before `hydrateNativeChatComposerDrafts` begins reading, then loads. Expect `"<run A text>\n\n<append>"`. A variant where the append happens after the load starts already passes, so the ordering is the whole difference.
## Suggested approach (optional)
- Make the append-before-load write base on the journaled draft for the scope (merge the pending whole-draft entry) rather than on storage alone, and/or keep whole-draft journal entries whose `at` predates an append-before-load write; then re-apply the append on the replayed draft instead of relying on `sequence > readAtSequence`.| get(), | ||
| getKnownRepoWorktreeIds(get(), projectId, ownerHostId).map((workspaceId) => ({ | ||
| workspaceId, | ||
| executionHostId: ownerHostId |
There was a problem hiding this comment.
Minor: ownerHostId here is getRepoExecutionHostId(ownerRepo), which is local for an unstamped repo, while resolveNativeChatDraftOwner stamps the draft from resolveWorktreeOperationRoute — runtime:<env> for an unstamped worktree with a single focused runtime. The owner match then fails and a closed conversation's draft is left behind. Same-id-across-hosts and SSH-through-a-HUB are documented as temporary; this write-vs-removal route mismatch is not.
Main now carries #24917 as landed; C2 already holds that version, so the files only main changed keep C2's resolution, and #24917's in-memory notes hold (new-agent-prompt-outcome, the outbox entry watch) stays deleted: the outbox-derived hold is the only one. From #24905: the per-scope write confirm, data additions that report whether they are durable (journaled at once), the load-pending guard on empty chats, and its paste wording and chat-image test. C2 moves onto them: - A hand-back finishes at once when its addition is durable, and otherwise when the draft's own write is confirmed (nativeChatComposerDraftWriteSettled), re-checking a refused one on the draft's next landed save. - It runs against the loaded drafts, waiting for the startup load, so the suffix rule and the image id check read what was saved (an addition made before the load is applied again to the loaded draft without them). - The suffix rule stays C2's (the end, as its own paragraph); #24905's `once` matches any text ending, so it isn't used for it. A whitespace-only draft counts as empty.
… own paragraph Replaying an addition after a crash skipped it whenever the draft merely ended with its text, so "go" given back onto "please go" was lost. It now counts only when the draft equals the text or ends with it after a blank line. A whitespace-only draft counts as empty when text is added, and an addition made before the startup load lands is made again on the loaded draft only where that draft doesn't already hold it.
…fts-survive-reload # Conflicts: # src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx # src/renderer/src/components/native-chat/native-chat-draft-cache.ts # src/renderer/src/components/native-chat/use-native-chat-draft.ts
…l that draft is saved After merging main's shared returned-text rule, a hand-back the draft already ends with changes nothing, so it wrote no backup entry yet reported durable. If the draft holding that text was not saved yet, a crash after the source copy was deleted lost it. Journal the addition whenever the draft is not confirmed saved.
…fts-survive-reload Main's new composer lines put NativeChatComposer.tsx over max-lines on the merged tree; the composer's dictation state moves into its existing dictation hook (renamed useNativeChatDictation), with no behaviour change.
…oast Main's #25668 imported from shared/worktree/removal twice in this file, which fails the type-aware lint gate (no-duplicate-imports) for every branch that merges current main.
…fts-survive-reload
…fts-survive-reload
…fts-survive-reload
…fts-survive-reload
Follow-up: up to date with main againWhat changed since the last ready head (99b7ec8).
Verified.
|
There was a problem hiding this comment.
Caution
A previous run's journaled whole draft is still silently lost when the next run appends to the same scope before its startup load begins reading. The three branch commits since the last pullfrog review do not touch this path, and it re-reproduces at 9a576cc.
Reviewed changes
Delta against the prior pullfrog review (99b7ec8) — the three branch commits since, plus merges of main:
- Returned-text dedup narrowed to whole paragraphs.
35904851d7made "already there" mean the draft equals the text or ends with it after a blank line, so"go"given back onto"please go"is added again, and a whitespace-only draft counts as empty. The merge withmainreplaced the local helpers with the sharedappendReturnedDraftText, which carries the same rule. - A no-op hand-back stays durable.
b6b576b573journals an addition even when it changes nothing, provided the draft is not confirmed saved, so a hand-back the draft already ends with survives a crash before that draft is written. - Lint fix.
3b2ed2b516merges the duplicatedshared/worktree/removalimport indelete-worktree-failure-toast.tsx.
The delta itself is correct and its tests pass (native-chat-composer-draft-addition.test.ts 5/5). The blocking issue below predates it and is unchanged, and the prior review's minor owner-route finding at repo-removal.ts:95 also remains open.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| const written = nativeChatComposerDraftStorage() | ||
| .update(scopeKey, (stored) => { | ||
| const base = parseStoredNativeChatComposerDraft(stored) ?? empty | ||
| return savedForm({ | ||
| ...append({ ...base, ...(base.owner || !owner ? {} : { owner }) }), | ||
| savedAt: nextSavedAt() | ||
| }) | ||
| }) | ||
| .then( | ||
| () => { | ||
| entry.committed = true | ||
| pruneNativeChatComposerDraftJournal(scopeKey, appendedAt) |
There was a problem hiding this comment.
The append-before-load read-modify-write bases only on storage, so it never sees a previous run's journaled whole draft; it stamps a fresh savedAt (line 182) and then pruneNativeChatComposerDraftJournal (line 188) drops that older whole-draft entry. replayNativeChatComposerDraftJournal treats the partial record as newer and skips the entry, and withAppends will not re-apply the append because it committed before readAtSequence — so the run-A text is gone. Re-reproduces at 9a576cc: run A types "typed before quit" with a write that never commits and quits; run B appends "returned by Stop" before hydrateNativeChatComposerDrafts begins reading. Expected "typed before quit\n\nreturned by Stop", got "returned by Stop". The reverse ordering (load reads before the append) passes, so the ordering is the whole difference.
Technical details
# A journaled whole draft is lost when the next run appends before its load reads
## Affected sites
- `native-chat-composer-draft-persistence.ts:177-188` — the append-before-load `update` bases on `storage` alone and prunes the journal with the new record's `savedAt`.
- `native-chat-composer-draft-journal.ts:127` — `stored.savedAt >= entry.at` drops the whole-draft entry as stale once the append's fresh `savedAt` lands in storage.
- `native-chat-composer-draft-load.ts:44-45` — `withAppends` skips re-applying the append when it committed before `readAtSequence` was captured.
## Required outcome
- A run that starts with a journaled (unconfirmed) whole draft and appends to the same scope before its load reads must end with the journaled text plus the addition, not just the addition.
## Suggested approach
- Make the append-before-load write base on the journaled draft for the scope (merge the pending whole-draft entry) rather than on storage alone, and/or keep whole-draft entries whose `at` predates an append-before-load write, then re-apply the append on the replayed draft instead of relying on `sequence > readAtSequence`.Drafts take #24905's merged store. A hand-back's text follows the shared returned-text rule (#25149) through returnNativeChatDraftText, which trims only the end and now trusts the append's durable answer; a composer is told of an append only when it changed the draft. The withdrawn-message restore test stays deleted: its one change was the rule's dedupe, which C2's returning tests already state. messagesUnsettled keeps C2's 'wait' in the moved reason-words table.

ELI5
If you type a message in a chat and don't send it, Orca now keeps it when the window reloads or Orca quits. The same goes for a message that Stop or a queued-message Edit puts back in the box. Before, that text was gone.
What Changed
The problem. The chat composer kept your unsent draft (text, its skill and file chips, and attached images) only in memory, so a reload, quit, update restart or crash threw it away.
That also hit text Orca gives back to the composer: Stop returns not-yet-sent messages there, and editing a queued message moves its text there. Its outbox entry or queued card is deleted right after, so a reload before you re-sent it lost the message for good.
Before / after, as you see it.
The mechanism.
native-chat-composer-draft-store.ts) holds each draft whole in memory: text, editor document and image references. A structured chat's draft is keyed by its conversation, so every composer showing that conversation shares it; a terminal-backed chat's is keyed by its pane.agent-session:<sessionId>for a structured chat and the pane for a terminal-backed chat./goalcommand is accepted), or while another composer of the same conversation was in use, removes only what was sent. Text typed or composed meanwhile and images attached meanwhile stay; a draft replaced or edited inside the sent text is left as it is. A goal still clears only a draft that is exactly as sent.native-chat-pastesfolder in Orca's own user data instead of the system temp folder, and the draft saves it as a real image. Terminal, editor and phone pastes still go to the system temp folder, as before. On restore, the composer asks the main process whether each paste is still kept: only when its real path (symlinks and junctions resolved) is a file inside that folder, which must not itself be a link. Anything else becomes the placeholder. Nothing is granted: the preview reads the file with main's chat-image access (fix(editor): restored tabs for files outside your projects no longer fail with Access denied #24489), which reads only image files and refuses devices and network shares, so the restore can't make any other file readable. Until main answers, the restored chip shows as still loading, so a paste that turns out not kept never flashes.Why these keys.
agent-session:<sessionId>. The session id outlives the tab, so a closed and reopened conversation finds its draft, and two panes showing one conversation share it, as the common pattern keys a draft by its conversation.Why
Keeping an unsent draft across a reload is the common pattern, and this matches its mechanism: one owner writes the whole draft, typing is saved after a short wait and flushed when the page hides or unloads, app-originated changes save at once, and an empty draft is removed.
Alternatives considered:
Differences from the common pattern
In short: the core works the way the common pattern does. Each conversation's unsent draft is saved, and comes back after a reload or restart with its text, formatting and images. Sending clears it, and it is deleted along with what owns it. The list below is long because it spells out honest edge cases, not because the design differs much. Three differences matter:
Orca-only needs
Storage
Cleanup
Intended: a structured chat's draft survives closing its tab. It is deleted when you remove its worktree, project or folder workspace in Orca, including conversations whose tabs were closed earlier. Each draft records which workspace owns it. Deletion happens only after the removal succeeds, as the common pattern clears a draft only after its delete succeeds. A listing refresh never deletes drafts, because the same refresh also runs when a server re-pairs, and deleting then would erase live drafts.
Intended: drafts have no expiry, so a few can be left behind for good. Examples:
One common design never deletes drafts at all; another deletes them only with their conversation. Expiring drafts would mean losing someone's text without notice. Leftovers are small (text and file references) and are loaded at each start.
Temporary: the owner check uses the worktree's current host route. A draft can be left behind for a same-id worktree on two hosts, an SSH repo reached through a paired server, or an older folder workspace without a recorded host. It never deletes another workspace's draft. Follow-up: record the tab's own host; not yet filed.
Images
Rare edge cases
Not this PR: returned text is added after what you've typed (already on main). The common pattern restores it only into an empty composer.
Linked Issue
N/A (maintainer). This is the draft-durability step of the native-chat failed-send follow-up to #24606.
Visual Proof
Captured on Windows in hidden, isolated app instances with a stand-in agent. Before is the base commit (cc73c8e); after is this PR at 9a7f9c9 for the table below and at the final head 4b9f91a for the images table; the conversation-keying and removal changes are shown below at their own heads.
Images after a restart (final head 4b9f91a, Windows; before this PR every row here was simply gone):
Drafts keyed by conversation, deleted with the workspace (M4Air, macOS):
Manual QA first caught that removing a workspace left its draft behind (a listing refresh could drop the tabs before the removal finished); the fix reads the draft keys before the removal and deletes them after the host confirms, shown above.
Drafts in their own storage (final head 393eb7f and 2fc2929, M4Air, macOS):
After merging main (8b6c8c5, M4Air; the later commits change only the quit backup's rules):
Final head (071c934, Windows):
Typing check on the after build: 223 characters typed fast, an insert mid-text, then an input-method composition ("nihao" committed as 你好). No lost or doubled characters, the caret stayed in place, and the same text came back after a reload. The keys and composition were sent through the browser debugging protocol, not a real OS input method.
Not captured: the "couldn't be saved yet" warning by Send (a refused save could not be forced in the app; unit tests cover it), the queued-message Edit path (still behind a feature flag), quitting from the menu (the rig closed the window or used the app's own quit), images attached from outside the workspace, two web-client tabs, and the send button's hover tooltip (its accessible label, which the tooltip shows, was checked), reopening a closed structured chat (the rig has no way to reopen one; closing keeps its saved draft, checked in storage), and one conversation shown in two panes (no UI to do that).
Testing
native-chat-composer-draft-indexeddb.test.ts(5 tests): against a stand-in database, a write and a read-modify-write settle only when their transaction completes, and a write whose transaction aborts is refused. Through the draft store, the per-scope confirm for a draft given text before the load landed stays pending until its transaction completes, then resolves true, and resolves false when the transaction aborts.native-chat-composer-draft-store.test.ts(40 tests). These simulate a reload by resetting module memory while keeping storage, an in-memory stand-in for IndexedDB behind the same interface (the IndexedDB adapter itself is thin, and is proven in the app). They cover:pagehide;native-chat-composer-draft-lifecycle.test.tsx(16 tests, real hooks):/compactis on its way stays after the command is accepted;native-chat-prompt-editor-store-value.test.tsx(4 tests, real editor): a draft that loads late is set on the editor and not saved back; a mention accepted next to a skill chip keeps the chip, also after the composer remounts; another window's send stands instead of being echoed back by the window that showed the draft; a draft that arrives later shows its skill chip from its saved document.Placeholder and not-saved UI:
NativeChatImageAttachmentPreview.test.tsxshows each placeholder chip with its name, its visible hint, its explanation and the remove button;NativeChatComposerActions.test.tsxshows the disabled send button naming what to do, and the not-saved icon by Send only after a refusal;use-native-chat-composer-submit.test.tsxrefuses to send while a placeholder is present.Keys:
native-chat-composer-draft-comparison.test.tscovers what a send leaves;NativeChatStructuredSession.test.tsxchecks that a structured chat's composer, and the Stop and queued-card hand-back, use the conversation key while drops still route by pane.native-chat-launch-draft-teardown.test.ts: a user tab close deletes only its own panes' drafts; closing a structured chat keeps its conversation's draft; a worktree removal deletes its terminal tabs' drafts and its structured chats' conversation drafts, and nothing else.workspace-chat-draft-removal.test.ts: a worktree removal deletes them even when a listing purge drops the tabs mid-removal, and also deletes the draft of a chat whose tab was closed earlier; a removal the host refuses keeps them; a listing purge alone, and re-pairing a server under the same id, keep them; removing a project or a folder workspace deletes only its own.use-native-chat-launch-draft-adoption.test.tsxwas updated for the launch-text change.use-native-chat-composer-submit.test.tsxnow drives the draft store: a goal that settles after the draft was edited leaves the edit alone.Ablations, each reverted: each protection was removed one at a time against the final code, and each made at least one of these tests fail. The one exception: removing the immediate delete of an emptied draft stays green, because the save that follows also removes an empty record.
src/main/window/native-chat-paste-files.test.ts(26 tests):\\?\and\\?\UNC\prefixes, letter case;/varis of/private/varon macOS;..) is not kept; neither that outside file nor the file a kept paste's stored spelling names becomes readable through chat-image access;clipboard-image-temp-file.test.tschecks that a terminal, editor or phone paste stays in the system temp folder and only a composer paste lands in the paste folder; the clipboard handler tests check that the new call refuses an untrusted sender;native-chat-runtime-send.test.tschecks that a spaced paste path is escaped when sent to an agent's input.Related suites: 530 test files: the ones that use the changed modules, render the app shell, the composer or the editor, or cover startup, sends, tab close, removal, purges and clipboard saves, plus every test file main changed since the last merge. 5,431 tests pass, 14 skipped. 49 files fail the same way on main itself in this checkout: 46 can't load main's new
stream-jsondependency (this machine's install predates it), and 3 editor image-paste files fail identically on a clean main checkout.Typecheck: web and node projects clean apart from main's own:
stream-jsonnot installed in this checkout, and main'srich-markdown-image-insert.test.tspassinginsertPos, which its type doesn't have.Lint: oxlint clean; the changed-code quality gate and React Doctor report 0 new findings.
Platforms: renderer-only logic on IndexedDB, a localStorage journal, a broadcast channel and window events, the same on macOS, Linux and Windows.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Merge order with fix(native-chat): start a new chat after an earlier start failed #24917: done. fix(native-chat): start a new chat after an earlier start failed #24917 is on main and merged in. Its check for whether a chat is empty (and can be reused, or take another request's text) now reads the chat's draft under its conversation key, text and images. While saved drafts are still loading at startup, no chat counts as empty, since it may hold a draft not yet loaded; a new chat opens instead. Tests: a chat whose draft holds text, or only an image, is never reused or taken over (these fail with the old per-pane key), and no chat is taken while drafts are loading.
Security: drafts are your own text and file references in the app's own storage, as the outbox already does. A restored paste is kept only when its real path is inside Orca's paste folder, checked in the main process; symlinks and junctions are resolved first, and Windows paths are compared without case and without
\\?\prefixes. No path is granted: the preview reads it as a chat image, which main checks on its own. The restore check for other images uses the existing file check, which refuses paths outside what Orca may read.Backwards compatibility: a new key that older builds ignore.
Performance: typing does no serialization. Each save writes only changed drafts, and the limits use an in-memory index built by one read per run.
Mobile: unaffected (renderer-only modules).
Author: @BrennanKB5
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)