Skip to content

fix(platform): say when an attachment is no longer available - #3104

Closed
Israeltheminer wants to merge 1 commit into
mainfrom
fix/attachment-blob-missing
Closed

fix(platform): say when an attachment is no longer available#3104
Israeltheminer wants to merge 1 commit into
mainfrom
fix/attachment-blob-missing

Conversation

@Israeltheminer

Copy link
Copy Markdown
Collaborator

A message that advertises an attachment whose bytes are gone now says so, instead of offering a download that fails with a browser error.

Closes #3017.

Why

materialize_email_attachments.ts builds the download URL at ingest and stores it in the message metadata, so the URL outlives the blob. Nothing notices: the message keeps offering the link, and the failure surfaces as a browser error with no explanation.

Seen on a deployment after a recovery cycle re-imported the database without file storage, so rows came back and blobs did not. The trigger was a one-off, but nothing in the product detects the state, so any lost blob — a restore, a bucket migration, a failed write — leaves the same broken offer.

What changed

transformConversation resolves attachment availability from live truth before projecting messages, and marks a missing one unavailable. The UI then shows "No longer available" in place of the file size and renders no download control.

The URL is withheld along with the flag. Leaving it would let anything holding the attachment offer the broken link regardless of what the flag says.

One deduped pass ahead of the projection, so the projection stays synchronous and a blob referenced by ten messages costs one indexed read.

Risk

It fails open past a cap. ATTACHMENT_CHECK_CAP is 60 distinct blobs per conversation view; beyond it nothing is marked. Wrongly telling someone a file is gone is worse than leaving a link that might 404, so the cap errs toward silence.

The check costs one indexed read per distinct blob. A long conversation pays for that on every view. The dedupe keeps it to distinct blobs rather than per attachment.

An attachment with no storageId is left alone — there is nothing to verify against, so nothing is claimed either way.

A duplication removed on the way

AttachmentCardProps hand-declared its own attachment shape instead of deriving it from the query type, and had already drifted: it was missing contentId, and would have silently ignored this field too. It now derives from the query's own type.

Tests

Three on the server: a missing blob marked and its URL withheld, a present blob untouched, and an attachment with no storageId left alone. Three on the UI: the unavailable text replacing the size, no download control even when a download handler is supplied, and the normal case still offering the download.

Six deliberate breakages, all caught. One survived the first round: the no-button case passed without the guard, because that fixture had no URL and no handler, so no button rendered either way. The fixture now supplies the handler, which is what makes the guard load-bearing — without a URL the chip would otherwise render a "fetch it" button that can only fail.

Locales: en, de, fr.

Scope

Detects a missing blob at read time. It does not clean up the message metadata, and it does not stop the URL being stored at ingest — the stored URL is still the mechanism, this only stops it being served once it cannot work.

Gate: repo-wide typecheck, oxlint --type-aware, oxfmt, knip, SAST 0 findings, platform suite 75,815 passing and the UI suite 3,463 passing.

@Israeltheminer

Copy link
Copy Markdown
Collaborator Author

This is rebasable rather than superseded — six of its eight files still exist on main. Port map, from reading the 0.5 tree:

convex/conversations/transform_conversation.ts and its test are gone. Its 0.5 counterpart is listConversationMessages in backend/domains/conversations/service.ts:371, a plain SELECT that returns each row's metadata verbatim to routes.ts:152. That is where the deduped pre-pass belongs, so the projection stays synchronous and a blob referenced by ten messages costs one check.

The check itself changes substrate. ATTACHMENT_CHECK_CAP, the dedupe and the fail-open past the cap all port as written, but the point read does not: 0.5 has no ctx.storage. Blobs live in object storage, so it becomes s3HeadObject against resolveObjectStore(orgSlug) — the pair retention/service.ts uses. app.file_metadata is the wrong source to check: #3017 is a deployment where the rows came back and the blobs did not, so a row-existence check would report every file as present.

The other four files need no change. validators.ts survives, and the three locale catalogs are untouched by #3107.

One thing worth deciding on the rebase: this adds an object-storage HEAD per distinct attachment per conversation open, where 0.4 paid a Convex storage read. The cap bounds it, but the round trip is now a network call.

@Israeltheminer

Copy link
Copy Markdown
Collaborator Author

Superseded by #3171.

Three of this branch's eight files were deleted by #3125, and the 0.5 shape turned out simpler than the port map I left above. The read path already re-derives each URL through a presign loop that walks every attachment, so the probe is one statOrgBlob call inside that loop — no separate deduped pre-pass, and no cap, because there is no second walk to bound. The deduped-pre-pass design was right for 0.4, where the check sat somewhere else entirely.

The fail-open rule from this branch carries over unchanged: a probe that throws is not evidence of absence.

#3017 stays open until #3171 lands.

@Israeltheminer

Copy link
Copy Markdown
Collaborator Author

Closing so the queue reflects reality — #3171 replaces this and is open. Three of this branch's eight files were deleted by #3125, so it cannot be rebased. #3017 stays open and is closed by #3171.

Israeltheminer added a commit that referenced this pull request Sep 4, 2026
A message that advertises an attachment whose bytes are gone offered a
download that failed with a browser error and no explanation.

Presigning does not prove the object is there — it signs a path. So the
read path already re-derived each URL and dropped it when the presign
itself failed, but a presign for a missing object succeeds, and the chip
went on showing a file size and a download button.

The probe rides the presign loop that already walks each attachment, using
the existing statOrgBlob rather than a second pass over the same metadata.
A probe that THROWS is not evidence of absence — an unreachable store, an
unresolved org — so it fails open and the attachment is offered as before.
Only a definite miss marks it: url withheld, unavailable stamped, and the
chip reads 'No longer available' where the size was.

Closes #3017. Supersedes #3104, three of whose files #3125 deleted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: a message advertises an attachment whose blob is gone

1 participant