Skip to content

Support non-image file attachments on desktop - #189

Open
katulevskiy wants to merge 4 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/395
Open

katulevskiy wants to merge 4 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/395

Conversation

@katulevskiy

Copy link
Copy Markdown
Owner

Upstream mirror

Source: zeronsh#395
Original author: @v14
Original revision: 9e7518f8d8bb8b6611b0bcd847c2dd2da1357449

This PR is an isolated Contribution Manager pilot mirror. Its reviews, checks and merge state are separate from upstream. An empty [skip ci] commit suppresses automatic bulk CI; the code tree matches the imported upstream revision.


Fixes #143.

Summary

Selecting a Markdown file or archive in the attachment picker silently discarded it. Desktop attachments now accept any regular file up to 24 MiB, including source files, .gz/.tar.gz archives, files without extensions, and empty files. Images retain thumbnails; other files display an icon and filename.

Root cause

The composer filtered out non-image extensions before staging, and attachment readback enforced an image-only policy even for files in the upload store. Queue editing and transcript rendering also assumed every attachment was an image.

Changes

  • Accept files through the shared picker, paste, and drop intake, with errors for unreadable files, directories, and files above the size limit.
  • Preserve original bytes through the existing chunked upload and queued delivery flow. Transfer attachments to the session host and resolve host-local paths before agent execution.
  • Allow non-image readback from the upload store so queued attachments can be restored for editing. Keep reads restricted to approved paths; workspace files outside the upload store remain image-only.
  • Render non-image attachments as soft, rounded filename pills in the composer and transcript, with generic file icons in the queue. Use an “Attached files” trailer for document or mixed messages, preserving image-only formatting and Appshot context.

This change covers desktop and engine code. iOS file attachment support, document preview, and automatic archive extraction are outside its scope. Existing iOS clients do not display the new file attachments as tiles. Reading non-image attachments back from a remote host, including when editing queued messages, requires the host-side update.

Validation

  • Latest main merged and composer conflict resolved: all 1,229 UI tests, 237 engine tests, and 16 upload/queue/relay integration tests passed; two engine tests ignored. Engine tests passed serially after a process-lock test failed in parallel. Formatting and diff checks passed.
  • After rebasing onto current main: all 182 engine unit tests and 16 upload/readback, queued attachment, and relay integration tests passed. Formatting checks for all changed Rust files and git diff --check passed.
  • After rebasing: all 1,004 desktop UI tests passed against the updated dependencies.
  • Two-engine relay regression verifies exact bytes, multi-chunk transfer, resolved host paths before execution, remote readback, and duplicate-delivery suppression across separate stores.
  • Desktop build and git diff --check passed. All 18 attachment tests passed again after the final mixed-Appshot parser adjustment.
  • Manually verified on macOS with a mock agent: mixed Markdown/ZIP/text/PNG selection, Finder file paste, sending, queue editing and dispatch, and picker cancellation. All 10 uploaded copies matched their source bytes.
  • Live Mac-to-Linux test: the tester confirmed successful file sending from the patched macOS preview to their Linux machine. The Linux engine was not updated for this test.

Not yet tested: running the patched desktop app on Linux or Windows, restoring queued file attachments from a patched Linux host, and native drag-and-drop. The live test covered sending from macOS to Linux; exact-byte comparisons and duplicate-delivery checks were covered by the automated relay test.

Attachment appearance update and screenshots.

@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added the review:needs-triage Managed by Contribution Manager label Oct 6, 2026
@contribution-manager-katulevskiy

Copy link
Copy Markdown

Contribution Manager

needs triage · medium risk · untriaged

Open review workspace · Revision f9c9edd9

Next actions:

  • Direction decision needed
  • Scope, risk, and platform triage needed
  • Independent code review needed
  • Required check: core-tests (missing)

This summary is maintained by the review app. Decisions and test evidence are recorded in the workspace; the app does not merge PRs.

@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added cm:inbox Contribution Manager: waiting for triage and removed review:needs-triage Managed by Contribution Manager labels Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cm:inbox Contribution Manager: waiting for triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants