pneumo

M4: Files — share sheet and composer send; Download → Open/Share (ENG-266) #30

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-266-m4-files-send-from-share-sheet-and-composer-receive-with

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-266-m4-files-send-from-share-sheet-and-composer-receive-with

Linear: https://linear.app/engineereddev/issue/ENG-266

What's in this PR (104 files, all under apps/android/):

Send: ShareActivity handles ACTION_SEND with EXTRA_STREAM and ACTION_SEND_MULTIPLE (one push per file); the Home composer gets Attach (ACTION_OPEN_DOCUMENT, multiple). Name, MIME and size via OpenableColumns; ciphertextSize checked against maxFileBytes before reserving.
UploadWorker (expedited where allowed, foreground progress on channel transfers): POST /api/files → stream-encrypt with FileCrypto through an OkHttp RequestBody of known length to the presigned (direct) or proxy URL → POST /api/pushes; retries on network errors, 429 and 5xx (ENG-264 classification). Source content is snapshotted to private cache (also for picker documents, so retries never re-encrypt changed bytes under the same key) and deleted afterwards; job metadata is encrypted.
Receive: the notification's Download → DownloadWorker: GET /api/files/:id → stream → decrypt → MediaStore.Downloads/Pneumo/<name> (API 29+) or app-specific Downloads (26–28), with progress; the same notification becomes Open / Share; tag failure deletes the partial file and shows "This file was corrupted or tampered with"; collisions → "name (1).ext". Home file rows get Download / Open / Share. Notification identity and the group summary rules from ENG-263 are preserved (new group regressions).
Generic downloaded MIME types fall back to the file extension for Open/Share; explicit MIME types win.
Tests: worker tests with MockWebServer + WorkManagerTestInitHelper (round trips 0 B / 1 B / 65,536 B / 1 MB, direct and proxy endpoints, tamper cleanup, retry), share-intent parsing, managed-device file exchange; 24 new goldens and 30 re-recorded (named in the commit). New test dependency: androidx.work:work-testing:2.12.0.

Verify (lead, rebased on 0ceccf2):

mise run check:android → exit 0
./gradlew testDebugUnitTest --rerun → exit 0 (335 tests, 0 failures)
./gradlew pixel6Api34DebugAndroidTest --rerun → exit 0 (14 tests, 0 failures)
Acceptance (implementer, API 34 emulator, local random-port server): Photos → share → Pneumo → dev-client download byte-identical; dev-client send file → notification Download → Open in Photos; SHA-256 matched both ways. Lead checked the Download-complete notification with Open/Share.

Deviations: picker documents are snapshotted to private cache rather than kept by persistable grant; MIME fallback by extension for generic types.

🤖 Generated with Claude Code

Commits 2

M4: Fix file transfer retry keys and bulk writes (ENG-266)

+858 -71

M4: Add encrypted Android file transfers (ENG-266)

+4065 -142

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

The PR builds the whole ENG-266 flow (share sheet with one or many streams, composer attach, upload and download workers, MediaStore with IS_PENDING, the same notification updated to Open/Share through PushNotificationGroup.post); check:android (335 tests) and the managed-device suite (14) pass.

FIX (score ≥ 5)

often=8 bad=4 score=32 — transfer/DownloadWorker.kt:402-419: StorageOutputStream extends FilterOutputStream and calls super.write(buf, off, len), which writes one byte at a time; a 64 KiB decrypted block becomes 65,536 write(int) calls (1 MiB ≈ 1.4 s on a Mac; minutes of CPU on a phone for large files). Fix: delegate bulk writes; add a test that the destination receives bulk writes.
often=3 bad=5 score=15 — transfer/UploadWorker.kt:209-234: a failed PUT re-reserves a new file id but re-encrypts the same snapshot with the same file key, so every chunk nonce repeats under a different AAD (AES-GCM nonce reuse; breaks §6.6 "one FK per file"). Fix: fresh FK per reservation, re-encrypt the envelope under the same push id, persist before the PUT; test it.

Missing definition-of-done tests: UploadScheduler.enqueue (size check, blank title/body, caption on the first file only, one item per file); download collision naming; the Home file-send path; the upload dispatcher should assert direct vs proxy paths.

Spec: the manifest adds FOREGROUND_SERVICE_DATA_SYNC, not in §14.2. Round 2 adds ADR 0003 and the §14.2 line.

FILE (score < 5, code left alone, recorded on ENG-275)

score=4 — a stale FAILED upload WorkInfo keeps a red composer error for a day.
score=4 — ShareActivity shows the session-error text for every attachment failure, and copies the whole source before the size check.
score=4 — picker snapshots in cacheDir/upload_sources are orphaned after process death; slow cleanup after sign-out.
score=3 — partial download output (pending MediaStore row or partial file) left after process death.
score=3 — every EOFException is "corrupted", including network EOFs before decryption.
score=3 — setForeground outside try; a background FGS block would fail without cleanup.
score=2 — the share path accepts file:// and Pneumo's own FileProvider authority.
score=1 — hardcoded transfer notification strings.

Note for the owner: Home file rows lost the Copy button (matches web §13). Next: omp round 2.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2 (head 2e83fb3), APPROVE #

The round fixes both FIX findings, adds every missing test and makes an accurate one-line ADR/SPEC change; only the 11 files in the round-2 prompt changed (no goldens, manifest untouched).

FIX 1 resolved (byte-at-a-time writes): StorageOutputStream now extends OutputStream and passes bulk writes straight through; multiChunkDecryptionWritesPlaintextToDestinationInBulk fails on the round-1 code.
FIX 2 resolved (FK reuse): every reservation gets a fresh file key and a re-encrypted envelope under the same push id, written to the spool before the PUT; POST retries resend the identical request. failedPutRetriesWithANewReservationFileKeyAndEnvelope fails on the round-1 code and also if the spool write moves after the PUT.
Missing tests resolved: UploadSchedulerTest (limit message, one item per file, caption on the first only, blank fields omitted), DownloadDestinationTest (API 28 collisions), HomeFileSendTest (5 cases), exact direct/proxy paths.
Spec: ADR 0003 and the §14.2 FOREGROUND_SERVICE_DATA_SYNC line are accurate.
No new findings; the eight round-1 FILE findings stay on ENG-275.

Verify (reviewer and lead): check:android exit 0; testDebugUnitTest --rerun 346/0 failures; pixel6Api34DebugAndroidTest --rerun 14/0. CI 4/4 on 2e83fb3.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

pneumo-reviewer: round 2 (head 2e83fb3), APPROVE #