Skip to content

Preserve image attachments when dropping or pasting into terminals - #12752

Merged
austinywang merged 25 commits into
mainfrom
issue-12684-image-drop-attachment
Sep 17, 2026
Merged

austinywang merged 25 commits into
mainfrom
issue-12684-image-drop-attachment

Conversation

@austinywang

@austinywang austinywang commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Dropped or pasted image files must remain available while Claude Code or Codex reads them asynchronously. Keep cmux-owned local image files until the existing app-exit cleanup instead of deleting them when terminal text delivery finishes.

A copied image can also advertise a source URL on the clipboard. Resolve its image/rich-text payload before that auxiliary URL so image bytes accompanied by a folder URL, web URL, or missing file path become an image attachment. Ordinary Finder file/folder pastes retain their file-URL identity and remote-upload routing; visible rich text keeps its existing priority. Terminal drag routes now carry the original pasteboard through the pane/overlay adapters, so those paths cannot discard the image bytes before shared preparation. Existing backing files still take priority over drag thumbnails.

Validation

  • The original test-only lifetime commit failed both drop/paste cases; the lifetime fix passed both on the hosted macOS lane.
  • Separate test-only and fix commits prove the paste regression: all three auxiliary-URL cases fail before; all seven lifetime/priority/control cases pass after.
  • Expanded the suite to drag mode and a live PTY consumer. It checks bracketed-paste boundaries and reads the delivered image from disk through direct and routed drops. Current drag validation is running.
  • Tagged fleet build at 5395d8562bcdf8a5644ab23257e457297c20abe6 succeeded, installed, launched, and responded through its tag-bound CLI. The subsequent commit changes only CI runner selection. Full CI validation is still in progress.
  • Corrected concrete inherited CI failures in runner selection, isolated iOS lane-version fixtures, Dock audit coverage, and the test flag fixture. Replaced fixed-delay assertions with causal completion signals; the transport installer exposes an internal wait for its existing task so its cancellation test can wait for completion. The determinism gate passes with zero exemptions. The runner guard, 68 workflow-security tests, and iOS lane-identity tests pass.
  • Swift file-length budgets, Package.resolved policy, and diff whitespace checks pass.
  • Localization audit: no user-facing strings changed.

Fixes #12684. Related: #12680.

Summary by CodeRabbit

  • Bug Fixes

    • Improved paste handling so copied images and rich text take priority over accompanying URLs.
    • Preserved pasted image files after insertion, preventing them from becoming unavailable unexpectedly.
    • Maintained correct handling for text, image, file, folder, and URL pastes, including drag-and-drop operations.
  • Tests

    • Added coverage for image-transfer persistence, content-priority rules, and Finder paste behavior.
    • Improved test synchronization for more reliable results.
  • Chores

    • Updated release delivery automation to use the configured build runner.

Exercise the shared local image transfer path for both image drops and Cmd+V paste. The materialized file must remain available after the path is sent so Claude Code and Codex can read it asynchronously.
Do not delete cmux-owned image files when the local terminal path is delivered. Claude Code and Codex read that path asynchronously after sendText returns; retain the file for the existing process-lifetime cleanup instead.
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c77b42c5-e3c3-4739-ac86-ea5a2a558ef3

📥 Commits

Reviewing files that changed from the base of the PR and between ddf32d3 and 5373b4a.

📒 Files selected for processing (23)
  • .github/workflows/cmux-tui-release-delivery.yml
  • Packages/Shared/CmuxIrxTransport/Sources/CmuxIrxTransport/IrxRelayCredentialInstaller.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxLiveQUICTests.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxLivenessTests.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxRelayCredentialInstallerTests.swift
  • Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService+TextContents.swift
  • Sources/DragOverlayRoutingPolicy.swift
  • Sources/FileDropOverlayViewHitTesting.swift
  • Sources/GhosttyNSView+PreparedImageTransfer.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/PaneDropContainer.swift
  • Sources/PasteboardFileURLReader.swift
  • Sources/TerminalImageTransfer.swift
  • Sources/TerminalPaneDropTargetView.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/MobileHostConnectionEventLaneTests.swift
  • cmuxTests/TerminalLocalImageTransferFileLifetimeTests.swift
  • tests/test_dock_shortcut_routing_guard.py
  • tests/test_ios_appstore_lane_identity.py
  • web/tests/bun-test.d.ts
  • web/tests/client-config-route.test.ts
  • web/tests/client-config-runtime-cache.test.ts
  • web/tests/iroh-dashboard-controller.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e0eceda0-1215-4214-b83b-6637fbf5b4ae

