feat(web): upload image attachments before sending - #8048
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 |
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. |
There was a problem hiding this comment.
UI consistency review: one finding on the new upload-progress overlay in the composer attachment thumbnails.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Two new retry actions convey the upload failure reason through the native title attribute instead of the shared Tooltip primitive. The repo enforces t3code/no-native-title-tooltip ("Use Tooltip + TooltipTrigger + TooltipPopup from components/ui/tooltip") — the rule only skips these because it ignores capitalized components, and Button forwards title straight onto the native <button>. Since upload.reason is the only place the specific failure ("Unsupported image type", "Upload rejected (500)", "Not connected") is surfaced, a native tooltip makes it unreachable on touch and keyboard focus, and it looks unlike the adjacent Tooltip in the same composer block.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a604df5. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a full signed image-upload workflow with new HTTP and WebSocket APIs, authorization changes, filesystem persistence, cleanup, and client-side upload state management. Its broad cross-layer runtime impact and security-sensitive upload path warrant human review. You can add or adjust custom eligibility rules. Learn more. |
## What's Changed * fix(web): clarify terminal sidebar grouping by @StiensWout in pingdotgg/t3code#7967 * fix(codex): show app access approval prompts by @t3dotgg in pingdotgg/t3code#8058 * feat(web): upload image attachments before sending by @t3dotgg in pingdotgg/t3code#8048 **Full Changelog**: pingdotgg/t3code@v0.0.34-nightly.20260824.1174...v0.0.34-nightly.20260824.1175 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.34-nightly.20260824.1175
Range 2274444..f035a0f. 80 files touched by both sides; 20 conflicted, 33 hunks. Four of upstream's commits landed in subsystems this fork has rewritten, and three of the conflicts were git splicing two unrelated blocks onto shared scaffolding. Competing implementations, resolved toward a hybrid superset: - pingdotgg#5891 (stop kills lingering Claude work) rewrote `interruptTurn` into a single hard `stopSessionInternal`. This fork's Stop is a two-rung client ladder (cooperative interrupt, then a deliberate second press escalating to `session.stop`), so collapsing the rungs would make that band vestigial for Claude and charge every stop-to-redirect a cold restart. Kept the fork's cooperative `interruptTurn` (bounded `stopTask` sweep + bounded `query.interrupt()`); took upstream's hardened `stopSessionInternal` (`close()` at the very top, `task.completed` sweep over `liveTaskIds`, `stopSessions` failure collection) and kept the fork's bounded `Fiber.interrupt` on top. Upstream removed `ClaudeQueryRuntime.interrupt`/`.stopTask` OUTSIDE any conflict marker; both restored. Upstream's four new tests retargeted to `stopSession`, not deleted. - pingdotgg#7412 (recover from provider interrupt failures) adopted on top of the fork's live-session gate, which settles to `stopped` rather than erroring when nothing is live. Its three tests only projected a session, so each needed a `runtimeSessions.push` to reach the path it tests. - pingdotgg#7969 (settled pinned threads) extracted the pin markup into `pinIndicator` byte-identically; took the extraction, kept `ThreadProviderChip`. - pingdotgg#7949 (Codex feedback) added a send-abort condition; folded into the fork's `shouldAbortSendBeforeOfflineQueue` helper so every abort reason stays in one tested place, with a case for it. - pingdotgg#8048 (attachment uploads) and pingdotgg#7999/pingdotgg#7971 merged additively; the offline-outbox branch stays ahead of the new feedback branch in `handleSend`, since it must run before anything mutates the draft. Three conflicts were splices, resolved by reset-and-reapply rather than hunk surgery: `ClaudeAdapter.test.ts` and `ProviderCommandReactor.test.ts` (two unrelated tests sharing `makeHarness()` scaffolding) and `http.ts` (two route layers sharing an `Effect.gen` preamble; `attachmentUploadRouteLayer` re-added whole). Invariants: all ten re-probed against the merged tree and holding. Migrations 47 entries, ids unique and monotonic, max 48, 34 still burned; patches now 16 files with exactly one fork-owned and 15 byte-identical to upstream (upstream added `@legendapp/list`); `Sidebar.tsx` still the default; the three MessagesTimeline helpers, the Collapsible wrapper, the steering test and `serverRuntimeStartup.reconcile.test.ts` all still absent; four `isSendBlocked=` sites; both footer panels inside `SidebarUtilityMenu`; both project-row entry points. Registry gains invariants 7-9 for the decisions above. Sweeps: resurrected 1, dropped 11, fork-loss 17, every entry named. The one resurrection is a text-level false positive (a bare `if (` the fork restructured, matching upstream's new attachment block verbatim). Two real findings the sweep caught that the gate could not: the fork's deliberate deletion of `shouldShowBranchMismatchBanner` was re-imported in two files by an additive resolution, and `ChatMarkdown.tsx` carried a `markdown-file-link-tooltip-scroll` class that was never defined in any stylesheet — a pre-existing fork defect, so the tooltip's thin scrollbar had silently fallen back to the browser default. Restored upstream's working inline utilities. Gate (`pnpm verify`, the fork's only gate — CI never runs here): format clean, typecheck 0 errors, lint 23 warnings and 0 errors (all pre-existing), tests 9873 passed / 7 skipped / 0 failed across 14 projects. Three failures were fixed en route: the third pingdotgg#7412 test above, an invalid `status: "starting"` in the runtime-session fixture it needed, and upstream's `entrypoint.test.ts` symlink case, which cannot pass on macOS because `os.tmpdir()` is itself a symlink that `realpathSync` resolves along with the fixture's own link — proven with a live probe, fixed in the fixture only, and worth sending upstream. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Image attachments currently travel inside turn-start WebSocket messages as large base64 strings. The earlier direct-upload proposal would also break released mobile apps and older remote servers.
Images now upload over signed HTTP URLs as soon as they are attached. Existing mobile clients keep their current inline attachment format, and newer web clients fall back to inline uploads when an older server does not support direct uploads. Existing drafts and stashed prompts are preserved.
Based on #6276.
Verified with focused contract, server, web, mobile, and client-runtime tests, all 129 server router tests, the production web build, and targeted type checks.
Built by GPT-5.6 Sol in the Codex harness.
Note
High Risk
Adds a signed-token HTTP upload path and filesystem persistence for pending attachments, plus claim/copy logic during turn dispatch. Token validation, size checks, and pending-only deletes are security-sensitive.
Overview
Composer images now upload immediately over signed HTTP URLs instead of riding as base64 in the turn-start WebSocket payload. Newer web clients wait for uploads (with progress, retry, and send blocking) and send attachment ids; older servers and mobile clients keep the inline
dataUrlpath via an optionalattachmentUploadscapability.The server issues HMAC-signed upload URLs, accepts POST bytes at
/api/attachments/upload/*, stores them aspending-*files, and claims them on dispatch by copying to a new thread-scoped id so failed bootstraps can retry. Stale pending (24h) and.part(1h) files are swept on startup and periodically; delete RPCs only remove pending uploads.Thread ids that sanitize to
pendingare remapped to_pendingso they cannot collide with the reserved pending segment.Reviewed by Cursor Bugbot for commit 2718e15. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Upload image attachments before sending messages via signed URLs
/api/attachments/upload, atomic.part-based file storage, and stale upload sweeping (1h for partials, 24h for pending) at startup and URL issuanceNormalizer.normalizeDispatchCommandto claim pending uploads into thread-owned final paths with size/type validation, rollback of claimed copies on failure, and cleanup of claimed attachments on dispatch errorsChatComposerandChatViewto start uploads on image add, display per-image progress and retry UI, use uploaded attachment metadata instead of inline data URLs, and release uploads after successful turn starttoSafeThreadAttachmentSegmentnow maps sanitized thread IDs equal topendingto_pending; any existing thread whose ID sanitizes topendingwill have its attachment directory path change.ClientThreadTurnStartCommandschema now accepts a union ofUploadChatAttachmentandChatAttachmentfor message attachments.Macroscope summarized 2718e15.