feat(kit): async product sync with job queue, dry-run, and purge-local#127
Conversation
Replace the synchronous push-sync actions with a background-job model. The previous public actions held the dashboard's HTTP fetch open for the entire ASC / Play catalog walk, which iOS Safari aborted on cellular and backgrounded tabs as `TypeError: Load failed` (Sentry 486194d4, PR #124 follow-up from LukasB-DEV's report). - New `productSyncJobs` table stores queued/running/succeeded/failed state plus `progress.phase`, `result`, `error`, `cancelRequested`, `expectedDeadline`. Indexes back the dashboard's reactive `getActiveSyncJob` query, the reaper, and the pruner. - `enqueueProductSync` mutation returns immediately with `{ jobId, deduped }`; idempotent on (project, platform). Schedules the appropriate worker via `ctx.scheduler.runAfter`. - `runProductSyncIOS` / `runProductSyncAndroid` internalActions drive the existing pull/push pipelines with phase-boundary cancel checks (PULL.iaps -> PULL.subscriptions -> PUSH.drafts). - New `direction: "purge-local"` empties kit's local catalog for a platform without touching the upstream store. Confirm dialog warns about lost unpushed edits and permanently-deleted Draft rows before proceeding. - Android `performAndroidSync` now supports dryRun across all four PUSH paths (subscription patch / IAP patch / subscription create+activate / IAP insert), returning `plannedWrites` so operators can preview Play Console changes the same way they already could for App Store Connect. - New `Tooltip` component replaces the inline hover popover with a reusable component; explicit opaque surfaces fix transparency that let table content bleed through. - `HorizonCatalogNotice` banner surfaces the Meta API constraint (no catalog REST endpoint) so Horizon-enabled projects don't look for a missing sync button. - `Toaster closeButton` lets operators dismiss notifications. - Sentry `beforeSend` tags `TypeError: Load failed` events from Convex reconnects with `source: convex-reconnect` and a stable fingerprint so triage can downsample the noise. - `reapStaleProductSyncJobs` cron flips workers stuck past the 9-min deadline to failed; `pruneProductSyncJobs` cron drops succeeded rows after 7d / failed after 30d. - Fix: `files/internal.ts` `readFileAsBase64` was using `Buffer` in a V8 isolate runtime where it isn't a global, throwing `ReferenceError: Buffer is not defined` on every download. Replaced with chunked `btoa(String.fromCharCode(...))`. - Docs: kit-backend page documents async polling + cancel endpoints; CONVENTION.md adds a "Long-running operations" section documenting the pattern for future workers. Tests: jobs.test.ts covers truncateFailures + retention-constant invariants. 346/346 vitest, typecheck/lint/format clean, smoke:server green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/gemini review |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughA new long-running product sync architecture replaces direct synchronous sync actions with asynchronous Convex Jobs that enqueue background workers, report progress, support cancellation, and persist results. HTTP endpoints return 202 with job IDs; clients poll for status. Workers for iOS (ASC) and Android (Google Play) now run as internal actions governed by job state. A UI overhaul reflects this model through reactive job querying and controls. ChangesLong-Running Product Sync Jobs
Sequence DiagramsequenceDiagram
participant Client
participant API as HTTP API
participant Convex as Convex Backend
participant iOS as iOS Worker
participant Android as Android Worker
participant Cron as Scheduled Crons
Client->>API: POST /sync/ios<br/>{direction, dryRun}
API->>Convex: enqueueProductSync(platform, direction, dryRun)
Convex->>Convex: Check for existing queued/running job<br/>(dedup logic)
Convex->>Convex: Insert job record (queued status)
Convex->>Convex: Schedule iOS worker action
Convex-->>API: {jobId, deduped}
API-->>Client: HTTP 202 {jobId, deduped}
Client->>API: GET /sync/jobs/{jobId} (poll)
API->>Convex: getSyncJobById(jobId)
Convex-->>API: Job {status, progress, result}
API-->>Client: {status, progress, result}
par Worker Execution
Convex->>iOS: runProductSyncIOS(jobId)
iOS->>Convex: isCancelRequested?
Convex-->>iOS: false
iOS->>iOS: PULL phase<br/>(checkCancelled, reportPhase)
iOS->>Convex: updateJobProgress(phase: "pull")
iOS->>Convex: PUSH phase<br/>(plan/draft/promote)
iOS->>Convex: markJobSucceeded(pulled, pushed, failures)
and
Convex->>Android: runProductSyncAndroid(jobId)
Android->>Convex: isCancelRequested?
Convex-->>Android: false
Android->>Android: PULL phase
Android->>Convex: updateJobProgress(phase: "pull")
Android->>Android: PUSH phase<br/>(plan/patch/create)
Android->>Convex: markJobSucceeded(pulled, pushed, plannedWrites)
end
Client->>API: POST /sync/jobs/{jobId}/cancel
API->>Convex: cancelProductSync(jobId)
Convex->>Convex: Set cancelRequested: true
Worker->>Convex: isCancelRequested?
Convex-->>Worker: true
Worker->>Convex: markJobFailed(error: "Cancelled")
Worker-->>Client: Job status → failed
Cron->>Convex: reapStaleProductSyncJobs() (every 5 min)
Convex->>Convex: Find running jobs<br/>past expectedDeadline + grace
Convex->>Convex: Mark as failed (timeout)
Cron->>Convex: pruneProductSyncJobs() (every 6 hours)
Convex->>Convex: Delete old succeeded/failed jobs<br/>past retention windows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
🚥 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. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the product sync process for App Store Connect and Google Play from synchronous actions to an asynchronous background job system. This change addresses browser-side fetch timeouts, particularly on iOS Safari, by introducing a job-based architecture involving a new productSyncJobs table, internal worker actions, and maintenance crons. The REST API and dashboard UI have been updated to handle job enqueuing, state polling, and cancellation. Additional improvements include a portable base64 encoding fix for V8 isolates, a custom tooltip component, and Sentry filtering for transient network errors. Feedback focused on improving code maintainability by recommending dedicated interfaces for complex function signatures in the sync helpers and the use of named constants for batch sizes in cron tasks.
There was a problem hiding this comment.
Pull request overview
This PR migrates kit’s product sync from long-running synchronous actions to an async job-queue model backed by a new productSyncJobs table, enabling progress reporting, cancellation, retention/reaping, and “purge-local” catalog resets. It also updates the dashboard UI to reflect job state reactively, adds a reusable Tooltip, reduces Sentry noise from Convex reconnect fetch failures, and fixes base64 encoding in Convex isolate runtime.
Changes:
- Introduce
productSyncJobsschema + enqueue/cancel/dismiss/job lifecycle logic (workers, progress, retention, reaper/pruner crons). - Update dashboard Products page to enqueue jobs (incl. dry-run and purge-local), show progress/result banners, and provide cancel/dismiss UX.
- Add Tooltip component, Toaster close button support, Sentry
beforeSendtagging/fingerprinting for Convex reconnect “Load failed”, and fixreadFileAsBase64for isolate runtime.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/kit/src/pages/auth/organization/project/products.tsx | Switch UI to job-based sync with progress/result banners, dry-run/purge-local/cancel/dismiss actions, and Horizon notice. |
| packages/kit/src/lib/sentry.ts | Tag and fingerprint Convex reconnect “Load failed” noise in beforeSend. |
| packages/kit/src/components/Tooltip.tsx | Add a lightweight tooltip component with opaque surface styling. |
| packages/kit/src/components/AuthTransition.tsx | Enable toast close buttons via Toaster closeButton. |
| packages/kit/server/api/v1/products.ts | Replace sync endpoint with job enqueue + add job poll/cancel routes. |
| packages/kit/convex/schema.ts | Add productSyncJobs table + indexes. |
| packages/kit/convex/projects/internal.ts | Add internal getProjectById for worker job → project lookup. |
| packages/kit/convex/products/sync.ts | Add bounded platform-catalog delete mutation for purge-local. |
| packages/kit/convex/products/play.ts | Replace synchronous Play sync action with job worker + Android dry-run plannedWrites support. |
| packages/kit/convex/products/jobs.ts | New job lifecycle module: enqueue/dedup, cancel, dismiss, progress, reaper/pruner, purge-local worker. |
| packages/kit/convex/products/jobs.test.ts | Unit tests for failure truncation and retention constant sanity checks. |
| packages/kit/convex/products/asc.ts | Replace synchronous ASC sync action with job worker + progress/cancel support. |
| packages/kit/convex/files/internal.ts | Fix base64 encoding in isolate runtime (remove Buffer dependency). |
| packages/kit/convex/crons.ts | Add reaper/pruner intervals for product sync jobs. |
| packages/kit/convex/_generated/api.d.ts | Include generated typings for products/jobs. |
| packages/kit/CONVENTION.md | Document the long-running background job pattern for kit. |
| packages/docs/src/pages/docs/kit-backend.tsx | Update docs to describe async sync job endpoints and polling model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hyochan
added
🛠 bugfix
There was a problem hiding this comment.
Code Review
This pull request migrates the product sync process for App Store Connect and Google Play from synchronous actions to an asynchronous background job system to prevent browser-side timeouts. The implementation introduces a productSyncJobs table, dedicated worker actions with cancellation and progress reporting, and maintenance crons for reaping and pruning jobs. The dashboard UI has been updated to support this lifecycle, including progress visualization and dry-run previews. Additionally, the PR fixes a runtime error in base64 encoding within V8 isolates and introduces a custom tooltip component. Feedback was provided regarding the optimization of the getActiveSyncJob query by utilizing a composite index to avoid linear scans.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
packages/kit/convex/files/internal.ts (1)
195-198: 💤 Low valueDrop the
as unknown as number[]double cast in favor of spread.
Uint8Arrayis iterable, soString.fromCharCode(...bytes.subarray(i, i + CHUNK))produces the same result without the type-system escape hatch and is the more idiomatic form here. The double cast (as unknown as number[]) silences the compiler rather than expressing intent and can mask future regressions if the underlying expression changes. Behavior is identical: chunked spreads of 32768 bytes stay well below V8's argument-count limit.♻️ Proposed refactor
const arrayBuffer = await blob.arrayBuffer(); const bytes = new Uint8Array(arrayBuffer); const CHUNK = 0x8000; let binary = ""; for (let i = 0; i < bytes.length; i += CHUNK) { - binary += String.fromCharCode.apply( - null, - bytes.subarray(i, i + CHUNK) as unknown as number[], - ); + binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK)); } const base64 = btoa(binary);🤖 Prompt for 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. In `@packages/kit/convex/files/internal.ts` around lines 195 - 198, Replace the double-cast usage in the string-concatenation chunk where you build `binary` from `bytes` (the line using String.fromCharCode.apply with `bytes.subarray(i, i + CHUNK) as unknown as number[]`) by using the iterable spread instead; call String.fromCharCode with a spread of the subarray (i.e., String.fromCharCode(...bytes.subarray(i, i + CHUNK))) so you remove the `as unknown as number[]` type escape while preserving the chunking behavior with `CHUNK` and `bytes`.packages/kit/src/components/Tooltip.tsx (1)
18-30: ⚡ Quick winUse an interface + JSDoc for the exported Tooltip API.
The component currently uses inline props object definition and lacks a JSDoc comment on the exported function. Extract the props to a
TooltipPropsinterface and add a JSDoc block to match repo TypeScript API conventions (per guidelines: "Prefer interface for defining object shapes in TypeScript" and "Add JSDoc comments for public functions and exported APIs").♻️ Suggested patch
-type Side = "top" | "bottom" | "left" | "right"; -type Align = "start" | "center" | "end"; +type Side = "top" | "bottom" | "left" | "right"; +type Align = "start" | "center" | "end"; + +interface TooltipProps { + children: ReactNode; + content: ReactNode; + side?: Side; + align?: Align; + widthClass?: string; +} @@ -export function Tooltip({ +/** + * Lightweight hover/focus tooltip wrapper for inline trigger elements. + */ +export function Tooltip({ children, content, side = "bottom", align = "end", widthClass = "w-80", -}: { - children: ReactNode; - content: ReactNode; - side?: Side; - align?: Align; - widthClass?: string; -}) { +}: TooltipProps) {🤖 Prompt for 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. In `@packages/kit/src/components/Tooltip.tsx` around lines 18 - 30, Extract the inline props type into an exported interface named TooltipProps (including children, content, side, align, widthClass with their types and defaults documented) and update the Tooltip function signature to accept props: TooltipProps; add a JSDoc block above the exported Tooltip function describing the component, its props, and default values (mention children, content, side default "bottom", align default "end", widthClass default "w-80") so the API follows repository TS conventions; ensure exported interface name and the Tooltip function are used consistently.
🤖 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 `@packages/kit/convex/products/jobs.ts`:
- Around line 386-435: The progress.failuresCount uses different semantics
between markJobSucceeded and markJobFailed: change markJobFailed to set
progress.failuresCount to the rawFailures length (const rawFailures =
args.failures ?? []; use rawFailures.length) so it matches markJobSucceeded's
"true count" behavior; keep using truncateFailures(rawFailures) to populate
result.failures and failuresTruncated but ensure progress.failuresCount
references rawFailures.length, not failures.length.
- Around line 297-314: The comment above the dismissCompletedJob mutation is
stale and mentions shifting completedAt and a dismissed:true field that don't
exist; update the comment to accurately describe the implemented approach: this
handler checks the job status, and when completed sets progress.phase =
"dismissed" (used by the client to mark dismissed jobs) instead of altering
completedAt or adding a dismissed field, and keep the note about reusing
cancelRequested vs. a dismissal marker if desired; locate the comment
immediately above dismissCompletedJob and rewrite it to reflect the actual
behavior and rationale.
In `@packages/kit/convex/products/sync.ts`:
- Around line 359-373: Ensure the mutation validates that args.limit is a
positive integer before using it in handler: check the incoming args.limit (used
where page = ... .take(args.limit + 1), hasMore and toDelete are computed) and
either throw a validation error or coerce to a minimum of 1 so you never call
.take(0) or compute hasMore that deletes zero rows; update the args validation
or add an early guard in the handler to enforce limit > 0 and reject or adjust
invalid values.
In `@packages/kit/server/api/v1/products.ts`:
- Around line 160-163: The type cast for the request query variable direction
currently omits "purge-local"; update the cast expression where direction is
declared so it includes "purge-local" in the union (i.e.
(c.req.query("direction") as "pull" | "push" | "both" | "purge-local" |
undefined) ?? "both") so the local TS type matches the Convex directionValidator
and the documented supported values; keep the default "both" behavior unchanged.
In `@packages/kit/src/lib/sentry.ts`:
- Around line 53-60: The Convex noise classifier currently builds `message` from
`exception` (from `hint.originalException`) and checks `looksConvex` using
`message + " " + (event.request?.url ?? "")`; update it to fallback to
`event.message`/`event.logentry?.message` when `hint.originalException` is not
present so events that only carry an event-level message are detected. In
practice, in the same block that computes `message` and `isFetchLoadFailed`,
augment the source string used for `looksConvex` to include `event.message ||
event.logentry?.message` (in addition to the existing `message` and
`event.request?.url`) before running the
`/convex\.cloud|\/api\/(action|query|mutation)/i` test. Ensure references to
`message`, `looksConvex`, and `event.request?.url` remain and that the fallback
is used only when the original exception-derived message is empty.
In `@packages/kit/src/pages/auth/organization/project/products.tsx`:
- Around line 91-137: The toasts are re-shown after a reload because dismissal
sets progress.phase = "dismissed" but status remains terminal; update the
useEffect that watches iosJob/androidJob to skip showing any toast if
job.progress?.phase === "dismissed" (i.e., treat dismissed as
terminal-no-toast). Concretely, inside the loop (where you currently compute
terminal and check lastShownJobIdRef), add a guard like if (job.progress?.phase
=== "dismissed") continue; so iosJob/androidJob toasts are only shown for
terminal jobs that are not dismissed; keep references to iosJob, androidJob,
lastShownJobIdRef and progress.phase when implementing.
---
Nitpick comments:
In `@packages/kit/convex/files/internal.ts`:
- Around line 195-198: Replace the double-cast usage in the string-concatenation
chunk where you build `binary` from `bytes` (the line using
String.fromCharCode.apply with `bytes.subarray(i, i + CHUNK) as unknown as
number[]`) by using the iterable spread instead; call String.fromCharCode with a
spread of the subarray (i.e., String.fromCharCode(...bytes.subarray(i, i +
CHUNK))) so you remove the `as unknown as number[]` type escape while preserving
the chunking behavior with `CHUNK` and `bytes`.
In `@packages/kit/src/components/Tooltip.tsx`:
- Around line 18-30: Extract the inline props type into an exported interface
named TooltipProps (including children, content, side, align, widthClass with
their types and defaults documented) and update the Tooltip function signature
to accept props: TooltipProps; add a JSDoc block above the exported Tooltip
function describing the component, its props, and default values (mention
children, content, side default "bottom", align default "end", widthClass
default "w-80") so the API follows repository TS conventions; ensure exported
interface name and the Tooltip function are used consistently.
🪄 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
Run ID: b0c20407-6ee4-4795-b9bc-ff9821304d33
⛔ Files ignored due to path filters (1)
packages/kit/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (16)
packages/docs/src/pages/docs/kit-backend.tsxpackages/kit/CONVENTION.mdpackages/kit/convex/crons.tspackages/kit/convex/files/internal.tspackages/kit/convex/products/asc.tspackages/kit/convex/products/jobs.test.tspackages/kit/convex/products/jobs.tspackages/kit/convex/products/play.tspackages/kit/convex/products/sync.tspackages/kit/convex/projects/internal.tspackages/kit/convex/schema.tspackages/kit/server/api/v1/products.tspackages/kit/src/components/AuthTransition.tsxpackages/kit/src/components/Tooltip.tsxpackages/kit/src/lib/sentry.tspackages/kit/src/pages/auth/organization/project/products.tsx
Address all 15 unresolved review threads on PR #127: - **Auth (Copilot, critical)**: HTTP server's `enqueueProductSync` / `getSyncJobById` / `cancelProductSync` calls were going through `getAuthUserId`, which fails server-to-server (no logged-in user). Replace with apiKey-only auth via `resolveProjectByApiKey` / `resolveJobByApiKey` (matching the existing `upsertProduct` pattern). `(apiKey, jobId)` is verified together so a stolen jobId from another project can't be cancelled / read cross-project. Logged-in users still get membership-checked, with `createdBy` recorded best-effort. - **Schema index (Gemini)**: add `by_project_platform_created` composite index and switch `getActiveSyncJob` to use it — replaces the prior `by_project_and_created` + in-memory `.filter(platform)` which scanned every job for the project. - **Purge loop guard (CodeRabbit, critical)**: `deletePlatformCatalog` now throws when `limit < 1` to prevent a non-progressing purge worker that returns `hasMore: true` while deleting zero rows. - **failuresCount semantics (CodeRabbit)**: `markJobFailed` now records `progress.failuresCount` from the raw upstream count (not the post-truncation slice length) — matches `markJobSucceeded` semantics so analytics readers don't see inconsistent values. - **O(n²) string concat (Copilot)**: `readFileAsBase64` now accumulates chunks in an array and `join("")`s once, instead of growing a `binary` string per iteration. Avoids quadratic memory thrash on multi-MB uploads. - **Sentry classifier (CodeRabbit)**: `beforeSend` now also reads `event.message` and `event.logentry?.message`, so reconnect events that arrive without `originalException` still get tagged. - **Toast on dismiss (CodeRabbit)**: products.tsx toast effect now skips terminal jobs with `progress.phase === "dismissed"` so the success/failure toast doesn't re-fire after every page reload. - **Direction cast (CodeRabbit)**: HTTP route's TS cast on `direction` query param now includes `"purge-local"`. - **Reaper batch (Gemini)**: `PRODUCT_SYNC_REAPER_BATCH = 50` and `PRODUCT_SYNC_PRUNER_BATCH = 100` named constants replace the hardcoded `.take(50)` / `.take(100)` calls. - **Sync options (Gemini)**: extracted `IosSyncOptions` / `AndroidSyncOptions` interfaces to clean up the long `performIosSync` / `performAndroidSync` signatures. - **Stale comment (Copilot, CodeRabbit)**: rewritten `dismissCompletedJob` doc to match the actual `progress.phase = "dismissed"` implementation (no `completedAt` shifting, no `dismissed: true` field). Tests: jobs.test.ts unchanged, 346/346 vitest still passing. Typecheck / lint / format / smoke:server all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request migrates the product synchronization process to an asynchronous background job system to prevent browser timeouts, introducing a productSyncJobs table, background workers, and a polling mechanism. Key updates include refactoring sync logic into internal actions, implementing cron jobs for task reaping and pruning, and enhancing the dashboard UI with progress tracking and cancellation support. It also fixes a base64 encoding bug in V8 isolates and improves Sentry noise filtering. Feedback highlights a missing currency validation in the Android sync path and low-contrast text colors in the UI result banner that need theme-aware adjustments.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address the second pass of Copilot/Gemini findings on PR #127: - **Atomic enqueue dedup (Copilot critical)**: the prior index-based dedup wasn't transactional — two concurrent `enqueueProductSync` mutations could both observe an empty `productSyncJobs` index range, both insert separate rows, and fan out conflicting workers. Replaced with an `activeSyncJobIds: { IOS?, Android? }` field on `projects`. Reading + patching that field forces Convex's OCC to collapse the two callers onto one job — only one commit wins, the loser retries, sees the lock, and returns deduped. Worker terminations (`markJobSucceeded` / `markJobFailed` / reaper) clear the matching lock entry so the next enqueue can claim the slot. - **Android USD guard (Gemini)**: Play's `regionalConfigs` requires `currencyCode` to match `regionCode`. Pushing `regionCode: "US"` with a non-USD price returns a generic 400 from the API. Match the iOS path's currency-validation pattern and surface an actionable failure when the row's currency != USD before the request fires (dry-run + real run both validate). - **Modal promoted to shared component (Copilot)**: `AuthModal/Modal.tsx` moved to `src/components/Modal.tsx` with `showCloseButton` and `contentClassName` props so the destructive `PurgeConfirmDialog` reuses the focus trap, escape handler, scroll lock, and focus-restore behavior instead of re-implementing them inline (the prior dialog let keyboard users tab into background controls while a destructive confirm was open). AuthModal updated to import from the new location. - **Tooltip a11y (Copilot)**: trigger now gets `aria-describedby` pointing at a stable `useId`-generated tooltip id so screen readers announce the help text alongside the button label. `onFocus` paired with `onBlurCapture` + a `relatedTarget` containment check keeps the tooltip open while focus moves between descendants of the wrapper instead of closing on every bubbled blur. - **Cancel toast (Copilot)**: `onCancel` now branches on the mutation's returned `{ ok, reason }`. Job already completed between render and click → "sync already finished"; otherwise → "cancellation requested". The misleading "cancellation requested" toast on no-op cancels is gone. - **Light-mode contrast (Gemini)**: result banner + reset button + amber warning + delete button colors now have `text-{color}-700 dark:text-{color}-200` pairs. The earlier dark-only palette (`text-rose-200` etc.) was unreadable in light mode against the bg-tint surface. 346/346 vitest, typecheck/lint/format clean, smoke:server green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request transitions the product sync process from a synchronous action to an asynchronous background job system to prevent browser timeouts, specifically addressing iOS Safari's TypeError: Load failed. It introduces a new productSyncJobs table, worker actions for iOS and Android, and a polling mechanism for the dashboard and API. Key changes include atomic job deduping using a project-level lock, a "purge-local" recovery feature, and a shared Modal component. Feedback focuses on improving the robustness of base64 encoding for large files to avoid stack size limits and ensuring idiomatic clearing of nested lock fields in Convex patch calls to avoid leaving empty objects in the database.
| activeSyncJobIds: { | ||
| ...project.activeSyncJobIds, | ||
| [job.platform]: undefined, | ||
| }, |
There was a problem hiding this comment.
In Convex, setting a field to undefined within a patch call causes that field to be ignored during the merge. While this works for top-level fields to omit them from the update, when used inside a nested object like activeSyncJobIds, it results in the key being omitted from the new object value. This effectively clears the lock, but it leaves an empty object {} in the database if no other platform locks are present. A more explicit approach using delete or setting the specific platform key to undefined on a copy of the object is preferred for clarity.
const nextActiveSyncJobIds = { ...project.activeSyncJobIds };
delete nextActiveSyncJobIds[job.platform];
await ctx.db.patch(project._id, {
activeSyncJobIds: nextActiveSyncJobIds,
});| ...project.activeSyncJobIds, | ||
| [job.platform]: undefined, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Similar to the success path, clearing the lock by spreading and setting a key to undefined relies on Convex's behavior of ignoring undefined values in objects. Using delete on a copy of the object is more idiomatic for removing keys from a nested map-like structure.
const nextActiveSyncJobIds = { ...project.activeSyncJobIds };
delete nextActiveSyncJobIds[job.platform];
await ctx.db.patch(project._id, {
activeSyncJobIds: nextActiveSyncJobIds,
});| ...project.activeSyncJobIds, | ||
| [job.platform]: undefined, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The reaper should also use an explicit key deletion to clear the lock, ensuring consistency with the success and failure terminal states.
const nextActiveSyncJobIds = { ...project.activeSyncJobIds };
delete nextActiveSyncJobIds[job.platform];
await ctx.db.patch(project._id, {
activeSyncJobIds: nextActiveSyncJobIds,
});| String.fromCharCode.apply( | ||
| null, | ||
| bytes.subarray(i, i + CHUNK) as unknown as number[], | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
While String.fromCharCode.apply is a common pattern for converting byte arrays to strings, it is subject to stack size limits in many JavaScript engines (including V8). Although the CHUNK size of 0x8000 (32,768) is generally safe, very large files could still potentially hit limits if the environment's stack is constrained. Using TextDecoder (if available in the runtime) or a manual loop to build the string from the Uint8Array is more robust for arbitrary file sizes.
for (let i = 0; i < bytes.length; i += CHUNK) {
const subarray = bytes.subarray(i, i + CHUNK);
let chunk = "";
for (let j = 0; j < subarray.length; j++) {
chunk += String.fromCharCode(subarray[j]);
}
chunks.push(chunk);
}
…ASC private key `resolveAscCredentials` required both `iosAscIssuerId` AND `iosAscKeyId` to be populated before using the ASC pair. The Settings UI deliberately exposes only ONE Issuer ID input (documented as "shared with Server API above") because Apple uses a single Issuer per team across both API gateways — so production projects always have `iosAscIssuerId = undefined` and `iosAscKeyId = "<asc team key id>"`. Old gate (`iosAscIssuerId && iosAscKeyId`) returned false → fallback path used `iosAppStoreKeyId` (the In-App Purchase / Server API key id) but signed the JWT with the ASC private key content (loaded via `getAppleAscApiKey`). Apple rejected every request with 401 because `kid` claim didn't match the signing key. Affects every production tenant; reported by LukasB-DEV in the PR #127 thread, reproduced by hyochan on kit.openiap.dev ("localhost works, prod doesn't" — dev DBs happened to have matching values from earlier UI iterations). Fix: gate on `iosAscKeyId` alone, fall back issuer to `iosAppStoreIssuerId` when the dedicated ASC issuer slot is empty. Behavior: - ASC slot has key id → ASC pair (`issuerId`: ASC slot ?? legacy shared, `keyId`: ASC slot, `.p8`: ASC slot ?? legacy) - ASC slot empty → legacy Server API pair end-to-end (back-compat for projects that uploaded the ASC key into the legacy slot before the second slot existed) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
pushSyncProducts*actions that iOS Safari aborted asTypeError: Load failedon long catalogs — Sentry 486194d4, fix for @LukasB-DEV's report on PR #124).productSyncJobstable +enqueueProductSyncmutation that returns immediately, workers (runProductSyncIOS/runProductSyncAndroid/runProductSyncPurgeLocal) write progress + result back to the row, dashboard subscribes reactively. Cancel + dismiss + reaper + pruner all wired up.direction: "purge-local"resets kit's local catalog without touching upstream stores; confirm dialog warns about lost unpushed edits and permanently-deleted Draft rows.dryRunparity with iOS — all four PUSH paths returnplannedWritesso operators can preview Play changes the same way they preview ASC changes.Tooltipcomponent (opaque surfaces fix bg-popover transparency);HorizonCatalogNoticebanner explains Meta's catalog-API absence;Toaster closeButtonenables toast dismissal.beforeSendtags Convex reconnectLoad failedevents withsource: convex-reconnect+ stable fingerprint to downsample noise.files/internal.tsreadFileAsBase64was usingBufferin a V8 isolate runtime where it isn't a global, throwingReferenceError: Buffer is not definedon every download. Replaced with chunkedbtoa(String.fromCharCode(...)).Surface map (HTTP)
Schema
productSyncJobs:Indexes:
(projectId, platform, status)for active-job lookup,(projectId, createdAt)for dashboard,(status, expectedDeadline)for reaper,(status, completedAt)for pruner.Crons
reapStaleProductSyncJobs(5 min) — flipsrunningrows pastexpectedDeadline + 1min graceto failedpruneProductSyncJobs(6 h) — succeeded > 7d, failed > 30d → deletedTest plan
bun run --filter @hyodotdev/openiap-kit typecheck— 0 errorsbun run --filter @hyodotdev/openiap-kit lint— 0 issuesbun run --filter @hyodotdev/openiap-kit test— 346/346 (5 new injobs.test.ts)bun run --filter @hyodotdev/openiap-kit format:check— cleanbun run --filter @hyodotdev/openiap-kit smoke:server— greenTypeError: Load failedfrom/api/actionfor kit dashboard sessions🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes