feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat: let agents work with PDFs, ZIPs, and other files - #8092

Closed
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads
Closed

feat: let agents work with PDFs, ZIPs, and other files#8092
t3dotgg wants to merge 14 commits into
mainfrom
t3code/generic-file-uploads

Conversation

@t3dotgg

@t3dotggt3dotgg commented Aug 24, 2026

Copy link
Copy Markdown
Member

T3 Code only accepted image attachments. PDFs, ZIP archives, and large text files had no direct route to an agent.

Web, desktop, and mobile can now upload files up to 50 MB directly to their environment. Agents receive the saved file path, images keep their existing behavior, and mobile supports the system file picker and share sheet.

Uploads stream to disk, pending files reuse hard links, and downloaded documents cannot run as pages.

The mobile share-sheet change requires a new iOS and Android store build before later OTA updates work.

Built with GPT-5.6 Sol in the Codex harness.


Note

High Risk
Changes attachment ingestion, local file lifecycle, outbox delivery, and server upload paths—failures can lose shares, leak disk, or send without proper uploads; share-sheet config needs a store build.

Overview
Mobile now treats composer attachments as images or generic files, not images only. Thread and new-task composers can pick files (when the connected server advertises fileAttachments), show file thumbnails in the attachment strip, and offer Photos vs Files from the + control.

Files are copied into app-owned storage (with size checks for Android content:// URIs) instead of being read as base64. Sending uploads file bytes through signed upload URLs, stores pending attachment IDs on drafts/outbox messages for retries, and deletes pending server uploads after a successful turn. The outbox drain uploads before startTurn, rejects or restores undeliverable messages to the composer when uploads fail or the server lacks file support, and cleans up local files only when no draft or queued message still references them.

System share ingestion accepts file/audio/video payloads (broader Android MIME types and iOS file activation), persists shared files on disk, rolls back persisted files if the durable inbox write fails, and filters attachments by the destination server’s file limit when importing into a project draft.

Thread feed renders non-image attachments as tappable rows that open a signed asset URL. Server (in this slice) marks resolved attachment assets for download disposition when the attachment id carries a file extension.

Requires a new native build for expanded share-sheet / MIME registration in app.config.ts before OTA can rely on those targets.

Reviewed by Cursor Bugbot for commit 75199da. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add support for generic file attachments (PDFs, ZIPs) across mobile, web, and server

  • Introduces ChatFileAttachment and PROVIDER_SEND_TURN_MAX_FILE_BYTES (50 MB) in orchestration.ts; environment descriptors now advertise fileAttachments.maxUploadBytes
  • Server streams file uploads instead of buffering them, embeds normalized file extensions in attachment IDs via attachmentFileExtension, and serves downloadable assets with Content-Disposition: attachment headers in http.ts
  • Web composer accepts files via button and paste in ChatComposer.tsx, uploads through the server queue, stashes/restores file references, and renders download links in MessagesTimeline.tsx
  • Mobile picks and persists files to app-owned storage in composerImages.ts, supports incoming file shares (including Android text+file intents via an expo-sharing patch), and uploads/restores file attachments during outbox drain in use-thread-outbox-drain.ts
  • Risk: makeProviderService.sendTurn in ProviderService.ts now filters attachments to images only and appends file paths to the text prompt; normalizeDispatchCommand in Normalizer.ts prefers hard links over copies when claiming uploads — verify provider adapters and filesystem permissions handle these correctly

Macroscope summarized 75199da.

@coderabbitai

coderabbitaiBot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5745e26-94bb-4d07-8515-e1a5cdc7d93f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 24, 2026
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.3 KiB+41 B (+0.3%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+2 B (+0.0%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.4 KiB+39 B (+0.6%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB55.6 KiB+44 B (+0.1%)66.4 KiB
CodexLive turn messages1011+1 (+10.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB−3 B (−0.0%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−4 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+1 B (+0.0%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB0 B (0.0%)66.4 KiB
ClaudeLive turn messages11110 (0.0%)21

Baseline: e67074f · PR result: 75199da · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/composerDraftStore.ts Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/server/src/assets/AttachmentUpload.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/web/src/composerDraftStore.ts
Comment threadapps/mobile/src/features/sharing/incoming-share-model.ts
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
@macroscopeapp

macroscopeappBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This introduces a broad cross-platform file-attachment workflow, including native sharing, local persistence, streaming uploads, server-side storage, provider access, outbox recovery, and downloads. Its scope and runtime impact extend well beyond a small isolated change and should receive human review.

No code changes detected at 75199da. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One UI consistency finding in the new web composer file attachment row. Everything else in the changed web scope (the paperclip Button size="icon-sm" + Tooltip/TooltipTrigger render composition, the icon-xs retry/remove actions, the timeline download anchor, and the semantic color tokens used) matches the existing composer/timeline conventions.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/mobile/src/features/threads/new-task-flow-provider.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding in the changed web UI: the new attachment download link inside the user message bubble hovers to text-primary, which is the solid-control fill role rather than a foreground role owned by the message surface. Details inline. (The unbounded retry tooltip in ChatComposer.tsx file rows from the earlier review is still open; not re-posting it.)

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/components/chat/ChatComposer.tsx
Comment threadapps/web/src/composerDraftStore.ts Outdated

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One new finding in scope, plus one carried-over finding that is still unresolved.

New: the stashed-file count badge in ComposerStashMenu does not pin a type scale or an icon tone, so it renders larger than every sibling metadata span in the row and its icon is re-colored by the CommandItem primitive. Inline comment below.

Still open from a previous run (not re-posted):apps/web/src/components/chat/MessagesTimeline.tsx:1075 — the file download link on the user message bubble takes its hover color from --primary, which index.css defines as the solid-control fill role rather than a text role owned by the bg-message surface, so the hover state is not guaranteed to stay legible in themed palettes. A message-surface-owned foreground (e.g. the existing text-message-foreground with an opacity/underline hover) keeps the hover state under the surface that renders it.

Everything else in the changed web UI looks consistent: the new composer attach control uses Button variant="ghost" size="icon-sm" inside the standard Tooltip/TooltipTrigger render composition, the failed-upload tooltip now matches the image tile's max-w-64 whitespace-normal leading-tight cap, and the attach affordance is correctly gated out of the collapsed-mobile and approval footers.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI consistency review of the changed web files. Two findings, both about draft/stash surfaces that summarize composer attachments and were not updated for the new files list. The previously flagged message-link hover token and the uncapped retry tooltip in ChatComposer are both resolved in this revision.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/ComposerStashMenu.tsx Outdated
Comment threadapps/web/src/composerDraftStore.ts
@baptisteArno

Copy link
Copy Markdown

YES

@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from d014010 to ae9756aCompareAugust 25, 2026 10:33

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consistency issue found in the new user-message file attachment rows. Everything else (composer file list, stash menu badge, sidebar draft count, paperclip trigger) follows the shared Button/Tooltip contracts.

Posted via Macroscope — UI Consistency

Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/features/threads/use-project-actions.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-composer-drafts.ts
Comment threadapps/mobile/app.config.ts
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/mobile/src/state/use-composer-drafts.ts Outdated
Comment threadapps/mobile/src/lib/attachmentUpload.ts Outdated
Comment threadapps/mobile/src/state/use-thread-composer-state.ts
Comment threadpackages/contracts/src/orchestration.ts
Comment threadapps/mobile/src/state/use-thread-composer-state.ts Outdated
Comment threadapps/web/src/lib/attachmentUploadQueue.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/features/threads/use-project-actions.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/features/threads/NewTaskDraftScreen.tsx
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/web/src/components/ChatView.tsx
Comment threadapps/mobile/src/features/sharing/IncomingShareProvider.tsx
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
Comment threadapps/mobile/src/state/thread-outbox-manager.ts
Comment threadpatches/expo-sharing@56.0.18.patch Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts Outdated
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/state/use-thread-outbox-drain.ts
Comment threadapps/mobile/src/lib/composerImages.ts Outdated
@t3dotgg
t3dotggforce-pushed the t3code/generic-file-uploads branch from 4a4d660 to 75199daCompareAugust 25, 2026 18:42

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

!currentMessages().some((candidate) => candidate === expectedMessage)
) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale outbox write wins CAS race

Medium Severity

Conditional update writes the new payload to durable storage before re-checking expectedMessage. enqueue can replace that message in memory during the write, so the CAS fails and skips the in-memory publish while leaving the stale payload on disk. A crash or failed enqueue write can reload that stale version and drop concurrent edits.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

warnings.push(error instanceof Error ? error.message : `Could not read '${name}'.`);
} finally {
await releaseOwnedFiles(input.fileReader, [uri, payload.value]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Share import leaks persisted files

Medium Severity

If persistFile succeeds and a later step throws, the catch path never releases persistedFileUri. finally only drops the original share URIs, and a successful inbox write does not run rollback, so the copied file can remain under app storage even though it was never attached.

Additional Locations (1)
Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit 75199da. Configure here.

const uris = new Set<string>();
for (const payload of payloads) {
if (payload.shareType === "image") {
if (["image", "file", "audio", "video"].includes(payload.shareType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Criticalsharing/IncomingShareProvider.tsx:112

Replay cleanup deletes arbitrary sender-owned file:// files, not just T3 temporary files. The generic branch adds every file, audio, and video URI to the deletion set, while removeOwnedFile treats any file: URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/sharing/IncomingShareProvider.tsx around line 112:
Replay cleanup deletes arbitrary sender-owned `file://` files, not just T3 temporary files. The generic branch adds every `file`, `audio`, and `video` URI to the deletion set, while `removeOwnedFile` treats any `file:` URI as deletable; restrict cleanup to known app/share-owned paths or copied destinations.
Evidence trail:
Reviewed commit 75199da: `apps/mobile/src/features/sharing/IncomingShareProvider.tsx:94-129,142-183`; `apps/mobile/src/features/sharing/incoming-share-inbox.ts:89-99,117-130`; `apps/mobile/src/lib/composerImages.ts:49-111`. Expo SDK 56: https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/dataParsers/SimpleShareIntentDataParser.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-sharing/android/src/main/java/expo/modules/sharing/SharingModule.kt ; https://github.com/expo/expo/blob/sdk-56/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemPath.kt. Verification command: `git show 75199da -- apps/mobile/src/features/sharing/IncomingShareProvider.tsx`.

return completeDelivery(deliveryResult);
const delivered = await completeDelivery(deliveryResult);
if (delivered) {
await deletePendingMobileAttachments(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumstate/use-thread-outbox-drain.ts:523

useThreadOutboxDrain removes the queued message and treats cleanup as successful even when attachmentsDelete fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. deletePendingMobileAttachments currently ignores the failure result from each runAtomCommand, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 523:
`useThreadOutboxDrain` removes the queued message and treats cleanup as successful even when `attachmentsDelete` fails, so uploaded attachments become orphaned on disconnect and repeated sends can accumulate unowned files. `deletePendingMobileAttachments` currently ignores the failure result from each `runAtomCommand`, and no retry is scheduled after this call; propagate cleanup failures and retain or retry the pending attachment IDs until deletion succeeds.
Evidence trail:
Reviewed commit 75199daf: apps/mobile/src/lib/attachmentUpload.ts:50-64; apps/mobile/src/state/use-thread-outbox-drain.ts:404-423, 521-528, 758-775; packages/client-runtime/src/state/runtime.ts:279-289; apps/mobile/src/state/attachments.ts:11-14

environmentId: input.project.environmentId,
attachments: input.initialAttachments,
});
if (uploaded.pendingAttachmentIds.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/use-project-actions.ts:74

When onAttachmentsUploaded fails (for example, flushComposerDrafts rejects), the send is reported as failed but the files already uploaded by uploadMobileAttachments are not deleted. Because this callback runs inside the try, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete uploaded.pendingAttachmentIds in this failure path (while preserving the existing cleanup for upload failures).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/use-project-actions.ts around line 74:
When `onAttachmentsUploaded` fails (for example, `flushComposerDrafts` rejects), the send is reported as failed but the files already uploaded by `uploadMobileAttachments` are not deleted. Because this callback runs inside the `try`, the catch at line 83 bypasses cleanup and leaves the uploaded files unreferenced on the server, potentially leaking up to 50 MB per failed attempt until stale-upload sweeping. Delete `uploaded.pendingAttachmentIds` in this failure path (while preserving the existing cleanup for upload failures).
Evidence trail:
Commit 75199daf: apps/mobile/src/features/threads/use-project-actions.ts:68-87,118-123; apps/mobile/src/features/threads/NewTaskDraftScreen.tsx:788-802; apps/mobile/src/state/use-composer-drafts.ts:218-230; apps/mobile/src/lib/attachmentUpload.ts:50-64,141-192; apps/server/src/assets/AttachmentUpload.ts:227-245; apps/server/src/attachmentStore.ts:205-245. URL: https://github.com/pingdotgg/t3code/blob/75199daf/apps/mobile/src/features/threads/use-project-actions.ts#L68-L87. Commands: git show --stat 75199daf; git diff MERGE_BASE REVIEWED_COMMIT -- apps/mobile/src/features/threads/use-project-actions.ts

Comment on lines +184 to +185
yield* fileSystem.link(claim.currentPath, claim.finalPath).pipe(
Effect.catch(() => fileSystem.copyFile(claim.currentPath, claim.finalPath)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highorchestration/Normalizer.ts:184

A provider edit to claim.finalPath also mutates the retry source at claim.currentPath, so a later retry uploads the agent-modified bytes instead of the original attachment. fileSystem.link gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.

Suggested change
yield*fileSystem.link(claim.currentPath,claim.finalPath).pipe(
Effect.catch(()=>fileSystem.copyFile(claim.currentPath,claim.finalPath)),
yield*fileSystem.copyFile(claim.currentPath,claim.finalPath).pipe(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Normalizer.ts around lines 184-185:
A provider edit to `claim.finalPath` also mutates the retry source at `claim.currentPath`, so a later retry uploads the agent-modified bytes instead of the original attachment. `fileSystem.link` gives both paths the same inode; use a byte-for-byte copy here to keep the pending upload immutable.
Evidence trail:
Reviewed commit 75199daf. Inspect: `apps/server/src/orchestration/Normalizer.ts:141-185,269-296`; `apps/server/src/orchestration/Normalizer.attachments.test.ts:76-101,169-200`; `apps/server/src/provider/Layers/ProviderService.ts:732-744`; `apps/server/src/orchestration/http.ts:96-104`; `apps/server/src/ws.ts:1189-1191`. Verification commands: `git show 75199daf -- apps/server/src/orchestration/Normalizer.ts`; `git grep -n "cleanupFailedUploadedAttachments\|Attached .*saved at" 75199daf -- apps/server/src`.

className="aspect-[1.3] w-full rounded-[14px] bg-white/15"
onPressImage={props.onPressImage}
/>
) : (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumthreads/ThreadFeed.tsx:1163

Opening a generic attachment through tryOpenExternalUrl drops attachment.name, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via Content-Disposition.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1163:
Opening a generic attachment through `tryOpenExternalUrl` drops `attachment.name`, so mobile download targets save the document using the opaque ID-derived URL filename instead of the name shown in chat. Use a download path that supplies the original filename or return it via `Content-Disposition`.
Evidence trail:
Commit 75199daf
- apps/mobile/src/features/threads/ThreadFeed.tsx:211-264, 1160-1170
- apps/server/src/assets/AssetAccess.ts:282-300, 424-430, 452-475
- apps/server/src/http.ts:53-74, 230-241
- apps/server/src/server.test.ts:4533-4558
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Disposition
- https://httpwg.org/specs/rfc6266.html

attachments: draft.persistedAttachments,
...(draft.files.length > 0
? {
files: draft.files.flatMap((file) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumsrc/composerDraftStore.ts:1962

Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This flatMap emits no entry until both uploadedAttachmentId and uploadEnvironmentId exist, while the draft still contains the local File; persist a recoverable pending-file representation or otherwise retain it across hydration.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1962:
Pending files are silently dropped from persisted drafts, so reloading or navigating away before their upload completes permanently loses the attachment. This `flatMap` emits no entry until both `uploadedAttachmentId` and `uploadEnvironmentId` exist, while the draft still contains the local `File`; persist a recoverable pending-file representation or otherwise retain it across hydration.
Evidence trail:
Commit 75199da. Inspect apps/web/src/composerDraftStore.ts:1962-1975, 2258-2268, 3759-3786; apps/web/src/components/chat/ChatComposer.tsx:835-845; apps/web/src/lib/attachmentUploadQueue.ts:180-253.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

returndeliveryAction==="remove"

The deliveryAction === "remove" path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls removeQueuedMessage directly instead of completeDelivery; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-outbox-drain.ts around line 748:
The `deliveryAction === "remove"` path removes file-bearing queue entries without deleting their pending server uploads or releasing their local attachment files, so abandoned attachments remain orphaned. This path calls `removeQueuedMessage` directly instead of `completeDelivery`; route removal through the same attachment cleanup (including persisted upload IDs) before or after deleting the queue entry.
Evidence trail:
Reviewed commit 75199daf. `apps/mobile/src/state/use-thread-outbox-drain.ts:404-414, 521-526, 597-603, 636-757`; `apps/mobile/src/state/thread-outbox-model.ts:149-172`; `apps/mobile/src/lib/attachmentUpload.ts:50-64`; `apps/mobile/src/state/use-composer-drafts.ts:233-264`. Git commands: `git show 75199daf -- apps/mobile/src/state/use-thread-outbox-drain.ts`; `git grep -n "removeThreadOutboxMessage\|deletePendingMobileAttachments\|releaseUnusedComposerAttachmentFiles" 75199daf -- apps/mobile/src`

if (isHeicImageFile(file)) {
return "image";
}
if (!file.type.toLowerCase().startsWith("image/")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumchat/composerAttachmentFiles.ts:13

When file.type is empty, a valid JPEG or PNG is classified as "file" unless its name ends in .heic/.heif, so addComposerAttachments sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining "file" for unrecognized extensions).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 13:
When `file.type` is empty, a valid JPEG or PNG is classified as `"file"` unless its name ends in `.heic`/`.heif`, so `addComposerAttachments` sends it through the generic-file path and vision-capable providers receive only a file path instead of image data. Treat supported image extensions as images when the MIME type is missing (while retaining `"file"` for unrecognized extensions).
Evidence trail:
Reviewed commit 75199da
- apps/web/src/components/chat/composerAttachmentFiles.ts:7-16
- apps/web/src/components/chat/ChatComposer.tsx:2787-2815
- apps/web/src/components/ChatView.tsx:5745-5763
- apps/server/src/provider/Layers/ProviderService.ts:732-758
- apps/server/src/provider/Layers/CodexAdapter.ts:1786-1815

if (!supportsAttachmentUploads) {
for (const image of composerImages) {
releaseAttachmentUpload(image.id);
for (const attachment of [...composerImages, ...composerFiles]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highchat/ChatComposer.tsx:819

When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 819:
When attachment uploads are temporarily unsupported, this loop releases every hydrated file upload, deleting its server-side pending attachment while the file remains in the draft. A reconnect or version-skew transition therefore makes the later upload verification fail and forces the user to reattach the persisted file. Do not release persisted file uploads solely because the current capability snapshot is unsupported; limit this cleanup to attachments whose upload lifecycle is owned by this composer state.
Evidence trail:
Commit 75199daf
- apps/web/src/components/chat/ChatComposer.tsx:814-833
- apps/web/src/composerDraftStore.ts:2247-2268
- apps/web/src/lib/attachmentUploadQueue.ts:112-153, 314-438
- apps/web/src/components/ChatView.tsx:2134-2139, 5702-5750
Verification command: `git show 75199daf -- apps/web/src/components/chat/ChatComposer.tsx apps/web/src/composerDraftStore.ts apps/web/src/lib/attachmentUploadQueue.ts apps/web/src/components/ChatView.tsx`

Comment on lines +110 to +111
await source.copy(destination);
return destination.uri;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumlib/composerImages.ts:110

source.copy(destination) can leave a partial file in t3-composer-attachments when the copy fails, and pickComposerFiles cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes destination before rethrowing the error.

- await source.copy(destination);+ try {+ await source.copy(destination);+ } catch (error) {+ if (destination.exists) {+ destination.delete();+ }+ throw error;+ }
return destination.uri;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/lib/composerImages.ts around lines 110-111:
`source.copy(destination)` can leave a partial file in `t3-composer-attachments` when the copy fails, and `pickComposerFiles` cannot clean it up because no URI is returned. Wrap the copy in cleanup logic that deletes `destination` before rethrowing the error.
Evidence trail:
t3code @ 75199daf: apps/mobile/src/lib/composerImages.ts:49-111, 164-197; apps/mobile/package.json:74-86. Expo FileSystem File.copy API: https://docs.expo.dev/versions/latest/sdk/filesystem/#copy-1. Expo Android implementation dispatches File.copy: https://github.com/expo/expo/blob/main/packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt

@t3dotgg

Copy link
Copy Markdown
MemberAuthor

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

The generic server upload path merged in #8235. #8236 owns the open web work, and #8237 owns the open mobile work. This 79-file bundled predecessor no longer has a clear role, and its remaining edge cases belong in those split branches.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@baptisteArno