Wait for an upload's URL to expire before deleting its bytes

[SunkenInTime]

Closes the race noted in #184: convex/images.ts could delete an upload intent while a presigned PUT against it was still in flight.

Bug
generateUploadUrl creates a pending imageAssets row and hands the client a PUT URL that works for 15 minutes (R2_UPLOAD_URL_EXPIRES_SECONDS, up to 7 days). If the asset is marked deleted while that PUT is running (the strategy is deleted, the image is removed and reclaimed, or the stale-upload cron fires), a sweep is scheduled right away:

1. The sweep sends DELETE for the object key. Nothing is there yet.
2. finalizeDeletedImageAsset deletes the row.
3. The PUT lands. The object now sits in R2 with no row naming it, and nothing ever reclaims it.

Users don't see this directly. It leaks storage, and the bytes of an image the user deleted stay at their public URL.

Fix

• Intents record uploadUrlExpiresAt (new optional field on imageAssets). The URL is signed with the intent's own timestamp, so the stored expiry and the URL's real expiry are the same number.
• claimDeletedImageAssets, which every physical deletion goes through, skips an asset until uploadUrlExpiresAt + 24 hours. R2 only checks the signature when the request starts, so a PUT that started just before expiry can keep sending for as long as a slow connection takes (a 15 MB image at 12 KB/s needs about 20 minutes). A day covers any real upload. Waiting costs little: a deleted element's image already stays for its 30-day tombstone. The hourly sweep-deleted-image-assets cron picks the asset up afterwards, and whatever bytes landed go with it. (The first version used 15 minutes; Greptile pointed out a slow upload can outlast that.)
• createR2UploadIntent's args are unchanged, so function_spec.json doesn't move.

Compatibility

• The schema only widens: one optional field, and no backfill is needed. Rows written before this deploy have no expiry and are handled exactly as before.
• No client changes. The live web build and shipped desktop builds call the same functions with the same args.

Tests

• New in convex/imageAssetLifecycle.test.ts: an upload requested through generateUploadUrl stores the expiry it returned. After the strategy is deleted mid-upload, nothing is sent to R2 and the row stays. 23 hours after expiry the sweep still leaves it. At 24 hours it deletes the object once and removes the row. Without the claim guard this test fails ("expected spy to not be called").
• npx tsc --noEmit passes. npm run test:convex passes: 168 tests in 10 files. npm run audit:convex-contract passes.

🤖 Generated with Claude Code

Confidence Score: 5/5

No outstanding findings block merging.

Findings

1.  Long uploads outlast grace ▶

Summary

Image uploads now have a 24-hour grace period after their signed URL expires before cleanup can delete the asset. The lifecycle test checks the new cutoff.

_{Reviews (2) · Last reviewed commit: "Give an in-flight upload a day, not 15 m..."}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: dc582cb5-4383-4ebc-8248-407efce4ce71

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Long uploads outlast grace

If R2 allows a PUT started before URL expiry to finish more than 15 minutes afterward, this cutoff lets the sweep delete the object and remove its asset row while the PUT is still in flight. The PUT can then write bytes to the public URL with no row left for a later sweep to reclaim them. Cleanup needs a reliable completion bound or a reclaimable record rather than this fixed grace period.

Artifacts

Delayed PUT reproduction source

• The authored Vitest test runs the actual Convex handlers against a stateful R2 mock and controls when the accepted PUT commits.

Reproduction test configuration

• The authored configuration runs the focused test in the repository's edge-runtime environment.

Before-change delayed PUT run

• The executed test against HEAD^ shows immediate HTTP 204 No Content deletion followed by HTTP 200 OK PUT and public GET with no asset row.

After-change deferred sweep run

• The executed test against HEAD shows deferred HTTP 204 No Content deletion followed by HTTP 200 OK PUT and public GET with no asset row.

_{Ran code and verified through T-Rex}

[SunkenInTime]

Addressed the long-upload finding in d4759c3: the grace after URL expiry is now 24 hours instead of 15 minutes, so a slow PUT that R2 accepted before expiry finishes long before its object and row are deleted. @greptileai