Repository navigation
feat(mobile): attachments go where the workspace lives; render cloud parts and path tags - #8852
Closed
superset-app[bot] wants to merge 10 commits into
Closed
superset-app[bot] wants to merge 10 commits into
superset-app[bot] wants to merge 10 commits into
Conversation
…-service bodies (SUPER-2658) Pasting or dropping an image into a cloud workspace failed with "Failed to send the file to the remote workspace" (terminal) or "Couldn't attach files / value$1 is not iterable" (chat). The desktop was shipping the bytes as a base64 `filesystem.writeFile` body over the streaming tRPC link. tRPC answers a streamed batch before it has read the request body, and a Vercel sandbox's ingress treats that early response as the end of the exchange and drops the upload still in flight — a race, not a size limit: a 500 KB body on a home uplink loses it about half the time. Desktop: the chat composer, the terminal rich input, and the terminal paste/drop override now take the route every other composer already takes (mobile, the new-workspace modal): `attachment.createUpload`, a presigned PUT to storage, `cloudWorkspace.attachFiles` for a box, then `attachments.materializeIntoWorkspace` on the host. The host-service request carries ids, never bytes. `uploadPastedFiles` is replaced by `sendAttachmentsToWorkspace` + `useSendAttachments`. host-service: `/trpc/*` buffers the request body before handing it to tRPC so no response can start before the request is complete; the integration test fails without it. `materializeIntoWorkspace` and the chat image writer share one helper that creates `.superset/attachments` with its `.gitignore`, which the former never wrote.
…ap files with the box
First API step of the attachment plan (SUPER-2874, seam 6).
- attachment.resolve no longer signs a URL for any composer upload in the
org. A kept (ready) file resolves only through a cloud workspace the
caller can see. An upload still in flight (pending) stays org-wide: a
host resolves with its owner's credential, and the sender can be a
teammate on that host. A box still sees only its own workspace's files.
- New attachment.resolveEach returns one entry per id, with
{ fileId, missing: true } for a file that is gone or not visible, so a
transcript can render the rest. resolve keeps its output and errors for
released hosts.
- The archive's final reap stage (and the fallback when it cannot be
queued) detaches and deletes the files anchored to the workspace.
…fileId on cloud copies Part of the attachments plan (PR 3: seams 8, 2, and the fileId carry-through of seam 3). The host's attachment store had no retention. Nothing deletes chat.db rows and a terminal launch has no session, so there is no session delete to hook. Instead, metadata.json records lastUsedAt. Upload and import set it, and every path that hands an attachment out sets it again: read, agents.run's attached files block, and the ACP resolver. A daily sweep (not in sandboxes) removes entries unused for ATTACHMENT_RETENTION_DAYS. An entry written before this change is stamped, not removed. The check and the rename that takes the entry out of the store run in the same tick, so a hand-out on this event loop cannot land between them. The slow delete runs async, off the loop. A box's cached copy now keeps the fileId it was imported from, in its metadata and in a small .by-file-id index, so the adapters (next PR) can find a cloud part's cached copy without a scan. The first prompt of a cloud launch spreads that metadata into its parts. Without fileId, the first attachments of a cloud session would be the one kind a reader without the box cannot render. MAX_INLINE_ATTACHMENT_BYTES moves to @superset/shared/attachment-limits with MAX_THUMBNAIL_BYTES and ATTACHMENT_RETENTION_DAYS. The host and the shared launch schema use MAX_ATTACHMENTS for their max(10).
…tachments Part of the attachments plan (PR 4, seam 4). The Claude SDK adapter rendered an attachment part as `[name]`, and the Codex adapter dropped it with a notice. Only ACP resolved it. This is why desktop chat still flattens attachments into path tags in the text. Now all three take one resolver from the host (chat-v3/resolveAttachment.ts). It branches on the identity the part carries. `fileId` is a cloud workspace's storage object: served from the box's cached copy, or imported now when the store is empty, as on a recreated box. `attachmentId` is the host store entry itself. Each hand-out stamps lastUsedAt for the sweep. Bytes are read only for images, since every adapter gives other files by path. - Claude: an image goes as a base64 image block in the SDK's content-array form. Any other file goes as a "# Attached files" line with its name and path, because store files are named by id. - Codex: an image goes as app-server's `localImage` by path. Any other file goes as the same text block. - An unreadable file is named to the agent, not dropped silently. - Resolution is chained per adapter, so prompt order holds. A text-only prompt with nothing queued is still pushed synchronously. The protocol's attachment part now needs `attachmentId` or `fileId` (it needed `attachmentId` before), so a client can send a cloud part by fileId alone. Clients send parts only to hosts at or above ATTACHMENT_PARTS_MIN_HOST_VERSION (1.41.0, the first release with this change). agents.run's attached files block uses the same attachedFilesBlock, so terminal launches also list files by name. A failed resolve reports to Sentry. The host has no PostHog client, so the plan's PostHog event is not sent from here.
… attachments An attachment-only message can carry an empty text part, and the Anthropic API rejects an empty text content block.
…floor Desktop and mobile both decide between attachment parts and path tags from a host's version. One function next to ATTACHMENT_PARTS_MIN_HOST_VERSION keeps the two clients on the same rule. It coerces the version, so a canary or a dev build of a release at or above the floor counts.
…parts and path tags Part of the attachments plan (mobile PR). Mobile uploaded every attachment to our storage when it was attached, and then had the host import it. That also happened for a workspace on a machine someone owns, whose bytes should never reach our servers. Mobile also showed a desktop path tag as raw text. - A host workspace's draft no longer uploads at attach time. At send, chat attachments go to the host as base64 attachments.upload (the part names attachmentId). Terminal attachments are written into the worktree with create-only base64 filesystem.writeFile. Both refuse a file over MAX_INLINE_ATTACHMENT_BYTES. - A cloud chat part carries its fileId and skips importFromCloud when the box reads parts (hostReadsAttachmentParts). An older box still gets the copy it imports. - useAttachmentFile reads a fileId part from storage (resolveEach, then a download into the cache), so it shows without the box. A host part still reads from the host. - Path tags in a message show as file cards. parseAttachmentTags moved from desktop into @superset/chat/core so both clients parse them the same way. - A send that finds an attachment that never started uploading (its draft's home was unknown when it was attached) starts it.
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
|
Preview update published for development build |
Collaborator
|
Folded into #9064, which now carries the whole attachments plan (commit list in its description). Closing in favor of that PR. |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The mobile part of the attachments plan (v6):
fileIdand skip the import call;Why
<attachment path>tags showed as raw text on mobile.How it works
WorkspaceScreentellslib/attachments/upload.tsthe home of its workspace draft once the host resolves (setDraftAttachmentHome). A host draft does not upload at attach time. The home-screen draft (NewChatWidget) is unchanged: its host is not chosen yet when files are attached.lib/attachments/chatParts.ts,chatAttachmentParts):attachments.uploadthrough the relay; the part namesattachmentId. A file overMAX_INLINE_ATTACHMENT_BYTESis refused before anything is sent.hostReadsAttachmentParts(hostVersion)): wait for the uploads,attachFiles, then send parts withfileIdonly. NoimportFromCloud.attachFiles+importFromCloud, so the parts haveattachmentId).attachments_sent/attachments_send_failedto PostHog with surface and home. The failure event usesrawErrorMessage.filesystem.writeFileinto.superset/attachments/(create-only.gitignore, collision-probed names;lib/attachments/worktreeWrite.ts). A box keeps storage +materializeIntoWorkspace.useAttachmentFiletakes asource(fileIdorattachmentId). A cloud part resolves throughattachment.resolveEachandFile.downloadFileAsyncinto the cache, with no box needed. A host part reads throughattachments.readas before. Path tags in a message's text show as file cards (name only, nothing fetched).parseAttachmentTags/formatAttachmentTagmoved from desktop'sChatSession/utils/attachmentTagsinto@superset/chat/core, with their test. Desktop's util index re-exports them, so desktop imports are unchanged.awaitAttachmentUploadsnow starts it. Before, it failed with "no longer available".Not in this PR / differs from the plan
useCreateTerminalWorkspace, home-screen new chat) still uploads at attach time and imports into the chosen host. The host is not known when files are attached, so a host-workspace launch from the home screen still goes through storage. The plan keeps the modal's cloud branch as it is and does not cover this case for mobile. It is a remaining goal-2 gap.tRPC compatibility
attachment.resolveEach(API, fix(desktop,host-service): send attachments through storage, not host-service bodies #8704), and the host'sattachments.upload,workspace.get,filesystem.createDirectoryandfilesystem.writeFile, which exist on every supported host.fileId-only parts go only to boxes at or aboveATTACHMENT_PARTS_MIN_HOST_VERSION.Testing
chatParts.test.ts: a cloud part carriesfileIdand skips the import; an older box gets the imported copy; a host part goes to the host and never to storage; a host refuses a file over the inline cap before sending.worktreeWrite.test.ts: create-only writes into the worktree, with a suffix around an existing name.apps/mobilebun test: 237 pass, 0 fail.packages/chattests pass (97), including the movedattachmentTagstest.apps/mobiletypecheck: no new errors. The tree has the same baseline errors as main, minus one this PR removes. Many of those baseline errors are "implicitly any" from client types that do not resolve in this sandbox, so mobile typecheck is weaker evidence here than usual.apps/desktoptypecheck passes, and the v2-workspace tests (896) pass after the parser move.bun run check:i18npasses, with translations written for all 16 locales. Biome is clean on the changed files.Known limitations