[server] Retry failed post-sync maintenance - #1901
Merged
Merged
Conversation
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
Reviewer's GuidePost-sync job processing is updated so required maintenance steps fail fast with Sentry context and step-specific progress while BullMQ post-sync queues now reuse the shared retry policy, and tests/docs are adjusted to cover the new rejection and retry behavior. Flow diagram for post-sync queues using shared retry optionsflowchart LR
SYNC_JOB_RETRY_OPTIONS["SYNC_JOB_RETRY_OPTIONS (attempts: 288, fixed 300000ms backoff, removeOnFail)"]
enqueueDebouncedPostSyncMaintenance["enqueueDebouncedPostSyncMaintenance"]
enqueueDebouncedUserRefit["enqueueDebouncedUserRefit"]
GlobalPostSyncQueue["BullMQ queue: global-maintenance"]
UserRefitQueue["BullMQ queue: user-refit"]
SYNC_JOB_RETRY_OPTIONS --> enqueueDebouncedPostSyncMaintenance
SYNC_JOB_RETRY_OPTIONS --> enqueueDebouncedUserRefit
enqueueDebouncedPostSyncMaintenance --> GlobalPostSyncQueue
enqueueDebouncedUserRefit --> UserRefitQueue
GlobalPostSyncQueue --> processPostSyncJob["processPostSyncJob (global maintenance)"]
UserRefitQueue --> processPostSyncJobUser["processPostSyncJob (per-user refit)"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughPost-sync user maintenance now rejects on required-step failures, records step-specific progress and Sentry context, and stops subsequent steps. Both post-sync enqueue paths reuse the shared BullMQ retry policy, with updated processor and queue tests. ChangesPost-sync retry behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BullMQ
participant processPostSyncJob
participant Sentry
participant PostSyncStep
BullMQ->>processPostSyncJob: Run post-sync maintenance
processPostSyncJob->>PostSyncStep: Execute ordered required step
PostSyncStep-->>processPostSyncJob: Return success or error
processPostSyncJob->>Sentry: Capture error with postSyncStep
processPostSyncJob-->>BullMQ: Rethrow error for retry
Assessment against linked issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the catch blocks for body refresh/refit/cache invalidation, consider wrapping
updatePostSyncProgressin its own try/catch so that a progress-update failure cannot mask the original maintenance error you're intentionally rethrowing for BullMQ retries. - The step progress percentages and messages (20/45/75/100 and their strings) are now duplicated across tests and implementation; factoring these into shared constants would reduce the risk of the tests drifting from the actual processor behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the catch blocks for body refresh/refit/cache invalidation, consider wrapping `updatePostSyncProgress` in its own try/catch so that a progress-update failure cannot mask the original maintenance error you're intentionally rethrowing for BullMQ retries.
- The step progress percentages and messages (20/45/75/100 and their strings) are now duplicated across tests and implementation; factoring these into shared constants would reduce the risk of the tests drifting from the actual processor behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/jobs/process-post-sync-job.test.ts`:
- Around line 194-196: Update the rejection assertion for processPostSyncJob to
use identity comparison with refreshError via toBe rather than deep error
matching via toThrow, ensuring the original body-refresh error instance is
rethrown.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 786fef8e-c344-40d2-a890-04aec7f878ee
📒 Files selected for processing (6)
docs/superpowers/plans/2026-07-24-post-sync-retry.mddocs/superpowers/specs/2026-07-24-post-sync-retry-design.mdsrc/jobs/process-post-sync-job.test.tssrc/jobs/process-post-sync-job.tssrc/jobs/queues.test.tssrc/jobs/queues.ts
Asherlc
changed the title
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
pnpm vitest run src/jobs/process-post-sync-job.test.ts src/jobs/queues.test.ts --project unitpnpm test:changedpnpm lintThe full local unit/mobile suite passed 13,224 tests but hit one unrelated load-sensitive timeout in the Garmin multi-chunk ZIP streaming test; that exact test passed immediately in isolation.
Closes #1730
Summary by Sourcery
Make post-sync maintenance steps fail fast and be retried using the shared BullMQ retry policy.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by cubic
Make required post-sync maintenance fail fast and reuse the shared retry policy so BullMQ retries both global and per-user post-sync jobs. Prevents stale personalized params and cache after sync; addresses #1730.
SYNC_JOB_RETRY_OPTIONS(attempts: 288, backoff: fixed 300_000 ms, failed-job retention) toenqueueDebouncedPostSyncMaintenanceandenqueueDebouncedUserRefitwhile keeping delay, deduplication, andremoveOnComplete.Written for commit 42f4306. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes