Skip to content

Clarify bulk sync impact and prevent duplicate full syncs - #2298

Merged
Asherlc merged 8 commits into
mainfrom
issue-2178-sync-action-hierarchy
Jul 30, 2026
Merged

Asherlc merged 8 commits into
mainfrom
issue-2178-sync-action-hierarchy

Conversation

@Asherlc

@Asherlc Asherlc commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • make the 7-day sync the clear primary action on web and mobile
  • explain and confirm full-history sync impact with cancel-first keyboard/accessibility behavior
  • share sync wording/range semantics across clients and preserve exact server errors/progress
  • coalesce overlapping user-triggered full syncs with BullMQ lifecycle-scoped deduplication while keeping continuations distinct

Validation

  • pnpm test:changed (28 files, 839 tests)
  • pnpm test (945 files, 14,613 tests)
  • pnpm tsc --noEmit
  • package typechecks for server, web, and mobile
  • pnpm storybook:web:build
  • pnpm storybook:mobile:build
  • responsive runtime Storybook audit for web and mobile default/confirmation states
  • code/policy lint and ClickHouse-backed analytics SQL lint
  • Redis lifecycle integration test (pending coalescing, completion/failure release, WHOOP continuation)
  • focused Stryker rerun covering every lifecycle-dedup false boundary and the router cooldown-skip option (18/18 mutants killed)

Fixes #2178

Asherlc added 4 commits July 29, 2026 10:40
Use BullMQ lifecycle deduplication to coalesce only pending initial full-history jobs while allowing later runs and checkpoint continuations.\n\nRefs #2178
Use the canonical mobile story background and retain the extended queued-job type in Redis lifecycle assertions.
Use inverse palette text for accent-filled sync actions and cover the accessible contrast role.
Apply the shared busy-state opacity to the mobile full-history link and cover it alongside disabled semantics.
Copilot AI review requested due to automatic review settings July 29, 2026 18:15
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

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.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 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.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sourcery-ai

sourcery-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Clarifies bulk sync hierarchy and semantics across web and mobile, centralizes shared sync action copy, and introduces BullMQ lifecycle-scoped deduplication for user-triggered full-history sync jobs to prevent duplicate work while preserving checkpoint continuations and exact error reporting.

Sequence diagram for full-history bulk sync with BullMQ lifecycle deduplication

sequenceDiagram
  actor User
  participant WebCtrl as SyncAllControls
  participant Panel as DataSourcesPanel
  participant SyncRouter as syncRouter.triggerSync
  participant Enqueue as enqueueSyncJob
  participant RequestDedup as enqueueSyncJobWithRequestDedup
  participant Queue as BullMQ Queue.add

  User->>WebCtrl: click full-history button
  WebCtrl->>Panel: onFullSync()
  Panel->>SyncRouter: syncMutation.mutateAsync({ sinceDays: undefined })
  SyncRouter->>Enqueue: enqueueSyncJob(providerId, jobData, { singleFlightFullSync: true })
  Enqueue->>RequestDedup: enqueueSyncJobWithRequestDedup(providerId, jobData, jobOptionsWithDedup)
  RequestDedup->>Queue: addJob("sync", jobData, { deduplication: { id: sync:full:... } })
  Queue-->>RequestDedup: job(id = J1)
  RequestDedup-->>Panel: EnqueuedSyncJob { id: J1, alreadyQueued: false }
  Panel-->>WebCtrl: busy = true

  alt duplicate full-history while J1 pending
    User->>WebCtrl: click full-history button
    WebCtrl->>Panel: onFullSync()
    Panel->>SyncRouter: syncMutation.mutateAsync({ sinceDays: undefined })
    SyncRouter->>Enqueue: enqueueSyncJob(... singleFlightFullSync: true)
    Enqueue->>RequestDedup: enqueueSyncJobWithRequestDedup(... deduplication: { id: sync:full:... })
    RequestDedup->>Queue: addJob("sync", jobData, { deduplication: { id: sync:full:... } })
    Queue-->>RequestDedup: job(id = J1)
    RequestDedup-->>Panel: EnqueuedSyncJob { id: J1, alreadyQueued: true }
  end

  Note over Queue,RequestDedup: BullMQ lifecycle dedup key released when job J1 completes or fails
Loading

File-Level Changes

Change Details Files
Make recent (7-day) bulk sync the primary action with shared semantics and copy on web and mobile, and gate full-history sync behind an explanatory confirmation.
  • Introduce shared ROUTINE_SYNC_DAYS and SYNC_ALL_ACTIONS contract in providers-meta for recent vs full sync wording and accessibility labels.
  • Replace web DataSourcesPanel inline Sync All / Full Sync All buttons with a SyncAllControls component that uses shared copy, emphasizes recent sync, and shows a modal confirmation for full history.
  • Replace mobile ProvidersScreen inline bulk sync buttons with a SyncAllControls React Native component that emphasizes recent sync, requires confirmation for full history, and ensures Cancel-first keyboard/accessibility behavior.
  • Update web and mobile tests (and new SyncAllControls tests/stories) to assert button labels, accessibility names, confirmation text, hierarchy, progress messaging, and error handling using the shared contract.
