feat: images upload the moment you attach them - #6276
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
There was a problem hiding this comment.
Effect service conventions review of the changed server/contracts code. Two findings on error modeling; the new AttachmentUpload module itself follows the repo's service/DI conventions (namespace subpath imports, dependencies acquired via yield* Foo.Foo, no hidden runtimes).
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
🟡 Medium
t3code/apps/web/src/composerDraftStore.ts
Line 3253 in 1331956
removePreviewAnnotation filters the annotation's image out of images but never calls releaseComposerAttachment or cancelAttachmentUpload. Removing a preview annotation while its image is still uploading leaves the upload job running, and removing one after upload completes leaves the server-side pending attachment allocated until the 30-day backstop sweep. The image disappears from the draft's images array, so the upload queue never sees it again to release or cancel it.
Also found in 1 other location(s)
apps/web/src/components/chat/ChatComposer.tsx:1510
The environment-retargeting path calls
retryAttachmentUploadfor an already-readyimage but never releases its existing server attachment.retryAttachmentUploadonly cancels an active job (there is none after readiness) and starts a new upload, whereasreleaseComposerAttachmentis the function that deletes a ready attachment. Switching a draft between environments therefore leaves one orphaned pending upload behind on each retarget until the 30-day sweep.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 3253:
`removePreviewAnnotation` filters the annotation's image out of `images` but never calls `releaseComposerAttachment` or `cancelAttachmentUpload`. Removing a preview annotation while its image is still uploading leaves the upload job running, and removing one after upload completes leaves the server-side pending attachment allocated until the 30-day backstop sweep. The image disappears from the draft's `images` array, so the upload queue never sees it again to release or cancel it.
Also found in 1 other location(s):
- apps/web/src/components/chat/ChatComposer.tsx:1510 -- The environment-retargeting path calls `retryAttachmentUpload` for an already-`ready` image but never releases its existing server attachment. `retryAttachmentUpload` only cancels an active job (there is none after readiness) and starts a new upload, whereas `releaseComposerAttachment` is the function that deletes a ready attachment. Switching a draft between environments therefore leaves one orphaned pending upload behind on each retarget until the 30-day sweep.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces substantial new capability (upload-on-attach for images) with new server endpoints, client upload queue, contract changes, and storage schema migrations. The scope exceeds what can be auto-approved, and there is an unresolved High-severity finding about stalled uploads potentially blocking sends indefinitely. You can customize Macroscope's approvability policy. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
Attachments used to ride the turn-start command as base64 data URLs: send carried the bytes, the stash re-encoded them into localStorage, and a 10MB image meant a 14M-char string in a single ws frame. Images now upload the moment they are attached. A ws RPC mints a pending-<uuid> id plus a signed, expiring upload URL (mirroring signed asset GETs, so it works against any environment); the browser POSTs the compressed bytes to it with progress and abort; the turn-start command carries id references only. The Normalizer renames pending files to their thread segment at send, resolving by uuid so retries after a partial send are idempotent. Never-sent uploads are deleted on chip removal and swept after 30 days. Breaking: the dataUrl upload variant is deleted with no compatibility path. RN mobile compiles with image attach gated off behind IMAGE_ATTACH_ENABLED and tells the user an update is needed; drafts and stash persist id references (v9/v3 storage, old payloads purged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real bugs from Macroscope and Bugbot, all verified against source: - awaitAttachmentUploads consulted only live jobs, so an upload that settled before the await lost its result and a ready image could drop from a pick-and-send message. Terminal states are now kept in a settled map that cancel/release clean up. - Stashing cancelled in-flight uploads before the localStorage write was confirmed; a quota failure stranded chips in uploading forever. Cancellation now happens only after the write lands. - Retargeting a draft to another environment overwrote a reload-restored attachment's ready state with a dead failed state. The mismatch is now derived (never written), so switching back recovers the attachment, and re-uploads release the old environment's bytes first. - The turn-start failure path restored chips from the pre-await snapshot, resurrecting uploading states no job would ever advance. - The "images were not attached" toast could fire when no message was sent; it now waits for the turn to actually start. - Removing a preview annotation or discarding a draft leaked the upload and its server-side bytes; both now release like chip removal. - Mobile paste drops now surface the needs-an-update banner instead of a console.warn. - Normalizer error mappings keep their PlatformError causes; the dead unstructured AttachmentUploadRequestError is deleted; the sweep test no longer reads the real clock (effect-diagnostics CI failure). Also folds the rebase onto main: the new moveComposerPromptAndImages draft-carry now merges the unified image list, and in-flight uploads are retargeted to the destination draft so moved chips keep their progress. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1331956 to
03196ef
Compare
| // moment they were attached. The composer blocks sending while an upload | ||
| // is in flight, but the preview "pick and send" gesture attaches and sends | ||
| // in one step, so wait for anything still running here. | ||
| const settledUploads = await awaitAttachmentUploads( |
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:5207
A stalled preview upload leaves the send operation pending indefinitely, so sendInFlightRef stays true and the message is neither sent nor restored. awaitAttachmentUploads waits on an XMLHttpRequest with the default timeout of 0, which never expires when the connection remains open; add an upload timeout or cancellation path before awaiting it.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatView.tsx around line 5207:
A stalled preview upload leaves the send operation pending indefinitely, so `sendInFlightRef` stays `true` and the message is neither sent nor restored. `awaitAttachmentUploads` waits on an `XMLHttpRequest` with the default `timeout` of `0`, which never expires when the connection remains open; add an upload timeout or cancellation path before awaiting it.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 03196ef. Configure here.
- Unique per-request .part suffix so concurrent POSTs of one token cannot interleave into a shared temp file. - xhr.timeout on uploads: a stalled POST now fails into the retryable state instead of blocking send (and pick-and-send) forever. - Cross-environment re-upload keeps the old environment's copy until the new upload succeeds (supersedes), so a failed re-upload never destroys the only server copy. - The plan follow-up branch releases attached images before clearing the composer instead of orphaning their pending files. - Mobile: a mixed text+image clipboard still pastes its text while attach is disabled, and the image drop is surfaced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Non-blocking architecture follow-up for Prompt Stash: this PR solves the hard attachment half by moving image bytes onto the target T3 environment, but I think the clean follow-up is to make the target ExecutionEnvironment/server authoritative for stash state, rather than putting canonical stash state in T3 Connect or a thread snapshot:
T3 Connect then remains transport to the execution environment instead of becoming a second state authority. I would keep this out of this PR because #6276 is already a large/high-risk attachment-contract change. The server-owned stash is a cleaner follow-up PR once this attachment model is stable. |
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing in favor of #8048. Image attachments now upload before sending, with compatibility preserved for existing mobile clients and older servers. |

Attached images used to ride inside the send command as base64 data URLs. Sending a message with a 10MB screenshot meant a 14M-character string in one ws frame: send felt slow, the turn could not start until the frame landed, and the stash had to re-encode every image to squeeze it into localStorage.
Now the bytes move while you are still typing. Attaching an image mints a
pending-<uuid>id plus a signed upload URL over ws (the same pattern as signed asset GETs, so it works against any environment), and the browser POSTs the compressed bytes to it with per-chip progress and cancel. The send command carries id references only, and the server renames pending files to their thread at turn start — resolving by uuid, so send retries and existing asset URLs survive the rename.What this buys:
.partthen rename, so a partial transfer is never a valid attachment, and the route enforces the exact minted byte count.Breaking, deliberately without a compatibility path: the dataUrl upload variant is deleted. Out-of-date clients get a loud schema error. React Native mobile compiles and degrades honestly behind
IMAGE_ATTACH_ENABLED = false("Image attach needs an app update"); the port is a fast-follow.apps/swift-iosis not on main, so the Swift mirror is untouched — a 7-edit patch list for that branch is in the plan doc.Design doc: https://rztz3kvilrh0.postplan.dev
Verified: contracts 227, server attachment suites 66 (full suite green except 6 launcher-version failures that reproduce on untouched main), web 2201, mobile 654. Lint and format clean. Not yet done: a manual browser pass (paste several images fast, cancel mid-flight, kill the network, reload with a ready draft) — automated browsers cannot see authenticated views here.
Built by Claude Fable 5 running in Claude Code.
🤖 Generated with Claude Code
Note
High Risk
Breaking attachment contract (no dataUrl), new signed upload HTTP surface and turn-start claim logic, plus draft v9 migration that drops unsent inline images on upgrade.
Overview
Composer images no longer ride in
thread.turn.startas base64 data URLs. Web uploads bytes as soon as a chip is added: WS mints apending-<uuid>id and signed URL, HTTP POST stores the file, and send carries id references only. The normalizer claims pending files into thread scope at turn start (uuid-stable resolution so retries and asset URLs survive renames).Server additions:
AttachmentUpload(token mint/validate,.partthen rename, size enforcement),POST /api/attachments/upload/*, RPCattachments.createUploadUrl/attachments.delete, pending attachment sweep on startup, andplanAttachmentClaimin the attachment store.Web composer/drafts: Upload queue with progress, cancel, retry, and environment mismatch handling; send is blocked while uploads are unsettled; draft persistence bumps to v9 (server id +
environmentIdonly—legacy inlinedataUrlattachments are dropped on rehydrate). Stash and sidebar discard release pending server bytes.Mobile: Image attach is disabled behind
IMAGE_ATTACH_ENABLED = false; picks/pastes surface “needs an app update,” legacy draft/outbox images are not sent (warnings viadroppedAttachmentsWarning), andbuildProjectThreadStartTurnInputalways sendsattachments: [].Reviewed by Cursor Bugbot for commit 36f6d76. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Upload composer image attachments immediately on attach instead of at send time
attachments.createUploadUrlandattachments.delete) mint signed upload URLs and delete pending attachments; the server validates tokens, enforces exact size, and persists bytes via a new POST endpoint.ClientThreadTurnStartCommandpayloads no longer inline base64 image data URLs; they carryChatAttachmentid references to pre-uploaded blobs. The composer send button is blocked while uploads are pending, failed, or belong to a different environment.File; discarding or removing an image aborts in-flight uploads and releases server-side bytes.ready(fully uploaded) attachments are persisted as environment-scoped id references; in-flight and failed uploads are not saved. Legacy v8 inlinedataUrlentries are dropped on rehydration.IMAGE_ATTACH_ENABLED = falsein composerImages.ts; callers receive a structured error and a warning banner when legacy attachments are dropped.Macroscope summarized 36f6d76.