📥 Commits

Reviewing files that changed from the base of the PR and between e57f6de and ddf32d3.

📒 Files selected for processing (19)
  • Packages/Shared/CmuxIrxTransport/Sources/CmuxIrxTransport/IrxRelayCredentialInstaller.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxLiveQUICTests.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxLivenessTests.swift
  • Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/IrxRelayCredentialInstallerTests.swift
  • Sources/DragOverlayRoutingPolicy.swift
  • Sources/FileDropOverlayViewHitTesting.swift
  • Sources/GhosttyNSView+PreparedImageTransfer.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/PaneDropContainer.swift
  • Sources/TerminalImageTransfer.swift
  • Sources/TerminalPaneDropTargetView.swift
  • cmuxTests/MobileHostConnectionEventLaneTests.swift
  • cmuxTests/TerminalLocalImageTransferFileLifetimeTests.swift
  • tests/test_dock_shortcut_routing_guard.py
  • tests/test_ios_appstore_lane_identity.py
  • web/tests/bun-test.d.ts
  • web/tests/client-config-route.test.ts
  • web/tests/client-config-runtime-cache.test.ts
  • web/tests/iroh-dashboard-controller.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Image transfer planning now prioritizes image payloads and preserves materialized files through terminal insertion. Drop handlers forward pasteboards to prepared transfers. Tests replace timing sleeps with explicit synchronization. Release and validation fixtures also receive targeted updates.

Changes

Image transfer handling

Layer / File(s) Summary
Paste selection and materialization
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService+TextContents.swift, Sources/TerminalImageTransfer.swift
Image and RTFD payloads no longer take the URL-paste path first. Paste planning reads text once, materializes image files when needed, and preserves eligible file URLs.
Drop routing and transfer execution
Sources/GhosttyNSView+PreparedImageTransfer.swift, Sources/GhosttyTerminalView.swift, Sources/DragOverlayRoutingPolicy.swift, Sources/FileDropOverlayViewHitTesting.swift, Sources/PaneDropContainer.swift, Sources/TerminalPaneDropTargetView.swift
Drop paths forward the source pasteboard. Prepared transfers no longer pass a text-completion cleanup closure. Obsolete drop helpers are removed.
Local image lifetime tests
cmuxTests/TerminalLocalImageTransferFileLifetimeTests.swift, cmux.xcodeproj/project.pbxproj
Added tests for owned PNG lifetime, image precedence, source URL preservation, and hosted terminal delivery. The test file is added to the Xcode target.

Asynchronous test synchronization

