[codex] Fix bundle size metadata queue#2476
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds retry-backed bundle-size retrieval for R2 uploads, enforces R2 path validation in version updates, centralizes zero-size metadata upserts, exposes test utilities, adds manifest-cleanup early-return, and extends unit tests and mocks to validate bundle-size metadata and retry behavior. ChangesBundle Size Retry and Metadata Handling
Test Seed Cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Merging this PR will improve performance by 81.51%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | /updates manifest response with metadata |
208.7 µs | 115 µs | +81.51% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/fix-version-size-queue (9a7a369) with main (5969436)
Footnotes
-
2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98fcf3d0bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@supabase/functions/_backend/triggers/on_version_update.ts`:
- Around line 177-180: The exported HTTP handler currently always returns 200
and thus never honors the retryable 503 thrown by updateIt(); modify the
exported app route handler to call and await updateIt() (the same updateIt used
by onVersionUpdateTestUtils) and propagate its thrown quickError/HTTP status up
to the HTTP response so a record with storage_provider === 'r2' and missing
r2_path/manifest results in a 503 retryable response instead of being
acknowledged with 200. Ensure the handler does not swallow exceptions from
updateIt() and maps quickError status and payload into the HTTP response.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f1fe932-46af-4d31-9627-75df40614d7e
📒 Files selected for processing (2)
supabase/functions/_backend/triggers/on_version_update.tstests/on-version-update-cleanup.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
98fcf3d to
b24f2d9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b24f2d955c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b24f2d9 to
045a2a3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 045a2a3b72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
045a2a3 to
9a7a369
Compare
|



Summary
Fixes
on_version_updateso app version size metadata is only written from a completed R2 bundle state.r2-directupload snapshots that already have anr2_path.503when a completedr2bundle still has no readable object size, so the queue keeps the message and applies its retry budget instead of deleting it as successful.r2-direct, successful completedr2, and retryable missing-size cases.Motivation
The exact production failure path was: CLI creates the version as
r2-direct, upload-link updatesr2_pathwhile the row is stillr2-direct, then the final upload update flips the row tor2. The old trigger treated the intermediate snapshot asno v2 pathand upsertedapp_versions_meta.size = 0. If the final size lookup later returned0, it logged and returned success, so the queue deleted the message and the zero remained.Business Impact
Bundle storage accounting becomes consistent again for new uploads. Failed or delayed R2 metadata reads are observable and retryable instead of silently creating wrong zero-byte metadata, which reduces undercounted storage and bad admin/debug data.
Test Plan
bunx vitest run tests/on-version-update-cleanup.unit.test.tsbun run lint:backendbun run typecheck:backendbun run test:backendbun run typecheckbun run lint(passes with one existing frontend JSDoc warning insrc/services/compatibilityEvents.ts)AI-Generated Content
This PR was generated by Codex.
Summary by CodeRabbit
New Features
Bug Fixes
Tests