packages/providers-meta/src/sync-actions.ts
packages/providers-meta/src/sync-actions.test.ts
packages/providers-meta/package.json
packages/web/src/components/DataSourcesPanel.tsx
packages/web/src/components/DataSourcesPanel.test.tsx
packages/web/src/components/SyncAllControls.tsx
packages/web/src/components/SyncAllControls.test.tsx
packages/web/src/components/SyncAllControls.stories.tsx
packages/mobile/app/providers/index.tsx
packages/mobile/app/providers/index.test.tsx
packages/mobile/app/providers/styles.ts
packages/mobile/app/providers/sync-all-controls.tsx
packages/mobile/app/providers/sync-all-controls.test.tsx
packages/mobile/app/providers/sync-all-controls.stories.tsx
Surface exact bulk sync startup errors and keep bulk actions disabled while underlying provider jobs are still active/polling, with shared progress messaging.
  • Track a syncAllError state on web and mobile bulk sync flows and display the exact trigger Error.message via alert-like UI tied to shared SYNC_ALL_ACTIONS.progressMessage semantics.
  • Extend web DataSourcesPanel syncAll handling to mark provider states as error using the same message and expose a global alert region; add tests to ensure errors appear both per provider and globally.
  • Extend mobile ProvidersScreen to set syncAllError on bulk sync failure and render it through SyncAllControls; update tests to assert error visibility for recent sync startup failures.
  • Add tests that enforce both bulk actions remain disabled while provider jobs are still polling, and that progress guidance text is shown via role=status/accessible messaging.
packages/web/src/components/DataSourcesPanel.tsx
packages/web/src/components/DataSourcesPanel.test.tsx
packages/mobile/app/providers/index.tsx
packages/mobile/app/providers/index.test.tsx
packages/mobile/app/providers/sync-all-controls.tsx
packages/mobile/app/providers/sync-all-controls.test.tsx
Introduce lifecycle-scoped BullMQ deduplication for user-triggered initial full sync jobs while keeping checkpoint continuations and non-user-triggered work distinct.
  • Extend EnqueueSyncJobOptions with singleFlightFullSync and add initialFullSyncDeduplicationId helper to compute a provider/user-scoped deduplication ID only for initial full-history jobs without checkpoints.
  • Wire deduplication into enqueueSyncJob by merging BullMQ queue options with a deduplication.id field when singleFlightFullSync is enabled and the job is a full-history initial request.
  • Update enqueue-sync-job tests to cover user-triggered initial full sync deduplication, checkpoint continuation jobs that intentionally omit deduplication, and non-user-triggered full sync jobs that keep prior behavior.
  • Adjust sync-request-job enqueueSyncJobWithRequestDedup to detect BullMQ lifecycle deduplication (job.id differs from requested jobId when deduplication is present) and mark the returned EnqueuedSyncJob as alreadyQueued.
  • Add sync-request-job tests verifying alreadyQueued semantics for lifecycle-deduplicated jobs and ensuring cooldown-delayed jobs do not carry deduplication options.
  • Update server sync router triggerSync to pass singleFlightFullSync:true so sync-all fan-out full jobs get lifecycle deduplication keys, while preserving behavior for other sync windows.
  • Add a Redis-backed integration test that proves pending initial full jobs coalesce under BullMQ deduplication and that completion or failure releases the deduplication key, allowing later full syncs; also verify checkpoint continuations enqueue as distinct jobs.
  • Add tests ensuring provider-specific sync-all fan-out jobs receive deduplication IDs for full syncs and that other paths (e.g., non-full windows) do not set deduplication.
src/jobs/enqueue-sync-job.ts
src/jobs/enqueue-sync-job.test.ts
src/jobs/sync-request-job.ts
src/jobs/sync-request-job.test.ts
src/jobs/sync-request-job.integration.test.ts
packages/server/src/routers/sync.ts
packages/server/src/routers/sync.test.ts
Document the sync action hierarchy and TDD plan used for this change set.
  • Add a superpowers plan document describing goals, behavior, scope, test strategy, file structure, and step-by-step tasks for implementing sync action hierarchy and deduplication.
  • Reference related docs such as BullMQ deduplication and package READMEs to contextualize the implementation.
  • Use checkbox-style task lists to make it suitable for agentic workers and future maintenance.
docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md

Assessment against linked issues

Issue Objective Addressed Explanation
#2178 Make the routine recent sync the primary, higher-visibility bulk sync action, with full-history sync as a secondary action on web and mobile provider settings screens.
#2178 Place full-history bulk sync behind an explanation and confirmation that clarifies range, duration/cost, and overwrite/reconciliation behavior, and surface progress guidance while the sync is running.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e812d9d9-5961-408a-8da5-63e96c6497b7

