Skip to content

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

Open
Israeltheminer wants to merge 1 commit into
mainfrom
fix/3017-attachment-unavailable
Open

fix(platform): say when an email attachment is no longer available#3171
Israeltheminer wants to merge 1 commit into
mainfrom
fix/3017-attachment-unavailable

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 and no explanation.

Closes #3017. Supersedes #3104, three of whose files #3125 deleted.

Why

The read path already re-derives each attachment URL rather than trusting the one stored at ingest, and drops it when the presign fails. That is not enough: presigning does not prove the object is there — it signs a path. For a missing object the presign succeeds, so the chip went on showing a file size and a download button, and the failure surfaced as a browser error.

Seen on a deployment after a recovery cycle re-imported the database without file storage, so rows came back and blobs did not.

What changed

The presence probe rides the presign loop that already walks every attachment, using the existing statOrgBlob — rather than a second pass over the same metadata, which would have been a divergent second copy of the same walk.

A probe that throws is not evidence of absence: an unreachable store, an unresolved org. Those fail open and the attachment is offered exactly as before. Only a definite miss marks it — url withheld, unavailable stamped, and the chip reads "No longer available" where the size was.

Withholding url alongside the flag means nothing downstream can offer the broken link even if it ignores the flag. The download button already keyed on the URL, so it disappears without a UI change.

unavailable is carried through the shared conversation projection the list and detail lanes both use, so the two cannot drift into different answers.

Note on the original design

#3104 built a separate deduped pre-pass with a 60-blob cap and its own fail-open rule. That was the right shape against 0.4, where the check was a Convex storage read in a different place. On 0.5 the presign loop already does the per-attachment object-storage work, so the cap and the second walk are unnecessary — the probe is one call inside a loop that was already running.

Tests

Three UI cases and one integration assertion, and the pairing matters: each asserts the negative and the positive, because a change that hid every attachment would pass the negatives alone.

Mutation Went red
the presence probe removed goneMarked=false (unavailableFlag=false)
the chip's label reverted to the size 1 of the 14 UI cases

The integration fixture caught its own bug first, which is worth recording: the seeded ref was not org-scoped, so requireOrgScopedKey refused it and the presign threw before the probe ever ran. The assertion failed for the wrong reason and looked like a code defect. A comment in the fixture now names that.

Run against a real Postgres and MinIO: 367/373, identical to a baseline run of unmodified main on the same box — the six failures are the object-store warm-bucket collision, two yt-dlp-dependent probes, and three agent-lane probes, all failing the same way without this diff. With the probe removed the count drops to 366, and the one extra failure is this assertion.

@tale/ui i18n suite 1173/1173, so the new key is present in all three locales.

Gate: typecheck, oxlint --type-aware, oxfmt --check, lint:sast green; branched from origin/main at 5f9dc6eb2.

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