Prevent orphaned photo uploads - #12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesPhoto storage lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant uploadPhotoBatch
participant SupabaseStorage
participant PhotosTable
Client->>uploadPhotoBatch: submit photo batch
uploadPhotoBatch->>SupabaseStorage: upsert content-addressed objects
uploadPhotoBatch->>PhotosTable: check clashes and insert fresh rows
uploadPhotoBatch-->>Client: return results and warnings
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/photo-upload.ts`:
- Around line 218-229: The catch block’s failedClientIds handling duplicates IDs
already recorded by the clash check when insertion throws. Update the catch
logic in the surrounding upload-save flow to append only rows whose client_id
was not previously added, preserving existing error-message handling and
ensuring each failed client ID appears once.
- Around line 126-127: Remove the removeObjects call from the imageUpload.error
branch in the photo upload flow, preserving successfully uploaded thumbnails for
retries or later orphan purging even when the image upload fails.
In `@scripts/purge-orphan-objects.mjs`:
- Around line 58-75: Update the apply-mode inventory operations in
fetchReferencedPaths and the related object-listing and deletion flows to use
the service-role writeKey rather than the anonymous key. Preserve read-only
behavior if applicable, but ensure every authoritative row fetch and storage
listing used to classify or delete orphans passes key: writeKey to rest.
- Around line 159-171: The purge flow around fetchReferencedPaths, confirmed,
and the deletion loop must prevent uploads or retries from creating references
between confirmation and deletion. Acquire and hold the existing shared
maintenance/advisory lock before re-reading rows, retain it through all
confirmed deletions, and release it afterward, ensuring upload paths honor the
same lock before creating references.
- Around line 104-113: Update partitionOrphans so objects with missing or
invalid createdAt values are retained in tooNew rather than classified as
orphans. Validate the parsed timestamp before applying the cutoff comparison,
while preserving the existing referenced-object handling and age-based orphan
classification for valid timestamps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a34ddc0e-68ac-4211-b626-b176ebadc1e7
📒 Files selected for processing (4)
.gitignorelib/photo-upload.test.tslib/photo-upload.tsscripts/purge-orphan-objects.mjs
A unique-violation insert usually means a racer inserted one of the batch's hashes after the clash check, and that row references the same content-addressed paths this batch uploaded. Blindly rolling back the fresh rows' objects would delete the racer's live objects, so re-query which hashes gained rows and remove only unreferenced paths; if the re-check itself fails, leave the objects for the retry/purge to handle. Also key the purge script's min-age guard on updated_at: a retried upload upserts onto its old key without resetting created_at, so a just-re-uploaded object awaiting its row insert must count as new.
What changed
Why
Failed upload retries used random object paths, leaving unreachable files in Storage. The production purge removed 2,712 existing orphans and reclaimed about 1.8 GB; this change prevents retries from recreating that growth pattern.
Safety
The collision re-check now handles query errors explicitly. It only removes clashing uploads when the existing row points somewhere else (legacy UUID storage), and it never blindly cleans up after an unknown insert outcome.
Validation
npm run ciuseMembership.tswarningnode --check scripts/purge-orphan-objects.mjsSummary by CodeRabbit
New Features
Bug Fixes