Skip to content

iOS: attach arbitrary files via a unified photo/file menu#12

Open
ehs208 wants to merge 8 commits into
NewTurn2017:mainfrom
ehs208:feat/iphone-file-attachment
Open

iOS: attach arbitrary files via a unified photo/file menu#12
ehs208 wants to merge 8 commits into
NewTurn2017:mainfrom
ehs208:feat/iphone-file-attachment

Conversation

@ehs208

@ehs208 ehs208 commented Jul 11, 2026

Copy link
Copy Markdown

What

Adds an iPhone attachment path for arbitrary files, not just photos. The workspace input bar's photo button becomes a single + menu (사진 / 파일). The 파일 option opens a document picker (any file type); the picked file is uploaded via the existing file.upload RPC, saved on the Mac under ~/Downloads/cmux-remote/, and its path is inserted into the command draft — the same flow photos already use. Handy for handing a file (Dockerfile, PDF, log, …) to a terminal agent.

Changes

  • iOS (WorkspaceView.swift): PhotoAttachButtonAttachMenuButton (Menu) + .photosPicker(isPresented:) / .fileImporter. New attachFile(_:) reads bytes off the main actor under a security-scoped resource, pre-checks the 12 MiB cap, and reuses uploadFile + appendPathToDraft. Existing photo flow is untouched.
  • iOS (AttachmentNaming.swift, new): pure, unit-tested helpers for basename sanitization (directory-traversal safe) and MIME inference.
  • Relay (HostServices.swift): RelayFileUploadService filename generation was photo-specific and mishandled arbitrary files — an extensionless name (e.g. Dockerfile) got a bogus .jpg appended, and an empty name leaked the cwd. It now respects the client filename verbatim (no forced extension), synthesizes upload.<ext> (extension via UTType) only when no usable name is given, and sanitizes with NSString.lastPathComponent. The relay remains the single source of the <yyyyMMdd-HHmmss>- prefix, so the client no longer double-timestamps.

Note on the build-fix commit

This branch includes a cherry-pick of the type-checker build fix from #11 (Sources/SharedKit/InboxNotification.swift) so the relay compiles on top of main. It's the identical change, so merging #11 first is conflict-free.

Verification

  • Relay: swift test — RelayServerTests 52 + SharedKitTests 37 pass, including new HostServicesTests cases for extensionless / non-image / blank-name / traversal filenames.
  • iOS pure logic (AttachmentNaming, and the too-large error copy) verified via a swift CLI harness using the real sources (all passing).
  • Deferred: the iOS app build, iOS XCTest bundle, and manual UI check were not run — this dev machine lacks the iOS simulator platform for the installed Xcode. A maintainer building in Xcode should confirm: the + menu opens both pickers, an extensionless file lands correctly named in ~/Downloads/cmux-remote/, and the >12 MiB error shows the friendly message.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added iPhone support for attaching files through a combined “Photo or File” menu.
    • Added file size validation, filename sanitization, MIME type detection, upload progress, and user-friendly error handling.
    • Improved notification field extraction across alternative payload formats.
  • Bug Fixes

    • Preserved uploaded filenames and extensions accurately, including extensionless and non-image files.
    • Added safe fallback names and MIME-derived extensions when filenames are blank.
  • Documentation

    • Added design documentation covering file attachment behavior, validation, permissions, and rollout considerations.

ehs208 and others added 8 commits July 11, 2026 01:54
Adds a unified "+" attach menu (사진 / 파일) to the workspace input bar.
The 파일 path opens a document picker (all file types), reads the bytes,
and reuses the existing generic file.upload RPC + appendPathToDraft flow —
saving to ~/Downloads/cmux-remote/ with a timestamp-prefixed filename and
inserting the Mac path into the draft. iOS-only; relay unchanged.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Pure filename-sanitize / timestamp-prefix / MIME-inference helpers, unit
tested. Verified via swift CLI harness (10/10) since the iOS simulator
platform is unavailable in this environment; XCTest run deferred to Xcode.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Replace the photo-only button with a '+' menu (사진/파일). The 파일 path
opens a document picker (all types), reads bytes, and reuses the existing
file.upload RPC + appendPathToDraft flow with a timestamp-prefixed filename.
12 MiB pre-check; existing photo flow unchanged. Updates the smoke UI test's
attach-button accessibility id.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
`swift build -c release` failed to compile InboxNotification.swift:

  error: the compiler is unable to type-check this expression in
  reasonable time; try breaking up the expression into distinct
  sub-expressions

