Any-file attachments to cloud agents and remote CLI sessions#4628
Merged
Conversation
…zation Extend cloud-agent attachments from a fixed 9-extension allow-list to any non-denied file type, with per-file materialization failure handling. - Add optional `extension` to the upload-presign input (personal + org). When present, the R2 key suffix derives from the validated extension and `contentType` relaxes to a MIME-shape string; when absent, the legacy allowed-MIME contract is preserved unchanged (released web hook and pre-upgrade mobile clients keep working). - Enforce a shared deny-list (exe, dll, msi, com, scr, apk, ipa, dmg, pkg) at every filename boundary: web schemas, worker persistence schema, runtime download validation, and the new download-presign helper. - Pin a canonical extension->MIME table; unknown/absent extensions resolve to application/octet-stream. No MIME-based extension inference. - Wrapper streams downloads with a 5 MB bound, deletes partials on overflow, and converts per-file failures (non-2xx, timeout, overflow) into explanatory text parts so the agent turn proceeds. Generic-binary attachments are surfaced as text parts with path + filename + mime + size. - Add generateCloudAgentAttachmentDownloadUrl (author-scoped key, 900s) and cloudAgentGetAttachmentDownloadUrlSchema for the remote-CLI download path.
Extend the agent composer from images/9 extensions to any non-denied file.
- Replace the closed allow-list with the shared deny-list policy; classify
by measured local byte size (never the picker's optional size), rejecting
zero-byte, deny-listed, >5 MB, and >5-file selections with specific
messages while keeping valid siblings in a mixed selection.
- Derive attachment MIME solely from the normalized extension (fallback bin
-> application/octet-stream); the picker MIME is never consulted.
- Per-chip determinate upload progress via expo-file-system/legacy
createUploadTask (binary PUT), with a blob-PUT fallback.
- Split upload failures into terminal (presign BAD_REQUEST/FORBIDDEN/
UNPROCESSABLE_CONTENT -> "This file can't be uploaded.", no retry) vs
retryable (network/timeout/408/429/5xx/PUT -> "Upload failed. Tap to
retry."); send stays blocked while any attachment is uploading or failed.
- Expose a typed composer submission payload { wire, messageUuid, files:
[{ remoteName, originalName, size }] } (no mime) for the remote-CLI send
path; widen the composer onSend signature with an optional third arg.
- Add iOS UIFileSharingEnabled + LSSupportsOpeningDocumentsInPlace for
Files-picker fixture ingress.
- Add cloudAgentNext.getAttachmentDownloadUrl: a personal, author-scoped presigned GET for remote-CLI attachment materialization, validated via S1's shared download schema. No org mirror; no key-derivation changes. - Plumb an optional CLI heartbeat capability flag through session-ingest: services/session-ingest accepts capabilities.attachments on the heartbeat, UserConnectionDO stores it per connection (mirroring protocolVersion) and projects it into both aggregateSessions() rows and the sessions.heartbeat event, omitting it when the latest heartbeat omits it. - Expose capabilities on the activeSessions.list session schema so the mobile client can gate the attachment UI on remote CLI sessions.
- Gate the composer paperclip on the remote CLI's advertised capability:
supportsAttachments for a remote session is true only when the heartbeat
carries capabilities.attachments === true, recomputed on every heartbeat
(upgrade/downgrade/reconnect/absent). Cloud-agent sessions are unchanged.
- SDK send input gains an optional attachmentParts field (distinct from the
cloud-only {path,files} attachments); cli-live-transport appends the file
parts after the text part in send_message. Remove the blanket
'Only Cloud Agent sessions support attachments' throw for capable remotes.
- Mobile builds ready FilePartInput parts for a capable remote session:
mint an author-scoped presigned GET per file, set filename to the
server-issued remoteName (<uuid>.<ext>) and derive mime solely from that
extension via the canonical table. The original filename never leaves the
device; the {path,files} cloud wire is untouched.
… presign-failure feedback - cli-live-transport resets the advertised attachment capability on live reconnect so the composer gate closes until the next heartbeat/sessions .list re-advertises it (fail-closed); add a transport-level reconnect test. - session-detail surfaces a retryable message and preserves the draft + attachments when minting a remote attachment's presigned GET fails before send, instead of silently swallowing the error.
Fix CI: oxfmt the new test file and make BuildRemoteAttachmentPartsResult module-internal (knip flagged it as an unused export).
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryCommit 92fe234 correctly resolves all five previously flagged code-level findings (extension re-derivation, deny-list error message, deny-list enforcement in the upload-URL helper, partial-file cleanup on non-overflow download failures, and the one-byte download-cap tolerance) with no new issues introduced in the changed files. Files Reviewed (10 files)
Previous Review Summaries (2 snapshots, latest commit 92fe234)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 92fe234)Status: No Issues Found | Recommendation: Merge Executive SummaryCommit 92fe234 correctly resolves all five previously flagged code-level findings (extension re-derivation, deny-list error message, deny-list enforcement in the upload-URL helper, partial-file cleanup on non-overflow download failures, and the one-byte download-cap tolerance) with no new issues introduced in the changed files. Files Reviewed (10 files)
Previous review (commit fb32f5a)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe most impactful issue is a client-side attachment extension re-derivation bug in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (60 files)
Reviewed by claude-sonnet-5 · Input: 32 · Output: 5.3K · Cached: 704.2K Review guidance: REVIEW.md from base branch |
…-depth, wrapper partial cleanup + cap boundary - mobile: retain the normalized extension on the attachment and send it to the presign instead of re-deriving (and casting) from the raw filename, so an invalid suffix stays consistent with its bin/octet-stream classification. - web schema: interpolate the offending extension into the deny-list error. - r2 upload helper: reject deny-listed extensions before signing (defense-in-depth, matching the download helper). - wrapper: delete the partial file on any download failure (not only overflow), and reject a file exactly one byte over the 5 MiB cap.
jeanduplessis
approved these changes
Jul 21, 2026
…ents # Conflicts: # apps/web/src/routers/active-sessions-router.test.ts # apps/web/src/routers/active-sessions-router.ts # services/session-ingest/src/dos/UserConnectionDO.test.ts # services/session-ingest/src/dos/UserConnectionDO.ts # services/session-ingest/src/types/user-connection-protocol.ts
iscekic
enabled auto-merge (squash)
July 21, 2026 12:16
A static import of the router module in the listInstances test evaluated config.server.ts (freezing SESSION_INGEST_WORKER_URL at the empty .env.test value) before beforeAll could set the env, breaking the dynamic-import dance. Move the schema-validation cases to active-sessions-router.schema.test.ts, which Jest sandboxes separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Lets the Kilo mobile app attach any (non-denied) file type to messages sent to both cloud-agent sessions and remote CLI sessions. Previously mobile attachments were limited to a fixed 9-extension allow-list on cloud-agent sessions only; remote CLI sessions had no attachment path at all. Image behavior is the parity floor — everything images could do, arbitrary files now do.
Companion CLI change (materialization + capability advertisement): Kilo-Org/kilocode#12394.
Policy (documented, enforced at every layer)
content-lengthheader alone).extensionfield on the upload presign switches policy from the old allow-list to a deny-list (exe, dll, msi, com, scr, apk, ipa, dmg, pkg). Whenextensionis absent (released clients, the existing web hook), the legacy allowed-MIME contract is preserved unchanged. The deny-list is enforced independently at every filename boundary: web tRPC schemas, the worker persistence schema, the runtime download validator, and the new download-presign helper.bin→application/octet-stream; there is no MIME-based extension inference anywhere.Remote CLI enablement
cloudAgentNext.getAttachmentDownloadUrl(900 s). Mobile mints one signed GET per attachment and sends the CLI a first-class file part whosefilenameis the server-issued<uuid>.<ext>basename; the CLI classifies and materializes from that validated extension.capabilities.attachmentsin its relay heartbeat; session-ingest stores it per connection and projects it into bothaggregateSessions()and thesessions.heartbeatevent; the mobile composer only enables attachments for a remote session whencapabilities.attachments === true, recomputed on every heartbeat (upgrade/downgrade/reconnect/absent) and fail-closed on reconnect. Released CLIs without the capability behave exactly as today (paperclip hidden).Interaction with the known download-key divergence (deliberately not changed)
There is a known, deliberately-unfixed divergence where the existing cloud-agent download presign derives its R2 key from the session-owner userId while upload keys use the message-author userId (they diverge on org/shared sessions). This PR does not change key-derivation semantics anywhere. The new
getAttachmentDownloadUrlderives its key from the caller (the author who uploaded), so it does not reproduce that divergence.Testing
Comprehensive automated coverage, all green:
services/cloud-agent-next: unit (2248) + Workers integration (209) + wrapper bounded-streaming/materialization/per-file-failure (253).services/session-ingest: unit (497) + integration (78) — capability true/false/absent transitions in both DO outputs.apps/webcloud-agent-sdk: 998 tests — capability gate, heartbeat recompute, reconnect reset, send-path gating,send_messagepart ordering.apps/mobile: 1080 tests — classify/deny-list/zero-byte/size, measured-size, failure classifier (terminal vs retryable), chip state/CTA presence/absence, capability gate,buildRemoteAttachmentPartsmime-from-remoteName contract, retryable presign-failure feedback.check:unusedare clean.On-device E2E note: the selection/empty/CTA state logic and all server/SDK/CLI behaviors are covered by the automated suites above. Full interactive on-device E2E of a live agent turn requires an environment with real agent-container provisioning (local dev has no running bot/sandbox for cloud-agent sessions, and the remote-CLI content-read proof needs a real model). This is a local-environment limitation, not a behavioral gap.
Non-goals
No web-app UI changes; no changes to
useCloudAgentAttachmentUpload.tsorlib/bot/attachments.tsbehavior; no change to the cloud-agent{path, files}wire (original filename stays mobile-display-only); no key-derivation change; no limit increase or chunked upload; no org mirror for the download presign. Android shares the same JS paths; E2E focus is iOS this round.