📥 Commits

Reviewing files that changed from the base of the PR and between e2faef6 and cac8fd5.

📒 Files selected for processing (9)
  • docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md
  • packages/mobile/app/providers/index.test.tsx
  • packages/mobile/app/providers/index.tsx
  • packages/mobile/app/providers/styles.ts
  • packages/mobile/app/providers/sync-all-controls.test.tsx
  • packages/mobile/app/providers/sync-all-controls.tsx
  • packages/providers-meta/package.json
  • packages/providers-meta/src/sync-actions.test.ts
  • packages/providers-meta/src/sync-actions.ts
📝 Walkthrough

Walkthrough

The PR adds shared recent/full sync action semantics, replaces web and mobile bulk-sync controls with confirmation-based full-history flows, surfaces progress and errors, and enables lifecycle-scoped deduplication for initial user-triggered full-sync jobs.

Changes

Sync action hierarchy

Layer / File(s) Summary
Shared sync contract
packages/providers-meta/*, docs/superpowers/plans/*
Defines the seven-day recent window, action labels, accessibility copy, full-history confirmation text, progress messaging, and implementation plan.
Full-history single-flight queueing
src/jobs/*, packages/server/src/routers/sync.*
Adds deduplication for initial user-triggered full syncs by provider and user, while excluding checkpoint continuations and non-user-triggered work.
Web sync controls
packages/web/src/components/*
Adds recent/full controls, confirmation and focus behavior, busy/error states, panel wiring, interaction tests, and Storybook states.
Mobile sync controls
packages/mobile/app/providers/*
Adds the React Native control component, modal confirmation and accessibility behavior, provider wiring, error/progress states, tests, and stories.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant SyncAllControls
  participant DataSourcesPanel
  participant syncRouter
  participant enqueueSyncJob
  Operator->>SyncAllControls: Select full-history sync
  SyncAllControls->>Operator: Show explanation and confirmation
  Operator->>SyncAllControls: Confirm full sync
  SyncAllControls->>DataSourcesPanel: Invoke onFullSync
  DataSourcesPanel->>syncRouter: Trigger provider syncs
  syncRouter->>enqueueSyncJob: Enqueue deduplicated jobs
Loading

Assessment against linked issues

Objective Addressed Explanation
Make routine sync primary and explain the impact of full-history sync [#2178]
Require confirmation before starting full-history sync and show progress [#2178]

Possibly related PRs

  • Asherlc/dofek#1359: Modifies the web provider panel’s sync targets and related sync-click behavior.
  • Asherlc/dofek#1878: Adds accessibility behavior to the mobile provider bulk-sync controls.

Suggested labels: area/mobile, area/web, area/server, type/feature

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is clear and imperative, but it omits the required area prefix for this multi-area change. Prefix it with the relevant area, e.g. "[web][mobile][server] Clarify bulk sync impact and prevent duplicate full syncs".
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Make recent sync primary and dedupe user-triggered full-history bulk syncs

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Promote 7-day “recent” sync as the primary bulk action across web and mobile.
• Gate full-history bulk sync behind an impact explanation + cancel-first confirmation.
• Coalesce duplicate user-triggered initial full sync jobs using BullMQ lifecycle deduplication.
Diagram

graph TD
  ProvidersMeta["providers-meta: sync-actions"] --> WebUI["Web: bulk sync"] --> SyncRouter["syncRouter trigger"] --> Enqueue["enqueueSyncJob"] --> BullMQ["BullMQ (dedupe)"] --> Redis[(Redis)]
  ProvidersMeta --> MobileUI["Mobile: bulk sync"] --> SyncRouter
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compute a stable full-sync request key (request-level dedup only)
  • ➕ Avoids coupling to BullMQ deduplication semantics/options
  • ➕ Keeps behavior inside existing request-query identity scheme
  • ➖ Hard to define a “stable” key for full-history when cursors/until windows move
  • ➖ Risk of accidentally deduping legitimate later runs if key is too coarse
  • ➖ Still requires careful handling to keep checkpoint continuations distinct
2. Explicit single-flight lock (Redis SETNX / Redlock) per user+provider
  • ➕ Works even if queue implementation changes (not BullMQ-specific)
  • ➕ Can express richer policy (timeouts, manual unlock, observability)
  • ➖ Adds operational complexity and failure modes (stale locks, lock loss)
  • ➖ Requires extra cleanup paths on worker crash and job failure
  • ➖ Duplicates functionality BullMQ already provides for lifecycle dedupe
3. Move confirmation + messaging server-side only (client-agnostic UX)
  • ➕ Ensures any future client sees the same gating behavior
  • ➕ Centralizes copy/versioning
  • ➖ Still requires client UI for modal/focus management and accessibility
  • ➖ Less flexible for platform-specific interaction expectations

Recommendation: Keep the PR’s approach: shared client contract + explicit confirmation UX, and BullMQ lifecycle-scoped deduplication for only the user-triggered initial full-history job. This minimizes custom state, aligns with BullMQ’s intended dedupe lifecycle (release on complete/fail), and preserves distinct checkpoint continuations.

Files changed (22) +1322 / -130

Enhancement (5) +363 / -65
index.tsxReplace inline bulk buttons with SyncAllControls and shared 7-day constant +13/-35

Replace inline bulk buttons with SyncAllControls and shared 7-day constant

• Imports ROUTINE_SYNC_DAYS and uses it for recent sync calls. Adds group-level error state for bulk sync failures and renders the new SyncAllControls component instead of bespoke buttons.

packages/mobile/app/providers/index.tsx

sync-all-controls.tsxIntroduce SyncAllControls component with confirmation modal and shared copy +217/-0

Introduce SyncAllControls component with confirmation modal and shared copy

• Implements recent sync as the primary accent action, full-history as a secondary link that opens a modal with impact text and explicit confirmation. Adds cancel-first ordering, accessibility focus management, busy progress messaging, and group-level error display.

packages/mobile/app/providers/sync-all-controls.tsx

sync-actions.tsAdd shared sync action contract (labels, accessibility, confirmation copy) +19/-0

Add shared sync action contract (labels, accessibility, confirmation copy)

• Defines ROUTINE_SYNC_DAYS and SYNC_ALL_ACTIONS with consistent labels, descriptions, confirmation text, and progress messaging shared across clients.

packages/providers-meta/src/sync-actions.ts

DataSourcesPanel.tsxUse SyncAllControls with shared 7-day constant and busy/error state +23/-30

Use SyncAllControls with shared 7-day constant and busy/error state

• Replaces the old two-button bulk UI with SyncAllControls and imports ROUTINE_SYNC_DAYS for recent sync. Tracks group-level trigger errors and computes a broader busy state (mutation pending, active polling, active syncs) to prevent duplicate triggers.

packages/web/src/components/DataSourcesPanel.tsx

SyncAllControls.tsxIntroduce web SyncAllControls with modal confirmation and shared copy +91/-0

Introduce web SyncAllControls with modal confirmation and shared copy

• Implements a primary recent sync button plus a secondary full-history link that opens a modal. Uses the shared SYNC_ALL_ACTIONS text, enforces explicit confirmation, supports initial focus on Cancel, and surfaces progress and group-level errors.

packages/web/src/components/SyncAllControls.tsx

Bug fix (3) +26 / -3
sync.tsEnable single-flight full sync option when triggering bulk sync +1/-1

Enable single-flight full sync option when triggering bulk sync

• Passes a new enqueue option to coalesce overlapping user-triggered initial full-history jobs during bulk sync fan-out.

packages/server/src/routers/sync.ts

enqueue-sync-job.tsAdd BullMQ lifecycle deduplication for initial user-triggered full sync +19/-1

Add BullMQ lifecycle deduplication for initial user-triggered full sync

• Introduces singleFlightFullSync option and derives a deduplication id keyed by provider+user only for initial full-history jobs (no checkpoint). Injects BullMQ deduplication options while preserving existing request-level dedup for other cases.

src/jobs/enqueue-sync-job.ts

sync-request-job.tsMark jobs as already queued when BullMQ lifecycle deduplication occurs +6/-1

Mark jobs as already queued when BullMQ lifecycle deduplication occurs

• Adjusts alreadyQueued detection to treat BullMQ deduplication (job.id differs from requested jobId) as a coalesced enqueue, improving downstream UX/state handling.

src/jobs/sync-request-job.ts

Refactor (1) +0 / -36
styles.tsRemove legacy Sync All button styles (moved to component-local styles) +0/-36

Remove legacy Sync All button styles (moved to component-local styles)

• Deletes the old ProvidersScreen bulk-button style block now encapsulated in SyncAllControls styles.

packages/mobile/app/providers/styles.ts

Tests (9) +651 / -25
index.test.tsxUpdate ProvidersScreen bulk sync tests for new hierarchy + errors +44/-20

Update ProvidersScreen bulk sync tests for new hierarchy + errors

• Adjusts expectations from “Sync All / Full Sync All” to recent-primary + full-history secondary action with confirmation. Adds coverage for exact bulk-trigger error propagation and updates mocks for modal focus behavior.

packages/mobile/app/providers/index.test.tsx

sync-all-controls.test.tsxAdd unit tests for mobile SyncAllControls UX + accessibility behaviors +205/-0

Add unit tests for mobile SyncAllControls UX + accessibility behaviors

• Covers recent-primary ordering, full-history explanation/confirmation gating, cancel-first + accessibility focus, request-close cancellation, busy disabled semantics, and exact error rendering.

packages/mobile/app/providers/sync-all-controls.test.tsx

sync-actions.test.tsAdd unit tests for shared sync action semantics and copy +30/-0

Add unit tests for shared sync action semantics and copy

• Validates ROUTINE_SYNC_DAYS=7, recent/full labels and accessibility labels, and that the full-history confirmation description includes the required impact disclosures and progress wording.

packages/providers-meta/src/sync-actions.test.ts

sync.test.tsAssert full-history fan-out enqueues with lifecycle deduplication ids +9/-2

Assert full-history fan-out enqueues with lifecycle deduplication ids

• Updates syncRouter tests to expect BullMQ deduplication ids for user-triggered full-history sync jobs per provider/user, and ensures non-deduped paths remain unchanged.

packages/server/src/routers/sync.test.ts

DataSourcesPanel.test.tsxUpdate bulk sync tests for new controls, errors, and busy disablement +36/-3

Update bulk sync tests for new controls, errors, and busy disablement

• Migrates clicks to the new recent-primary label, asserts group-level alert rendering for trigger failures, and adds coverage ensuring both bulk actions remain disabled while jobs are still polling.

packages/web/src/components/DataSourcesPanel.test.tsx

SyncAllControls.test.tsxAdd unit tests for web SyncAllControls focus + confirmation + errors +101/-0

Add unit tests for web SyncAllControls focus + confirmation + errors

• Covers recent-primary ordering/styling, full-history explanation and explicit confirmation, cancel-first initial focus and Escape-to-close focus restoration, busy disablement with progress output, and exact error rendering.

packages/web/src/components/SyncAllControls.test.tsx

enqueue-sync-job.test.tsAdd tests for lifecycle-scoped full-sync deduplication behavior +59/-0

Add tests for lifecycle-scoped full-sync deduplication behavior

• Verifies that user-triggered initial full syncs receive a BullMQ deduplication id, checkpoint continuations do not reuse it, and non-user-triggered full sync work remains non-deduped.

src/jobs/enqueue-sync-job.test.ts

sync-request-job.integration.test.tsAdd Redis-backed integration test for BullMQ dedupe lifecycle +136/-0

Add Redis-backed integration test for BullMQ dedupe lifecycle

• Runs BullMQ against Redis to prove pending full syncs coalesce, and that completion/failure releases the lifecycle key. Also asserts checkpoint continuations remain distinct from the initial operation.

src/jobs/sync-request-job.integration.test.ts

sync-request-job.test.tsAdd unit test for detecting lifecycle-deduped jobs as alreadyQueued +31/-0

Add unit test for detecting lifecycle-deduped jobs as alreadyQueued

• Validates that when BullMQ returns a different id due to lifecycle deduplication, the result is flagged as alreadyQueued and the deduplication options are passed through.

src/jobs/sync-request-job.test.ts

Documentation (1) +198 / -0
2026-07-29-sync-action-hierarchy.mdAdd TDD plan for sync action hierarchy + single-flight full sync +198/-0

Add TDD plan for sync action hierarchy + single-flight full sync

• Introduces a test-first implementation plan covering shared semantics/copy, web/mobile interaction requirements, and BullMQ lifecycle deduplication. Documents current issues, validation steps, and the intended file layout.

docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md

Other (3) +84 / -1
sync-all-controls.stories.tsxAdd Storybook stories for mobile SyncAllControls states +45/-0

Add Storybook stories for mobile SyncAllControls states

• Adds Default, confirmation-open, syncing, and error stories with a canonical background wrapper for runtime visual audit.

packages/mobile/app/providers/sync-all-controls.stories.tsx

package.jsonExport sync-actions entry point from providers-meta +2/-1

Export sync-actions entry point from providers-meta

• Adds a dedicated export for the new sync-actions module so web and mobile can import shared constants/copy without reaching into internal paths.

packages/providers-meta/package.json

SyncAllControls.stories.tsxAdd Storybook stories for web SyncAllControls states +37/-0

Add Storybook stories for web SyncAllControls states

• Adds Default, confirmation-open, syncing, and error stories to verify hierarchy, messaging, and modal behavior visually.

packages/web/src/components/SyncAllControls.stories.tsx

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Mobile Preview

Scan to open on device:

QR code for dofek://preview/pr-2298

Channel pr-2298
Deep Link dofek://preview/pr-2298
Commit e795e2d

To test on device:

  1. Build and install the preview client: PREVIEW_CHANNEL=pr-2298 pnpm expo prebuild --clean -p ios
  2. Or tap deep link on an existing preview build: dofek://preview/pr-2298

Each PR gets its own channel. Build a preview client with PREVIEW_CHANNEL=pr-{N} to test.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for e795e2d0 are ready:

This comment updates automatically on each PR push.

@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 216 rules

Grey Divider


Action required

1. Full-sync dedup ends early 🐞 Bug ≡ Correctness
Description
singleFlightFullSync only applies lifecycle deduplication to the initial full-history job and
explicitly skips checkpoint continuations, but providers can enqueue a continuation and return
continued: true, completing the initial job and releasing the dedup key while the continuation is
still queued/running. This allows users to start another full-history sync during an in-progress
continued run (e.g., Whoop/Garmin), reintroducing duplicate expensive work.
Code

src/jobs/enqueue-sync-job.ts[R18-31]

+function initialFullSyncDeduplicationId(
+  providerId: string,
+  jobData: SyncJobData,
+  options?: EnqueueSyncJobOptions,
+): string | undefined {
+  if (
+    !options?.singleFlightFullSync ||
+    jobData.targetRefreshWindow?.type !== "full" ||
+    jobData.checkpoint !== undefined
+  ) {
+    return undefined;
+  }
+  return `sync:full:${providerId}:${jobData.userId}`;
+}
Relevance

●● Moderate

Continuation semantics are subtle; repo cares about continuation correctness but no clear precedent
about lifecycle dedup across continuations.

PR-#1866

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds deduplication only when checkpoint is undefined, but the sync worker can enqueue a
continuation with checkpoint and then return early on continued, completing the initial job
(releasing lifecycle dedup). Whoop demonstrates this continued behavior explicitly after enqueueing
a continuation.

src/jobs/enqueue-sync-job.ts[18-31]
src/jobs/process-sync-job.ts[341-364]
src/jobs/process-sync-job.ts[371-382]
src/jobs/process-sync-job.ts[555-557]
src/providers/whoop/sync-orchestrator.ts[517-529]
docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md[46-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
BullMQ simple-mode deduplication is currently attached only to the *initial* full-history sync job and is skipped when `jobData.checkpoint` is present. However, the sync worker can enqueue a checkpoint continuation and then return (job completes), which releases the lifecycle deduplication key while the logical full-history sync is still ongoing via continuation jobs.

## Issue Context
- `processSyncJob` enqueues a continuation via `enqueueSyncJob(...checkpoint...)` and then returns early when `result.continued` is true.
- Providers like Whoop explicitly return `continued: true` immediately after enqueueing the continuation.
- The design doc in this PR states BullMQ deduplication keys are released on job completion/failure.

## Fix Focus Areas
- src/jobs/enqueue-sync-job.ts[18-65]
- src/jobs/process-sync-job.ts[341-365]
- src/jobs/process-sync-job.ts[371-382]
- src/jobs/process-sync-job.ts[555-557]

## Suggested fix direction
Implement single-flight for the *entire logical full-history operation*, not just the first BullMQ job. Options include:
1) **Operation-scoped Redis lock** keyed by `providerId:userId` acquired on user-triggered full sync and released only when the continuation chain finishes (i.e., when provider returns `continued: false` / checkpoint cleared). Continuation jobs should preserve/refresh the lock (e.g., extend TTL) so the lock persists through the chain.
2) **Keep a single BullMQ job alive** across checkpoints (e.g., store checkpoint on the same job and re-run via retry/delay) so the BullMQ lifecycle dedup key is not released mid-operation.

Add/extend tests to cover: initial full sync enqueues a continuation and completes, then a second user-triggered full sync attempt during the continuation should still coalesce (or be rejected) until the chain finishes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Parallel BullMQ teardown ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new Redis-backed BullMQ integration test runs events.close(),
queue.obliterate()/queue.close(), and worker.close() cleanup callbacks concurrently via
Promise.all, even though these operations are order-sensitive. This can cause nondeterministic
teardown behavior (e.g., obliterate while a worker/events consumer is still attached).
Code

src/jobs/sync-request-job.integration.test.ts[R25-27]

+  afterEach(async () => {
+    await Promise.all(cleanup.splice(0).map((close) => close()));
+  });
Relevance

●●● Strong

They’ve accepted BullMQ teardown robustness fixes; ordering cleanup to avoid flakiness is a
straightforward reliability improvement.

PR-#1638

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test registers cleanup callbacks for events, queue obliteration/close, and worker close, but
afterEach runs them all concurrently using Promise.all, so no shutdown ordering is guaranteed.

src/jobs/sync-request-job.integration.test.ts[22-39]
src/jobs/sync-request-job.integration.test.ts[76-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`afterEach` executes mixed BullMQ cleanup operations concurrently (QueueEvents close, queue obliterate/close, worker close). These operations can depend on each other, and running them in parallel makes teardown nondeterministic.

## Issue Context
The test pushes multiple cleanup callbacks into a shared `cleanup` array, then calls them with `Promise.all(...)`. Some callbacks close/obliterate the same underlying queue that other callbacks (worker/events) may still be using.

## Fix Focus Areas
- src/jobs/sync-request-job.integration.test.ts[22-40]
- src/jobs/sync-request-job.integration.test.ts[76-78]

## Suggested fix
Change teardown to be **sequential and dependency-ordered**, e.g.:
- Close workers first
- Then close QueueEvents
- Then obliterate and close queues

Implementation options:
- Replace `Promise.all(...)` with a `for...of` loop awaiting each close in an explicit order.
- Maintain separate arrays per resource type or store a structured object `{ worker, events, queue }` and close in a deterministic sequence.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Integration test skips test-helpers.ts 📘 Rule violation ▣ Testability
Description
The new integration test defines ad-hoc Redis/BullMQ setup helpers instead of reusing shared
utilities from src/test-helpers.ts, which increases duplication and inconsistency across
integration tests.
Code

src/jobs/sync-request-job.integration.test.ts[R1-41]

+import { randomUUID } from "node:crypto";
+import type { ConnectionOptions } from "bullmq";
+import { Queue, QueueEvents, Worker } from "bullmq";
+import { afterEach, describe, expect, it } from "vitest";
+import { registerSyncRequestQueryResolver } from "../lib/sync-request-query.ts";
+import { resolveWhoopSyncRequestQuery } from "../providers/whoop/sync-request-query.ts";
+import type { SyncJobData } from "./queues.ts";
+import { type EnqueuedSyncJob, enqueueSyncJobWithRequestDedup } from "./sync-request-job.ts";
+
+registerSyncRequestQueryResolver("whoop", resolveWhoopSyncRequestQuery);
+
+function testRedisConnection(): ConnectionOptions {
+  const parsed = new URL(process.env.REDIS_URL ?? "redis://localhost:6379");
+  return {
+    host: parsed.hostname,
+    port: Number(parsed.port || "6379"),
+    password: parsed.password || undefined,
+    maxRetriesPerRequest: null,
+  };
+}
+
+describe("full sync BullMQ lifecycle deduplication", () => {
+  const cleanup: Array<() => Promise<void>> = [];
+
+  afterEach(async () => {
+    await Promise.all(cleanup.splice(0).map((close) => close()));
+  });
+
+  function createQueue(suffix: string) {
+    const connection = testRedisConnection();
+    const queue = new Queue<SyncJobData>(`test-full-sync-${suffix}-${randomUUID()}`, {
+      connection,
+    });
+    const events = new QueueEvents(queue.name, { connection });
+    cleanup.push(async () => events.close());
+    cleanup.push(async () => {
+      await queue.obliterate({ force: true });
+      await queue.close();
+    });
+    return { connection, events, queue };
+  }
Relevance

●● Moderate

Repo recently accepted moving away from shared test-helpers toward local helpers; unclear they’d
require reuse here.

PR-#1677

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 721938 requires new/modified integration tests to use shared utilities from
test-helpers.ts rather than defining ad-hoc helpers. The added integration test file does not
import src/test-helpers.ts and instead introduces its own Redis connection and queue lifecycle
helpers, despite the presence of a shared src/test-helpers.ts module.

Rule 721938: Integration tests must use shared utilities from test-helpers.ts
src/jobs/sync-request-job.integration.test.ts[1-41]
src/test-helpers.ts[1-40]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added integration test (`*.integration.test.ts`) defines local infrastructure helpers (`testRedisConnection`, `createQueue`) and does not use shared utilities from `src/test-helpers.ts`, violating the integration-test helper reuse requirement.

## Issue Context
The repo contains a shared `src/test-helpers.ts` module intended to centralize common test infrastructure. This PR adds a new integration test with duplicated setup/teardown patterns.

## Fix Focus Areas
- src/jobs/sync-request-job.integration.test.ts[1-41]
- src/test-helpers.ts[1-40]
- src/jobs/bullmq-stall.integration.test.ts[16-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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 `@docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md`:
- Around line 3-4: Move the agent-specific execution guidance from the plan’s
opening block to the applicable AGENTS.md file. Keep the plan focused on
human-readable scope and task steps, preserving the checkbox tracking format
without agent-only instructions.

In `@packages/mobile/app/providers/index.test.tsx`:
- Around line 129-139: Update the Modal mock in the test setup to invoke the
provided onShow callback after mounting, while continuing to omit it from the
rendered props. Add or update the confirmation-modal test to assert
AccessibilityInfo.setAccessibilityFocus receives the cancel control’s node,
using the existing findNodeHandle and mock symbols.

In `@packages/mobile/app/providers/sync-all-controls.tsx`:
- Around line 29-36: Update the focusCancel callback to remove findNodeHandle
and AccessibilityInfo.setAccessibilityFocus, and invoke
AccessibilityInfo.sendAccessibilityEvent with cancelRef.current and the focus
event. Mark the Cancel TouchableOpacity as accessible={true} so the ref-based
accessibility API targets it correctly.

In `@packages/providers-meta/src/sync-actions.ts`:
- Around line 1-7: Keep the routine sync window canonical by interpolating
ROUTINE_SYNC_DAYS into both recent-action strings in
packages/providers-meta/src/sync-actions.ts (lines 1-7), and update the
corresponding assertions in packages/providers-meta/src/sync-actions.test.ts
(lines 5-10) to use `${ROUTINE_SYNC_DAYS}` instead of hard-coded 7.

In `@src/jobs/sync-request-job.integration.test.ts`:
- Around line 13-19: Update the Redis configuration parsing near the URL
construction to require process.env.REDIS_URL, failing immediately with an
explicit “REDIS_URL is required” error when it is absent; preserve the existing
host, port, password, and retry-option mapping for a provided URL.
- Around line 25-39: Update the afterEach teardown to execute cleanup callbacks
sequentially in reverse registration order instead of using Promise.all.
Preserve the existing cleanup.splice(0) behavior while ensuring each callback,
including worker.close() and queue obliteration/closure registered in
createQueue, completes before the next runs.
🪄 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: 4ce50f1f-6208-43fc-9897-62f651905145

📥 Commits

Reviewing files that changed from the base of the PR and between a11b457 and e2faef6.

📒 Files selected for processing (22)
  • docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md
  • packages/mobile/app/providers/index.test.tsx
  • packages/mobile/app/providers/index.tsx
  • packages/mobile/app/providers/styles.ts
  • packages/mobile/app/providers/sync-all-controls.stories.tsx
  • packages/mobile/app/providers/sync-all-controls.test.tsx
  • packages/mobile/app/providers/sync-all-controls.tsx
  • packages/providers-meta/package.json
  • packages/providers-meta/src/sync-actions.test.ts
  • packages/providers-meta/src/sync-actions.ts
  • packages/server/src/routers/sync.test.ts
  • packages/server/src/routers/sync.ts
  • packages/web/src/components/DataSourcesPanel.test.tsx
  • packages/web/src/components/DataSourcesPanel.tsx
  • packages/web/src/components/SyncAllControls.stories.tsx
  • packages/web/src/components/SyncAllControls.test.tsx
  • packages/web/src/components/SyncAllControls.tsx
  • src/jobs/enqueue-sync-job.test.ts
  • src/jobs/enqueue-sync-job.ts
  • src/jobs/sync-request-job.integration.test.ts
  • src/jobs/sync-request-job.test.ts
  • src/jobs/sync-request-job.ts
💤 Files with no reviewable changes (1)
  • packages/mobile/app/providers/styles.ts

Comment thread docs/superpowers/plans/2026-07-29-sync-action-hierarchy.md Outdated
Comment thread packages/mobile/app/providers/index.test.tsx Outdated
Comment thread packages/mobile/app/providers/sync-all-controls.tsx
Comment thread packages/providers-meta/src/sync-actions.ts Outdated
Comment thread src/jobs/sync-request-job.integration.test.ts Outdated
Comment thread src/jobs/sync-request-job.integration.test.ts
Comment thread src/jobs/sync-request-job.integration.test.ts
Comment thread src/jobs/enqueue-sync-job.ts
Comment thread src/jobs/sync-request-job.integration.test.ts
@codereviewbot-ai

Copy link
Copy Markdown

🤖 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.

@Asherlc

Asherlc commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Review root-cause note for the full-sync single-flight finding:

The finding is valid. The current BullMQ simple-mode key is owned by only the initial job. processSyncJob can enqueue a checkpoint continuation and then complete after continued: true, so BullMQ releases the key while the logical full-history operation is still active. Giving the continuation the same simple-mode key is not a fix: it is enqueued before the parent completes and BullMQ would deduplicate/discard it.

I have paused implementation under the repository strategy-pivot gate. The proposed replacement is an operation-scoped Redis lease keyed by provider + user, with an owner token propagated through continuations, compare-owner refresh/release, terminal-only release, and bounded expiry for abandoned operations. Required failure tests include enqueue failure after acquire, process crash/lease expiry, stale owner release, continuation enqueue failure, terminal success/failure, and a second request during the continuation chain.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 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.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 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.

@Asherlc

Asherlc commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Fixed the branch-owned web layout regression in b9b2ceb. The provider query initially rendered no bulk controls, then inserted the 76px action stack after resolution and shifted the Data Sources section by 52px. The heading row now reserves the existing 80px spacing token throughout loading, and the delayed-query RTL test resolves two providers to cover the conditional controls. Focused unit tests (19/19), web typecheck, Biome, and diff checks pass.

@Asherlc
Asherlc enabled auto-merge (squash) July 30, 2026 13:31
@Asherlc
Asherlc merged commit 9b92788 into main Jul 30, 2026
52 checks passed
@Asherlc
Asherlc deleted the issue-2178-sync-action-hierarchy branch July 30, 2026 13:39
@codereviewbot-ai

Copy link
Copy Markdown

🤖 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.

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.

[SET-05] Sync All and Full Sync All have equal weight without explaining impact

2 participants