The `body` value was built from a long `??` chain mixing
`stringValue(for:)` and `nestedStringValue(...)`. Because `??` is an
overloaded operator, a chain that long makes the Swift type-checker
explore too many overload combinations and time out.

Introduce a `firstStringValue(forAny:)` helper that returns the first
non-empty string among a list of keys, and use it to collapse the long
`??` chains for workspaceId, title, body, surfaceId, id, subtitle, and
threadId. Key ordering (first match wins) is preserved, so behavior is
unchanged. Release build and all 37 SharedKitTests pass.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
The relay's uniqueFilename was written for the photo flow and mishandled
non-image files:
- extensionless names (Dockerfile, Makefile) got a bogus ".jpg" appended
  because fileExtension() defaulted to jpg for any non-png/heic MIME;
- an empty client name leaked the relay's cwd via URL(fileURLWithPath:"").

Now the relay respects the client filename verbatim (no forced extension),
synthesizes "upload.<ext>" (extension derived from MIME via UTType) only when
no usable name is given, and sanitizes with NSString.lastPathComponent. iOS
drops its now-redundant timestamp prefix (the relay owns the single timestamp)
and reads file bytes off the MainActor. Adds relay tests for extensionless /
non-image / blank-name cases (4/4). relay: 52 XCTest pass; iOS logic verified
via swift CLI (8/8).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
CommandComposer.failSubmit renders errors via String(describing:), which
uses CustomStringConvertible.description, not LocalizedError.errorDescription.
Conform AttachmentError to CustomStringConvertible so the friendly Korean
message shows instead of the raw enum case dump. Also cross-reference the
12 MiB cap constant with the relay's maxBytes.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e7fc464-5276-4979-94a8-343aa54b0c34

📥 Commits

Reviewing files that changed from the base of the PR and between 9f805eb and 0ac68f4.

📒 Files selected for processing (8)
  • Sources/RelayServer/HostServices.swift
  • Sources/SharedKit/InboxNotification.swift
  • Tests/RelayServerTests/HostServicesTests.swift
  • docs/superpowers/specs/2026-07-11-iphone-file-attachment-design.md
  • ios/CmuxRemote/Workspace/AttachmentNaming.swift
  • ios/CmuxRemote/Workspace/WorkspaceView.swift
  • ios/CmuxRemoteTests/AttachmentNamingTests.swift
  • ios/CmuxRemoteUITests/SmokeUITests.swift

📝 Walkthrough

Walkthrough

The PR adds iOS document-file attachments with sanitized naming, MIME detection, size validation, relay upload integration, and updated UI tests. Relay filename generation preserves non-image names. Inbox notifications now extract the first non-empty value from alternative payload keys.

Changes

File Attachment Flow

Layer / File(s) Summary
Attachment naming contracts
ios/CmuxRemote/Workspace/AttachmentNaming.swift, Sources/RelayServer/HostServices.swift, ios/CmuxRemoteTests/AttachmentNamingTests.swift
Adds basename sanitization and MIME fallback helpers, and updates relay filename generation to avoid image-specific defaults.
File picker and upload flow
ios/CmuxRemote/Workspace/WorkspaceView.swift, docs/superpowers/specs/.../2026-07-11-iphone-file-attachment-design.md
Adds a photo/file attachment menu, single-file importing, size checks, asynchronous reads, uploads, draft updates, focus handling, and error reporting.
Attachment behavior validation
Tests/RelayServerTests/HostServicesTests.swift, ios/CmuxRemoteUITests/SmokeUITests.swift
Tests extensionless, non-image, and MIME-derived filenames, and updates the attachment accessibility identifier used by the keyboard-layout test.

Inbox Notification Parsing

Layer / File(s) Summary
Alternative notification payload values
Sources/SharedKit/InboxNotification.swift
Adds ordered first-non-empty string lookup for notification fields while preserving existing defaults and fallback derivations.

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

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AttachMenuButton
  participant WorkspaceView
  participant SurfaceStore
  participant RelayFileUploadService
  User->>AttachMenuButton: Select file
  AttachMenuButton->>WorkspaceView: Open fileImporter
  WorkspaceView->>WorkspaceView: Validate size and read file
  WorkspaceView->>SurfaceStore: Upload data, filename, and MIME type
  SurfaceStore->>RelayFileUploadService: Save upload
  RelayFileUploadService-->>SurfaceStore: Return uploaded path
  SurfaceStore-->>WorkspaceView: Return upload result
  WorkspaceView->>WorkspaceView: Append path to command draft
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding arbitrary file attachments to iOS through a unified photo/file menu.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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.

1 participant