Repository navigation
feat(native-chat): edit a queued message's images on desktop: remove, paste, drop or attach - #30815
Conversation
…st, wire and capability)
… attach, paste or drop
…he rarely-shown Save reasons
|
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
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
… and its wire takes a full image set An expired upload refused on Save read "The command didn't run": the queued message update fell through to the command write kind. It now has its own kind whose sentence is "Your edit wasn't saved." The images param caps at MAX_BLOCKS, matching the shared edit rule that counts them with the text.
…ops and pastes on its card - Removing a kept image drops its [Image #k] marker from the edit's text and renumbers the later ones (shared rule, reused by the phone), so the agent never reads a marker that names the wrong image. - A file dragged from Orca's own tree onto the editing card goes to the edit, like a Finder/Explorer drop; the chat box overlay no longer covers the card. - Cmd/Ctrl+V with focus on the card's buttons goes to the edit, not the chat box. - A "changed" answer says "Your edit is in the chat box" only when something went there; otherwise it says the edit wasn't saved. - The empty hint counts an uploading image as content and is not shown for agents without image input; the list grows only while thumbnails show. - Every hand-off releases the edit's preview URLs.
… chat box's owner check The edit no longer runs its own copy of the in-app drop check, which resolved the target host through a path that can read the Active Server setting. The chat box's claim checks the drop and routes it by the chat's owner as before, then hands the files to the editing card's sink when the drop landed on it. Also: the kept-image id list is readonly where it is read, and the marker rule looks up the remaining images in a Set.
…tes, and no drop joins a Save in flight The pane paste bridge no longer skips every paste inside a queued editor: the card's own paste handler already claims image and file pastes, which the bridge respects. A menu paste is left to the card only when the edit takes images (the card is a drop owner), so an older host or an agent without image input behaves as before, and text pasted with focus on the card's buttons goes to the chat box. An in-app drop handed to the card while its Save is in flight adds nothing.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
3653a1a6-0bcc-4476-be3c-002ccb9810f0
📒 Files selected for processing: 26
src/renderer/src/components/native-chat/NativeChatQueuedMessageCard.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageEditor.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageEditor.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.tsxsrc/renderer/src/components/native-chat/agent-session-write-notice-text.tssrc/renderer/src/components/native-chat/native-chat-pane-file-drop.test.tsxsrc/renderer/src/components/native-chat/native-chat-pane-file-drop.tssrc/renderer/src/components/native-chat/queued-message-edit-notices.tssrc/renderer/src/components/native-chat/structured-agent-session-queued-edit-images.tssrc/renderer/src/components/native-chat/use-native-chat-paste-bridge.tssrc/renderer/src/components/native-chat/use-native-chat-queued-edit-image-intake.tssrc/renderer/src/components/native-chat/use-native-chat-workspace-file-drop.tssrc/renderer/src/components/native-chat/use-structured-agent-session-queued-edit.tssrc/renderer/src/i18n/en-runtime-required.jsonsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/shared/agent-session-refusal-notice.test.tssrc/shared/agent-session-refusal-notice.tssrc/shared/agent-session-write-failure.tssrc/shared/agent-session-write-notice-copy.tssrc/shared/queued-message-edit.tssrc/shared/rpc-contract/structured-agent-session-queued-edit-params.ts
🚧 Files skipped from review as they are similar to previous changes: 7
- src/renderer/src/components/native-chat/NativeChatQueuedMessageCard.tsx
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/i18n/locales/fr.json
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/zh.json
- src/renderer/src/i18n/locales/ja.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
…answer, and an edit takes no files before its lease A re-check after regained contact that failed wrote "unknown" over a known answer for the same host, so features the host had reported (such as editing a queued message's images) vanished on a connection blip, mid-edit. A failed check now keeps the last answer; a different host still starts at unknown. Paste, Attach and OS or in-app drops into a queued edit are now blocked while it acquires its lease, as typing already is.
There was a problem hiding this comment.
🔇 Additional comments: 3
src/renderer/src/runtime/structured-agent-session-host-capability.test.tsx: 1
93-111: LGTM!src/renderer/src/components/native-chat/use-native-chat-queued-edit-image-intake.ts: 1
109-109: 🎯 Functional Correctness
captureExternalDropretains the destination, but the retained handler checks the livedisabledRef.currentbefore processing. Local uploads recheck it before attachment, and runtime-session uploads pass the same live predicate toattachNativeChatSessionAttachmentPaths, which drops the upload when it becomes abandoned. A stalebusyclosure cannot append images through this path.src/renderer/src/runtime/structured-agent-session-host-capability.ts-59-61: 1
59-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Keep failed-probe answers separate for each capability.
If a caller changes
capabilitywhile keeping the same host, a failed probe retains the previous capability's answer. For example,supportedfor queued-message editing can become a falsesupportedresult for image editing. Store the capability with the answer, and retain an answer only when both the host key and capability match.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6dd22c54-bbf5-47c3-ab75-04fb00de2421
📒 Files selected for processing: 3
src/renderer/src/components/native-chat/use-native-chat-queued-edit-image-intake.tssrc/renderer/src/runtime/structured-agent-session-host-capability.test.tsxsrc/renderer/src/runtime/structured-agent-session-host-capability.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
…he host stops reading as taking images Reverts the shared capability hook to keep main's behavior: a failed re-check reads as unknown. Instead the queued edit derives from live state that an edit whose image set changed can't be saved while the host doesn't read as taking images, so a connection blip never saves renumbered markers over the old images or drops added ones. Save comes back once the host answers again.
…dit-images # Conflicts: # src/renderer/src/components/native-chat/use-structured-agent-session-queued-edit.test.tsx # src/renderer/src/components/native-chat/use-structured-agent-session-queued-edit.ts # src/shared/native-chat-image-transcript-markers.test.ts
…hs it didn't take
Review loop status: done on f9f9914The problem. A message sent while the agent is busy waits in a queue, and since #28655 it can carry images. Editing it in place (#26886) changed only its text, so a wrong screenshot could only be fixed by deleting the message and sending it again, which loses its place in the queue. What changes for the user (desktop). While editing a queued message you can remove its images and add new ones by paste, drop (Finder/Explorer or Orca's file tree) or Attach. Save keeps the message in its place. On Save, added images get Fixed during review (8 rounds; Claude reviewers in rounds 1-5, Codex reviewers and fixers from round 6; two design-panel rulings):
Deferred (known limits, in the PR body):
Verified:
Not verified:
For Brennan's reviewBranch screenshots. Main-vs-branch comparisons are in the QA reports; on main, none of these flows exists.
Three places where a human judgment matters:
|




ELI5
A queued follow-up message can hold images, but editing it in place only let you change its text. Now, on desktop, editing a queued message also lets you remove its images and add new ones by pasting, dropping or attaching, and Save keeps everything together in the same spot in the queue.
What Changed
Before: since #28655 a queued message can hold images, and since #26886 you can edit a queued message in place. But the edit only changed the text: the editor showed "N images, which saving the text keeps", with no way to remove a wrong screenshot or add another. The only fix was to delete the message and send it again, which loses its place in the queue.
After, on desktop:
[Image #N]references in the text (the chat box writes these since feat(native-chat): mark each attached image as [Image #N] where it sits in the prompt #28736): the removed image's reference goes and later ones renumber.[Image #N]reference for each added image, numbered in the order the images are saved, so the saved message matches what the agent receives. Adding and then removing an image leaves the text exactly as it was.Mechanism:
agentSession.queuedMessageUpdatetakes a new optionalimagesfield: the complete ordered set of images the message should hold. Absent means "keep the images" (what every older client sends). The edit stays a compare-and-set on the body's fingerprint, so a resent Save whose answer was lost reads as unchanged, never as a double add.queuedMessageWithEditinsrc/shared/queued-message-edit.ts) builds the edited body for the client's Save button and the host's decision alike, with the same block and size limits as a fresh send.Known limits (intended):
[Image #N]references appear when you Save, not while you type (the chat box shows them as you attach). The thumbnails show what you've added in the meantime.Mixed versions: a new capability,
agent-session.queued-message-image-edit.v1, gates the image editing. An older host gets exactly today's text-only editor, and an older client never sendsimages, so the host keeps the images as today. The phone is unchanged in this PR: it still doesn't advertise queued image messages, so it never sees them, and its text edits keep images. The phone part follows in a separate PR. Until it lands, a text-only message that gains an image on desktop leaves the phone's queue view (the host shows image messages only to clients that can display them), and a phone editing that message at that moment loses its unsaved edit. It reappears once the phone PR ships.Why
A queued message's images should be editable (remove, replace, add by paste) the same way its text is. The common pattern sends the message's full new content on Save rather than add/remove steps, keeps removals local until Save, uploads images when they're added, drops references to removed images from the text, and claims new uploads in the same write as the edit; this does the same. Intended differences from the common pattern: the editor stays inline in the queued message instead of taking over the chat box, because Orca's queued edit must keep working while a question or approval holds the chat box; the one-line hint is Orca-only, because the inline editor is a plain field with no other cue that it takes images.
Linked Issue
Maintainer PR; follow-up to #28655 and #26886.
Visual Proof
Desktop QA with real agents, run against main for comparison. Head f9f9914 on the M4Air (macOS) unless noted.
[Image #1] [Image #2] [Image #3]and names Blue square, Green triangle, Purple diamond in order (head f9f9914, M4Air):Windows (Windows high-spec box, head f9f9914): every paste flow passed — Ctrl+V and Shift+Insert with focus on Attach, ✕, Save and Cancel (22 paste records), replacing a selected word keeps the cursor, and paste outside the message is unchanged; real Claude and real Codex each named the edited images in order. Report kept on the QA machine (no screenshots copied).
Testing
[Image #N]references renumber, paste a new one with focus on Attach, Save sends the new set) and the older-host editor (today's count, no images sent).Review
Author: Brennan Benson (@BrennanKB5)