fix(desktop): avoid Explorer DnD image preview freeze on Windows - #66546
fix(desktop): avoid Explorer DnD image preview freeze on Windows#66546stantheman0128 wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the expensive preview IPC path. The current main path still routes an OS-dropped image through attachmentPreviewDataUrl (apps/desktop/src/app/chat/hooks/use-composer-actions.ts:598, :421), and Electron then reads and base64-encodes the full file (apps/desktop/electron/main.ts:8051-8059).
Problems
- The new generic
clear()revocation is too early for the new optimisticblob:preview. Direct send clones attachments, clears the composer, then dispatches the clone (apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts:165-170). Revoking in that clear invalidates the blob URL before the optimistic message can use it. - Queueing has the same ownership issue: it stores attachments in the queued prompt and then clears the composer (
apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts:174-179).
Suggested changes
- Keep blob URLs alive across composer clear when a submitted or queued attachment snapshot retains them; revoke them only when the final optimistic/queued consumer is discarded or replaced.
- Add direct-submit and queued-prompt lifecycle tests for that ownership handoff.
Automated hermes-sweeper review.
| @@ -51,12 +62,17 @@ export function createComposerAttachmentScope($attachments = atom<ComposerAttach | |||
| } | |||
| }, | |||
| clear() { | |||
| for (const attachment of $attachments.get()) { | |||
There was a problem hiding this comment.
This generic clear runs after direct submit has cloned attachments for dispatchSubmit and after queueing has retained them. Revoking here invalidates the PR's new blob: optimistic preview before its retained consumer renders; defer revocation until that submitted/queued owner is released.
|
Thanks for the catch on the blob URL ownership. You were right: clear() was revoking the preview before the optimistic message (and the queued snapshot) got to use it. What changed in 52eca2e73:
Evidence: |
52eca2e to
aecb504
Compare
Stan Shih (stantheman0128) OS image drops used readFileDataUrl for chip previews, which base64-loads the whole file over IPC and freezes Desktop on Windows. Prefer URL.createObjectURL from the dropped File, and revoke blob previews on chip remove/clear. Closes NousResearch#63682 Co-authored-by: Cursor <cursoragent@cursor.com>
Stan Shih (stantheman0128) Direct submit and queue clone attachments then clear the composer. Revoking blob: URLs inside that clear invalidated the optimistic/queued preview. Retain previews across handoff and revoke when the final consumer is discarded or replaced. Co-authored-by: Cursor <cursoragent@cursor.com>
aecb504 to
28a8456
Compare
SummaryOne PR addresses #63682. #66546 replaces the freeze-causing IPC/base64 preview path for Windows Explorer image drops with object URLs while preserving path-based image attachment routing, and it adds ownership handoffs for direct submission and queued prompts. Related pull requests
Suggested consolidationKeep #66546 open with a salvage path: retain its targeted object-URL preview fix and ownership-handoff tests, while adding and testing production-side revocation when the final optimistic consumer is discarded or replaced. It remains the recorded best-fix consolidation target for #63682, and there are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I63682(["issue #63682 (open)"])
P66546["PR #66546 (open)"]
P66546 -->|best fix| I63682
class I63682 open
class P66546 open
class P66546 best
class P66546 target
click I63682 "https://github.com/NousResearch/hermes-agent/issues/63682"
click P66546 "https://github.com/NousResearch/hermes-agent/pull/66546"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 24 kB of PR diffs, 6 kB of issue/PR text, 2 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
Dragging a PNG/JPEG from Windows Explorer into Hermes Desktop chat was freezing the UI. OS drops were building the composer chip preview via
readFileDataUrl, which base64-loads the whole file over IPC (up to 16 MB). On a typical phone/camera image that stalls the renderer.This keeps the existing path-based
image.attachpipeline (so pasting a path still works the same), but prefersURL.createObjectURLfrom the droppedFilefor the chip/optimistic preview. Path-only attaches (paperclip) still use the IPC data-URL path. Blob preview URLs are revoked when chips are removed or cleared.Related Issue
Fixes #63682
Type of Change
Changes Made
apps/desktop/src/app/chat/hooks/use-composer-actions.ts:resolveImageAttachmentPreview; OS drops and blob saves pass the in-hand File/Blob for previewapps/desktop/src/store/composer.ts: revokeblob:preview URLs on remove/clear/replaceapps/desktop/src/lib/chat-runtime.ts: optimistic bubble rendersblob:previews as markdown imagesHow to Test
.png/.jpgfrom File Explorer into the chat composer.Checklist
Code
Documentation and Housekeeping
Evidence
AI assistance
Prepared with AI assistance (Cursor/Grok). Human author: Stan Shih (@stantheman0128).