Repository navigation
Keep picked skills and pending uploads when native chat composers close - #26538
Conversation
…2025/nc-split-draft-fixes
|
Native chat could lose a pasted image when its composer closed, and a completed file upload could flatten a picked skill into ordinary text or lose its original draft owner. This split preserves the stored editor document during text append and makes the existing pending cache complete accepted uploads into their owning draft after the composer closes. Pending pastes now use the existing chip and remove control, including pastes that will become file references. Removing one releases Send and rejects late results. The seven review rounds verified these corrections:
Draft movement and the planned same-conversation clear redesign are excluded. Host-side save deadlines remain deferred; removing a draft operation does not cancel the host promise. Fresh final-head review at 44330e9 found zero P0–P2 issues and reran 565 repository tests plus 93 prior controls. Final local validation passed 468 tests, all 3 queued typechecks and 22 static checks; one optional benchmark stayed skipped and is not a pass. Seventeen behavior ablations recorded intended failures. Final CI completed on the exact head. Full PR run 37776453242 passed all ten unit shards, both packaging jobs, relay integration and the required aggregate. The readiness PR run explicitly reused that identical successful source/workflow; the Headless Node run succeeded and reported no server inputs changed, so its platform lanes were skipped. Final confirmation found 16 passed checks, 21 skipped and no failures or pending checks. Skips are not passes; no end-to-end specs were selected. The five embedded captures show hidden macOS Electron with isolated profiles and stand-in providers: a native image finishes through a genuine composer unmount and returns with the picked skill; removing a reference paste releases Send and rejects its late result; plain-text menu-event paste keeps Send enabled. Captures are from the disclosed earlier heads. Live final-head/Windows/Linux/SSH/paired/mobile/real-provider/native-menu and concurrent-window behavior are unverified; final source and routing have automated coverage. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (33)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughClipboard image checks now use detected image sources, including validated Windows copied-image paths. Native chat paste operations create and track pending attachments across composer lifetimes, and reference settlement retains operation IDs. Draft additions update prompt-document content alongside text. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
ELI5
A picked skill could turn into ordinary text when a file finished uploading after the chat box closed. Local and SSH image pastes could also disappear when the composer closed before saving finished. Text appends discarded the saved editor document, and the composer still controlled some pending uploads even though the draft outlived it.
Reopening the chat now preserves picked skill chips and completed attachments. Every pending paste appears in the existing attachment row, including pastes that will become file references. Its remove button releases Send if saving stalls; removing an attachment or its workspace prevents a late result from adding it back.
What Changed
The existing draft-addition function appends text while preserving selected nodes. Normal store updates, delayed loading, and crash-journal replay all use that function. Pending file results keep their operation IDs until the attachment cache settles them into their owning draft, including its workspace owner. Pending local, SSH, and paired uploads survive composer teardown. Menu paste first checks for a supported clipboard image source; plain text registers no image operation or Send hold. Confirmed-image ownership starts before saving, independently of a thumbnail or image-input support; agents that use file references keep that presentation. The hidden pending state is removed: pending operations always use the existing chip and remove control, while file-reference agents still receive a path when saving succeeds. Desktop presence checks and saving share one source selector, including Windows single-file PNG/JPEG copies; file reads, decoding and existing limits remain in the save. Deferred paired previews recheck operation ownership before creating an object URL, so removal cannot leave a late thumbnail without an owner. Preview URLs remain owned by the composer. Unique operation IDs and per-scope composer lifetime prevent results from another composer being mistaken for current input.
These are the two independent fixes split out of #26054. Its draft-move behavior is excluded because the planned
/clearredesign keeps one conversation; that follow-up is coming separately. This PR does not change/clearbehavior.Why
The draft store and pending-attachment cache already own unsent input. Fixing their append and completion paths preserves input even without a mounted editor, rather than adding protections to individual upload callers. There is no draft-move registry, new persisted field, or wire-format change.
Differences from the common pattern:
Linked Issue
N/A — maintainer follow-up from a native chat audit.
Visual Proof
A genuine native clipboard save completed while a permission card had replaced the composer. Answering the card restored the same conversation with its picked Review pill, suffix and image. Hidden macOS Electron QA used a hooks-off isolated profile and a stand-in provider; the held request forwarded the original host handler and returned its actual file. Protected-file hashes matched all three checkpoints; recorded processes and rig were cleaned up. Workspace switching also preserved the draft, but retained the composer and is separate evidence.
Before: the image is still saving beside the picked skill and suffix.
The permission card replaces the composer while the save finishes.
After: the same conversation restores the skill pill, suffix and image.
A file-reference paste also shows the existing pending chip and remove control. In isolated OMP QA, removing it re-enabled Send; releasing the original native save afterward left the original question unchanged. Workspace switching preserved the same chat but does not establish a component unmount.
A plain-text app-menu paste preserved the question and kept Send enabled without registering an image. Hidden Electron QA used the production renderer menu event route and a genuine native text clipboard: the original presence handler returned false once and the save handler was never called. Physical menu interaction was not exercised.
Testing
The final post-sync regression run passes 468 tests across 37 files. The manifest explicitly lists 38 files; one optional cleanup benchmark file/test remains skipped and is not counted as passing. Coverage includes picker-created skills with an unmounted PDF completion, delayed loading, crash replay, local/SSH/paired unmount survival, original draft/workspace ownership, reference-only agents, removal, failures, workspace deletion and remount identity. All clipboard suites are included, covering shared image-source selection, Windows single-file validation, destination routing, older hosts and authorization. Seventeen recorded behavior ablations fail the intended assertions when their fixes are removed.
Fresh web, node and CLI typechecks pass through the machine queue after the single final main sync and frozen install. All 22 final local static checks pass. Final post-sync review passed with zero P0–P2 findings. Final CI completed on the exact head. Full PR run 37776453242 passed all ten unit shards, both packaging jobs, relay integration and the required aggregate. The readiness PR run explicitly reused that identical successful source/workflow; the Headless Node run succeeded and reported no server inputs changed, so its platform lanes were skipped. Final confirmation found 16 passed checks, 21 skipped and no failures or pending checks. Skips are not passes; no end-to-end specs were selected. No lockfile changes are included.
Hidden macOS Electron evidence was captured on the source committed as d49d546 (true composer unmount with picked skill and native image), 4dbd128 (visible reference chip/removal) and 7b93b2d (plain-text menu paste). The final source has automated coverage for the subsequent shared desktop clipboard selector. The rigs used isolated hooks-off profiles and stand-in providers, forwarded original native handlers, and were cleaned up; protected-file hashes matched all three checkpoints. Physical native-menu input was not used.
Live SSH, a live paired server, mobile, Windows and Linux were not exercised manually. SSH/paired ownership and Windows clipboard sources have automated coverage. Existing peer formats and execution-host ownership are preserved.
Review
Round seven reviewed the final post-sync head
44330e9c2660452e349c54e818dd7896544d6761and found zero P0–P2 findings. It reran 565 repository tests across 45 files and 93 earlier regression controls, verified the merge preserved the fixes, and downloaded/checked all five embedded screenshots. Earlier rounds corrected thumbnail-dependent ownership, reference-only paste ownership, invisible stalled operations, plain-text image feedback, previews created after removal, Windows file-source detection and two stale clipboard test fixtures. Host-side save deadlines remain deferred; no draft movement is included.Agent skill upstream boundary
Checklist