Layer / File(s) Summary
IRX lifecycle tests
Packages/Shared/CmuxIrxTransport/Sources/CmuxIrxTransport/IrxRelayCredentialInstaller.swift, Packages/Shared/CmuxIrxTransport/Tests/CmuxIrxTransportTests/*
The installer exposes waitUntilSettled(). IRX tests await connection, probe, and retry state transitions.
Mobile host and web tests
cmuxTests/MobileHostConnectionEventLaneTests.swift, web/tests/client-config-route.test.ts, web/tests/iroh-dashboard-controller.test.ts
Tests use event-driven waiters and promise gates instead of fixed-duration sleeps.

Release and validation updates

Layer / File(s) Summary
Release and fixture execution
.github/workflows/cmux-tui-release-delivery.yml, tests/test_ios_appstore_lane_identity.py
The delivery job uses the configured Linux runner. Isolated iOS repositories normalize fixture versions before test execution.
Validation contracts
tests/test_dock_shortcut_routing_guard.py, web/tests/bun-test.d.ts, web/tests/client-config-runtime-cache.test.ts
The shortcut guard includes mapped resize actions. MockFunction exposes mockImplementation. The cache fixture uses featureFlags.enabled.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant DragSource
  participant FileDropTextDropController
  participant GhosttyNSView
  participant TerminalImageTransferPlanner
  participant Terminal
  DragSource->>FileDropTextDropController: Pass URLs and dragging pasteboard
  FileDropTextDropController->>GhosttyNSView: Forward dropped URLs and pasteboard
  GhosttyNSView->>TerminalImageTransferPlanner: Prepare drop transfer
  TerminalImageTransferPlanner-->>GhosttyNSView: Return prepared file or image transfer
  GhosttyNSView->>Terminal: Insert prepared transfer
Loading

Merge Risk: ⚪ Minimal · up to ddf32

No actionable current-head risk remains from the reviewed changes.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 2 warnings)

Check name Status Explanation Resolution
Cmux Swift Package Boundaries ❌ Error The PR materially expands independent transfer-planning logic in the app target. Sources/TerminalImageTransfer.swift changes TerminalImageTransferPlanner.preparePaste and materializedFileURLs to… Extract the smallest changed boundary into a package target such as CmuxTerminalTransfer. Move the payload-preparation decision cut (TerminalImageTransferPreparedContent, the file-URL payload reader, preparePaste, and `materializedFil…
Cmux No Test Or Debug Seam In Production Source ❌ Error The PR adds waitUntilSettled() to the production source file Packages/Shared/CmuxIrxTransport/Sources/CmuxIrxTransport/IrxRelayCredentialInstaller.swift. The method awaits the private task state… Remove waitUntilSettled() from production source. In the test target, use @testable import CmuxIrxTransport and widen only private var task to internal if needed, then await the task directly from IrxRelayCredentialInstallerTests.…
Out of Scope Changes check ⚠️ Warning The pull request includes changes with no demonstrated connection to issue #12684. Examples include the IrxRelayCredentialInstaller synchronization changes, mobile-host and web test changes, iOS App… Remove the unrelated production, workflow, and test changes from this pull request, or link coding requirements that require them. Keep the image-transfer implementation and its focused tests.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 19 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (21 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #12684 requires existing, readable paths for screenshot drops and clipboard image pastes. TerminalImageTransferPlanner now materializes decodable image payloads before auxiliary file or URL ha…
Cmux Swift Actor Isolation ✅ Passed No changed production declaration introduces a listed actor-isolation mistake. The PR adds waitUntilSettled() to IrxRelayCredentialInstaller, which is already an actor. The image-transfer changes …
Cmux Swift Blocking Runtime ✅ Passed No changed production Swift line introduces a prohibited blocking or timing primitive. The new IrxRelayCredentialInstaller.waitUntilSettled() awaits the actor-owned task's explicit completion point …
Cmux Browser Automation Off-Main ✅ Passed PASS: The authoritative PR diff does not modify Sources/TerminalController.swift, Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift, or the policy…
Cmux Expensive Synchronous Load ✅ Passed PASS. The authoritative Swift diff adds no RestorableAgentSessionIndex, agent hook/session store, transcript, trajectory, workstream/event JSONL, broad directory scan, or large JSON parser. The chan…
Cmux Cache Substitution Correctness ✅ Passed PASS: The production Swift diff does not replace an authoritative read with a cached value in a persistence, history, undo, or snapshot path. The changed transfer code reads the current pasteboard, ma…
Cmux No Hacky Sleeps ✅ Passed PASS. The non-Swift changes are test-only Python/TypeScript files and a GitHub Actions workflow. The TypeScript test changes remove fixed setTimeout(0) waits and replace them with explicit promise a…
Cmux Algorithmic Complexity ✅ Passed PASS. The production changes do not introduce a prohibited complexity shape. The new urls.allSatisfy(isRemoteUploadableFileURL) in Sources/TerminalImageTransfer.swift:528 is a linear scan over dro…
Cmux Swift Concurrency ✅ Passed The PR does not introduce a prohibited legacy async pattern. The only new production concurrency API is IrxRelayCredentialInstaller.waitUntilSettled() async, which awaits the actor’s existing stored…
Cmux Swift @Concurrent ✅ Passed PASS. The Swift diff adds no @concurrent or nonisolated async declaration. The only new production async method, IrxRelayCredentialInstaller.waitUntilSettled(), is actor-isolated and awaits acto…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The authoritative PR diff changes no Package.swift, Package.resolved, or .gitignore file. The only Xcode project changes add TerminalLocalImageTransferFileLifetimeTests.swift to the test target;…
Cmux Swift Logging ✅ Passed PASS. The production Swift diff adds no print, debugPrint, dump, NSLog, ad hoc file logging, or new Logger declaration. The only added diagnostic is cmuxDebugLog inside #if DEBUG in `Sou…
Cmux User-Facing Error Privacy ✅ Passed The authoritative PR diff does not add or materially change user-facing error text, alerts, command output, API error bodies, or recovery copy. Production changes update image-paste/drop routing, file…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes terminal transfer logic, routing, tests, synchronization, and CI configuration. It adds no Swift UI/menu/alert/error copy, no localization catalog or Info.plist en…
Cmux Swiftui State Layout ✅ Passed PASS: The reviewed Swift diff introduces no new ObservableObject, @Published, @StateObject, @EnvironmentObject, @ObservedObject, @Bindable, GeometryReader, lazy/list row store reference,…
Cmux Architecture Rethink ✅ Passed PASS. The production Swift changes are small correctness fixes with clear ownership. Image files remain owned by TerminalPasteboardService, and AppDelegate.applicationWillTerminate remains the cle…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The Swift diff changes terminal image-transfer and drag-routing methods. It does not add or materially change a standalone cmux-owned window. The only new NSWindow construction is in cmuxTests/T…
Cmux Source Artifacts ✅ Passed The PR changes only source, test, workflow, declaration, and Xcode project files. The only added path is cmuxTests/TerminalLocalImageTransferFileLifetimeTests.swift, which is a deliberate test-syste…
Cmux No Ambient Global State ✅ Passed No ambient global state was introduced in the production Swift diff. The new APIs are instance methods on existing scoped types: IrxRelayCredentialInstaller.waitUntilSettled() and GhosttyNSView dr…
Title check ✅ Passed The title clearly summarizes the primary change: preserving image attachments during terminal drops and pastes.
Description check ✅ Passed The description provides a detailed summary and validation results. It does not include the template's Demo Video, Review Trigger, or Checklist sections, but the core change and testing information ar…
Full details: Out of Scope Changes check

Explanation

The pull request includes changes with no demonstrated connection to issue #12684. Examples include the IrxRelayCredentialInstaller synchronization changes, mobile-host and web test changes, iOS App Store fixture changes, the dock shortcut guard change, and the cmux-tui-release-delivery.yml runner change. These changes address unrelated transport, release, routing, and web-test behavior.

Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 19 files. (1 skipped: 1 too large.)

Full details: Cmux Swift Package Boundaries

Explanation

The PR materially expands independent transfer-planning logic in the app target. Sources/TerminalImageTransfer.swift changes TerminalImageTransferPlanner.preparePaste and materializedFileURLs to decide image, rich-text, auxiliary-URL, and durable-file precedence. This is domain logic, not AppKit or Ghostty view glue. The planner is reused by terminal paste/drop, the composer, file explorer insertion, file preview insertion, and multiple focused tests. It also already accepts an injected TerminalPasteboardService, which supports isolated testing. The pasteboard service and its image-materialization logic are in Packages/macOS/CmuxTerminal, but the new precedence logic remains in the app-root Sources/ target. The other changed app files are routing and Ghostty integration glue and are allowed.

Resolution

Extract the smallest changed boundary into a package target such as CmuxTerminalTransfer. Move the payload-preparation decision cut (TerminalImageTransferPreparedContent, the file-URL payload reader, preparePaste, and materializedFileURLs) behind a public TerminalPasteTransferPlanner API. Expose a small TerminalPasteboardTransferSource protocol, or an equivalent value-based input API, so the planner does not depend on GhosttyApp, view state, or app singletons. Keep AppKit pasteboard adaptation, Ghostty routing, target resolution, upload execution, and focus handling in the app target. Move the precedence and materialization unit tests to the package target.

Full details: Cmux No Test Or Debug Seam In Production Source

Explanation

The PR adds waitUntilSettled() to the production source file Packages/Shared/CmuxIrxTransport/Sources/CmuxIrxTransport/IrxRelayCredentialInstaller.swift. The method awaits the private task state and has no non-test caller. The changed test calls it solely to observe completion after releasing the retry latch. This adds a test-observation seam to production source. Other added DEBUG logging is on a real production drop path and is not a test accessor.

Resolution

Remove waitUntilSettled() from production source. In the test target, use @testable import CmuxIrxTransport and widen only private var task to internal if needed, then await the task directly from IrxRelayCredentialInstallerTests. Keep test synchronization and observation in the test target. See #6452.

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-12684-image-drop-attachment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang
austinywang marked this pull request as draft September 16, 2026 06:10
@austinywang austinywang changed the title Keep local image transfer files available to terminal consumers Keep dropped and pasted image files available to terminal consumers Sep 16, 2026
@kgwoo

kgwoo commented Sep 16, 2026

Copy link
Copy Markdown

Love cmux and would like to help where I can. I hit this one myself and put up
a fix in #12670 on 09-15, with a regression test and before/after videos. Looks
like the same change as here, and it's currently green on CI.

Either way works - just flagging it in case it's useful.

@austinywang
austinywang marked this pull request as ready for review September 16, 2026 22:56
@austinywang austinywang changed the title Keep dropped and pasted image files available to terminal consumers Preserve image attachments when dropping or pasting into terminals Sep 16, 2026
@cursor

cursor Bot commented Sep 16, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@austinywang
austinywang merged commit 9fe20d7 into main Sep 17, 2026
38 of 40 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 17, 2026
be7d9fa iOS: make the keyboard button Liquid Glass by default
9a916eb Add the cmux RC release channel (com.cmuxterm.app.rc) (manaflow-ai#12777)
ec4cbd4 Use PlanetScale for Cloud VM migrations and operator guidance (manaflow-ai#12821)
68df8ee fix: apply TUI rustfmt before release (manaflow-ai#12823)
9fe20d7 Merge pull request manaflow-ai#12752 from manaflow-ai/issue-12684-image-drop-attachment
5373b4a fix: preserve backing files before decoding Finder paste previews
c08bcd3 test: preserve Finder originals when the pasteboard includes a TIFF preview
b1ee541 refactor: keep pasteboard context limited to file insertion
4ca686d Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue-12684-image-drop-attachment
9b3b242 test: allow main-actor terminal startup during image delivery capture
ddf32d3 test: type mock implementations in Bun test declarations
1b332f7 fix: preserve complete image payloads through terminal drop routing
c082946 test: cover drag payload priority and bracketed image delivery
cdb73a1 test: synchronize client config concurrency through request admission
a5ab2bf test: await causal transport and dashboard events
fe8e7af test: use a generic flag in cache round-trip fixtures
7713752 test: recognize mapped pane resize actions in Dock routing audit
872f46e test: isolate App Store lane versions from release bumps
e57f6de ci: route the release delivery guard through the configured runner
5395d85 fix: prefer copied image payloads over auxiliary URLs
fd5d49d test: preserve image paste payloads with auxiliary URLs
aed23e6 Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue-12684-image-drop-attachment
5d3616c Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue-12684-image-drop-attachment
d7ac49c Revert "Fix duplicate Xcode build file IDs after main merge"
b3daf57 Revert "Restore auth observer required by merged mobile runtime"
38dcda7 Restore auth observer required by merged mobile runtime
42631af Fix duplicate Xcode build file IDs after main merge
a5b91d2 Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue-12684-image-drop-attachment
68edbe2 fix: keep local image transfer files alive
d815ff6 test: retain local image files through terminal delivery

# Conflicts:
#	.github/workflows/cloud-vm-migrate.yml
#	.github/workflows/cmux-tui-release-delivery.yml
#	.github/workflows/nightly.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in 0.64.23: dropping or pasting an image into the terminal inserts a clipboard-*.png path that does not exist on disk

2 participants