From 83d18fc7fe69ab709b21c06561108ea878d97c02 Mon Sep 17 00:00:00 2001 From: hyochan Date: Tue, 5 May 2026 18:34:59 +0900 Subject: [PATCH 1/3] feat(kit): async product sync with job queue, dry-run, and purge-local 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) --- packages/docs/src/pages/docs/kit-backend.tsx | 45 +- packages/kit/CONVENTION.md | 53 + packages/kit/convex/_generated/api.d.ts | 2 + packages/kit/convex/crons.ts | 19 + packages/kit/convex/files/internal.ts | 21 +- packages/kit/convex/products/asc.ts | 1274 +++++++++-------- packages/kit/convex/products/jobs.test.ts | 61 + packages/kit/convex/products/jobs.ts | 487 +++++++ packages/kit/convex/products/play.ts | 968 +++++++------ packages/kit/convex/products/sync.ts | 31 + packages/kit/convex/projects/internal.ts | 15 +- packages/kit/convex/schema.ts | 80 ++ packages/kit/server/api/v1/products.ts | 88 +- .../kit/src/components/AuthTransition.tsx | 4 +- packages/kit/src/components/Tooltip.tsx | 96 ++ packages/kit/src/lib/sentry.ts | 24 + .../auth/organization/project/products.tsx | 592 ++++++-- 17 files changed, 2742 insertions(+), 1118 deletions(-) create mode 100644 packages/kit/convex/products/jobs.test.ts create mode 100644 packages/kit/convex/products/jobs.ts create mode 100644 packages/kit/src/components/Tooltip.tsx diff --git a/packages/docs/src/pages/docs/kit-backend.tsx b/packages/docs/src/pages/docs/kit-backend.tsx index bafa0ca8..a999b5ea 100644 --- a/packages/docs/src/pages/docs/kit-backend.tsx +++ b/packages/docs/src/pages/docs/kit-backend.tsx @@ -183,14 +183,49 @@ if status.active:

- The{' '} + Sync is asynchronous —{' '} POST /v1/products/{apiKey}/sync/{ios|android} {' '} - endpoint returns the count of pulled / pushed rows plus per-product - failure messages so the dashboard surfaces upstream rejections - (price-tier conflicts, locale issues, missing review notes) without - dropping silent failures. + enqueues a job and returns { jobId, deduped }{' '} + with HTTP 202 immediately. The actual catalog walk (App Store Connect + REST or Play Developer API) runs in the background as a Convex + internalAction, writing progress and the final result back to a{' '} + productSyncJobs row. Earlier kit versions held the HTTP + connection open for the entire sync, which iOS Safari aborted on + cellular / backgrounded tabs as TypeError: Load failed. +

+

Clients poll the job state:

+ +

+ Backoff polls at ~3s intervals until status is{' '} + succeeded or failed. Most catalogs finish in + tens of seconds; large ones in 1–2 minutes. A{' '} + reapStaleProductSyncJobs cron flips workers stuck past + the 9-minute deadline to failed so a crashed action can't + pin the project's "active job" slot.

diff --git a/packages/kit/CONVENTION.md b/packages/kit/CONVENTION.md index 2f642271..784de0ad 100644 --- a/packages/kit/CONVENTION.md +++ b/packages/kit/CONVENTION.md @@ -151,6 +151,59 @@ boots it on port 3100, and probes `/health`, `/`, `/api/v1` — catches startup regressions (missing env, bind conflicts, missing `dist/index.html`). +## Long-running operations + +Convex actions cap at ~10 minutes; the browser fetch holding an +action result open is bounded much more aggressively (iOS Safari +aborts pending fetches when a tab backgrounds or the network +flips, surfacing as `TypeError: Load failed`). Anything that walks +an external catalog or fans out per-product API calls — App Store +Connect / Play Console sync, Meta Horizon reconciliation, future +Stripe price sync — must run as a background job, not as a +synchronous public action the dashboard awaits. + +Pattern (mirrors `convex/products/jobs.ts` + `runProductSyncIOS` / +`runProductSyncAndroid`): + +1. **Schema**: a `*Jobs` table with `status` + (`queued | running | succeeded | failed`), `progress` (`{phase, + current?, total?, failuresCount?}`), `result?`, `error?`, + `cancelRequested?`, `expectedDeadline?`, `createdBy?`, + `startedAt?`, `completedAt?`, `createdAt`. Indexes: + `(projectId, platform, status)` for active-job lookup, + `(status, expectedDeadline)` for the reaper, + `(status, completedAt)` for the pruner. +2. **Enqueue mutation**: validates membership, dedups against an + existing `queued`/`running` row for the same `(projectId, + platform)`, inserts the row, schedules the worker via + `ctx.scheduler.runAfter(0, internal..runX, { jobId })`. + Returns `{ jobId, deduped }`. +3. **Worker internalAction**: `args: { jobId }`. Reads job → + resolves project → runs the work, calling + `updateJobProgress` at phase boundaries and + `isCancelRequested` between phases. Wraps the body in + try/catch and finishes via `markJobSucceeded` / + `markJobFailed` so a thrown error never leaves the row in + `running` forever. +4. **Cron pair**: `reapStaleProductSyncJobs` (5 min, flips + `running` rows past `expectedDeadline + grace` to `failed`) + and `pruneProductSyncJobs` (6 h, deletes `succeeded` rows + older than 7 d / `failed` rows older than 30 d). +5. **Dashboard**: `useQuery(getActiveSyncJob)` for the reactive + button state + progress label; `useMutation(enqueue*)` to + start; `useMutation(cancel*)` to stop. The completion toast + fires once via a `useRef`-gated `useEffect` so reactive + updates don't re-toast. +6. **HTTP**: `POST .../sync/...` returns 202 with `{ jobId, + deduped }`; `GET .../sync/jobs/{jobId}` polls; `POST + .../sync/jobs/{jobId}/cancel` cancels. Clients backoff at ~3 s + intervals. + +Failures arrays should pass through `truncateFailures` (cap 200, +sets `failuresTruncated: true`) so a runaway sync where every +product fails for the same reason can't blow past Convex's +per-document size budget. + ## Commit messages Follow the monorepo-wide convention from the root diff --git a/packages/kit/convex/_generated/api.d.ts b/packages/kit/convex/_generated/api.d.ts index 30f95d3d..d879f8c1 100644 --- a/packages/kit/convex/_generated/api.d.ts +++ b/packages/kit/convex/_generated/api.d.ts @@ -29,6 +29,7 @@ import type * as organizations_mutation from "../organizations/mutation.js"; import type * as organizations_query from "../organizations/query.js"; import type * as plans from "../plans.js"; import type * as products_asc from "../products/asc.js"; +import type * as products_jobs from "../products/jobs.js"; import type * as products_jwt from "../products/jwt.js"; import type * as products_mutation from "../products/mutation.js"; import type * as products_play from "../products/play.js"; @@ -106,6 +107,7 @@ declare const fullApi: ApiFromModules<{ "organizations/query": typeof organizations_query; plans: typeof plans; "products/asc": typeof products_asc; + "products/jobs": typeof products_jobs; "products/jwt": typeof products_jwt; "products/mutation": typeof products_mutation; "products/play": typeof products_play; diff --git a/packages/kit/convex/crons.ts b/packages/kit/convex/crons.ts index 679a61b6..a926cf39 100644 --- a/packages/kit/convex/crons.ts +++ b/packages/kit/convex/crons.ts @@ -81,4 +81,23 @@ crons.interval( { batchSize: 50 }, ); +// Mark stuck product-sync jobs as failed. Convex caps actions at +// ~10min; the worker sets `expectedDeadline = startedAt + 9min`, +// and this reaper flips anything still `running` past +// `deadline + 1min` to failed("worker timed out"). Without it, a +// crashed action permanently pins the project's "active job" slot +// and the dashboard's button stays disabled forever. +crons.interval( + "reap stale product sync jobs", + { minutes: 5 }, + internal.products.jobs.reapStaleProductSyncJobs, +); + +// Drop succeeded jobs after 7d, failed after 30d. +crons.interval( + "prune product sync jobs past retention", + { hours: 6 }, + internal.products.jobs.pruneProductSyncJobs, +); + export default crons; diff --git a/packages/kit/convex/files/internal.ts b/packages/kit/convex/files/internal.ts index f3ca9d82..40ba9dac 100644 --- a/packages/kit/convex/files/internal.ts +++ b/packages/kit/convex/files/internal.ts @@ -179,10 +179,25 @@ export const readFileAsBase64 = internalAction({ // accessCount: (file.accessCount || 0) + 1, // }); - // Convert blob to base64 via Buffer (Convex actions run on Node/Bun - // — Buffer is available globally and avoids the per-byte loop). + // Convex `internalAction`s without `"use node"` run in the V8 + // isolate runtime where `Buffer` is NOT a global — using it + // throws `ReferenceError: Buffer is not defined` at request time + // (the prior `Buffer.from(...)` shipped here was the bug behind + // the dashboard's "download .p8" failing). Encode via `btoa` on + // chunked binary strings so the path stays portable to either + // runtime; chunking keeps the call-stack bound below the + // String.fromCharCode argument limit for files of any size. const arrayBuffer = await blob.arrayBuffer(); - const base64 = Buffer.from(arrayBuffer).toString("base64"); + 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[], + ); + } + const base64 = btoa(binary); return { fileId: file._id, diff --git a/packages/kit/convex/products/asc.ts b/packages/kit/convex/products/asc.ts index c25eda37..06cdc532 100644 --- a/packages/kit/convex/products/asc.ts +++ b/packages/kit/convex/products/asc.ts @@ -1,13 +1,24 @@ "use node"; import { v } from "convex/values"; -import { action, type ActionCtx } from "../_generated/server"; +import { action, internalAction, type ActionCtx } from "../_generated/server"; import { internal } from "../_generated/api"; import { getProjectByApiKey } from "../purchases/shared"; import { mapWithConcurrency } from "../utils/concurrency"; import { mintAscJwt } from "./jwt"; import { coerceBillingPeriod } from "./sync"; +// Cancel-check at phase boundaries. The worker reads +// `cancelRequested` between PULL.iaps → PULL.subgroups → PUSH.drafts. +// Granularity is per-phase, not per-product, but that's enough to +// stop a runaway sync within seconds on most paths. +class ProductSyncCancelledError extends Error { + constructor() { + super("Sync cancelled by operator"); + this.name = "ProductSyncCancelledError"; + } +} + // Resolve App Store Connect API credentials (issuer ID + key ID + .p8 // key content) for a project. Centralized so the two action handlers // (pushSyncProductsAppleIOS and listSubscriptionGroupsAppleIOS) share @@ -914,125 +925,268 @@ function extractAscError(parsed: unknown): string { // = "Draft" / "Ready" upstream. // --------------------------------------------------------------------------- -// NOTE on action duration: this handler runs sequentially across the -// project's catalog (with mapWithConcurrency=6 fan-out per pull -// step). Convex actions have a 10-minute hard ceiling. For typical -// commercial apps with <100 SKUs the round trips finish in ~10-30s -// even with throttled ASC. Catalogs north of ~500 SKUs may need -// batching — splitting drafts into chunks and chaining -// internal.scheduler runs from the kit dashboard. Tracked as a -// follow-up; not addressed here because v0 SaaS targets the -// long-tail-of-commercial-apps profile, not multi-thousand-SKU -// enterprise catalogs. -export const pushSyncProductsAppleIOS = action({ - args: { - apiKey: v.string(), - direction: v.optional( - v.union(v.literal("pull"), v.literal("push"), v.literal("both")), - ), - // Dry-run mode: read-only against ASC. Skips every POST/PATCH - // (create subscription / IAP, localization, price schedule, group - // create) and instead returns the sequence of write attempts - // kit would have made. Lets the operator preview a Sync without - // polluting their App Store Connect catalog with test rows that - // Apple won't let them delete cleanly. - dryRun: v.optional(v.boolean()), - }, - returns: v.object({ - pulled: v.number(), - pushed: v.number(), - failures: v.array(v.object({ productId: v.string(), reason: v.string() })), - plannedWrites: v.optional( - v.array( - v.object({ - productId: v.string(), - step: v.string(), - detail: v.optional(v.string()), - }), - ), - ), - }), - handler: async ( - ctx, - args, - ): Promise<{ - pulled: number; - pushed: number; - failures: Array<{ productId: string; reason: string }>; - plannedWrites?: Array<{ - productId: string; - step: string; - detail?: string; - }>; - }> => { - const project = await getProjectByApiKey(ctx, args.apiKey); - if (!project.iosBundleId) { - throw new Error("Project iosBundleId is not configured"); +// Worker that drives a single ASC sync job. Scheduled by +// `enqueueProductSync` (in `products/jobs.ts`); never called +// directly by the dashboard / HTTP / SDK paths so the long fetch +// can never hold a browser connection open. +// +// Convex actions cap at ~10 minutes; we set the job's expected +// deadline at 9 minutes and rely on `reapStaleProductSyncJobs` to +// flip anything still running 1 minute past that to failed. Within +// the action body we also poll `isCancelRequested` at phase +// boundaries (PULL.iaps → PULL.subgroups → PUSH.drafts) so an +// operator-initiated cancel takes effect within one phase. +export const runProductSyncIOS = internalAction({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args): Promise => { + const job = await ctx.runQuery(internal.products.jobs.getJobForWorker, { + jobId: args.jobId, + }); + if (!job) return; + if (job.status !== "queued") return; + await ctx.runMutation(internal.products.jobs.markJobRunning, { + jobId: args.jobId, + }); + const checkCancelled = async () => { + const cancelled = await ctx.runQuery( + internal.products.jobs.isCancelRequested, + { jobId: args.jobId }, + ); + if (cancelled) throw new ProductSyncCancelledError(); + }; + const reportPhase = async ( + phase: string, + extra?: { + current?: number; + total?: number; + failuresCount?: number; + }, + ) => { + await ctx.runMutation(internal.products.jobs.updateJobProgress, { + jobId: args.jobId, + phase, + current: extra?.current, + total: extra?.total, + failuresCount: extra?.failuresCount, + }); + }; + if (job.direction === "purge-local") { + // enqueue routes purge-local jobs to a different worker; this + // branch is unreachable in practice but narrows the type for + // the call below. + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: "purge-local routed to wrong worker", + }); + return; } - if (!project.iosAppAppleId) { - throw new Error("Project iosAppAppleId is required for ASC push-sync"); + try { + const result = await performIosSync(ctx, { + projectId: job.projectId, + direction: job.direction, + dryRun: job.dryRun, + checkCancelled, + reportPhase, + }); + await ctx.runMutation(internal.products.jobs.markJobSucceeded, { + jobId: args.jobId, + pulled: result.pulled, + pushed: result.pushed, + failures: result.failures, + plannedWrites: result.plannedWrites, + }); + } catch (error) { + const cancelled = error instanceof ProductSyncCancelledError; + const message = cancelled + ? "Cancelled by operator" + : error instanceof Error + ? error.message + : String(error); + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: message, + }); } - // ASC push-sync uses the App Store Connect API key (Team Key / - // Individual Key), which is genuinely different from the App Store - // Server API key used for receipt verification — Apple scopes them - // separately at the gateway. We prefer the dedicated ASC slot when - // the operator has populated it, but fall back to the existing - // Server API slot so projects that upload a Team Key into the old - // (single-slot) workflow keep working without a re-config dance. - // The 401 from Apple's gateway is what catches a wrong-kind key - // either way — the helpful message in `call()` points the operator - // at the right Apple page. The full pair-resolve + .p8-fallback - // logic lives in `resolveAscCredentials` so the matching - // listSubscriptionGroupsAppleIOS handler stays in lockstep. - const { issuerId, keyId, keyContent } = await resolveAscCredentials( - ctx, - project, - { detailedErrors: true }, - ); - const client = new AscClient(issuerId, keyId, keyContent); + }, +}); + +async function performIosSync( + ctx: ActionCtx, + options: { + projectId: import("../_generated/dataModel").Id<"projects">; + direction: "pull" | "push" | "both"; + dryRun: boolean; + checkCancelled: () => Promise; + reportPhase: ( + phase: string, + extra?: { + current?: number; + total?: number; + failuresCount?: number; + }, + ) => Promise; + }, +): Promise<{ + pulled: number; + pushed: number; + failures: Array<{ productId: string; reason: string }>; + plannedWrites?: Array<{ productId: string; step: string; detail?: string }>; +}> { + const project = await ctx.runQuery( + internal.projects.internal.getProjectById, + { projectId: options.projectId }, + ); + if (!project) { + throw new Error("Project not found for sync job"); + } + if (!project.iosBundleId) { + throw new Error("Project iosBundleId is not configured"); + } + if (!project.iosAppAppleId) { + throw new Error("Project iosAppAppleId is required for ASC push-sync"); + } + const args = { + direction: options.direction, + dryRun: options.dryRun, + }; + const { checkCancelled, reportPhase } = options; + // ASC push-sync uses the App Store Connect API key (Team Key / + // Individual Key), which is genuinely different from the App Store + // Server API key used for receipt verification — Apple scopes them + // separately at the gateway. We prefer the dedicated ASC slot when + // the operator has populated it, but fall back to the existing + // Server API slot so projects that upload a Team Key into the old + // (single-slot) workflow keep working without a re-config dance. + // The 401 from Apple's gateway is what catches a wrong-kind key + // either way — the helpful message in `call()` points the operator + // at the right Apple page. The full pair-resolve + .p8-fallback + // logic lives in `resolveAscCredentials` so the matching + // listSubscriptionGroupsAppleIOS handler stays in lockstep. + const { issuerId, keyId, keyContent } = await resolveAscCredentials( + ctx, + project, + { detailedErrors: true }, + ); + const client = new AscClient(issuerId, keyId, keyContent); + + const direction = args.direction ?? "both"; + const failures: Array<{ productId: string; reason: string }> = []; + let pulled = 0; + let pushed = 0; + const dryRun = args.dryRun ?? false; + const plannedWrites: Array<{ + productId: string; + step: string; + detail?: string; + }> = []; - const direction = args.direction ?? "both"; - const failures: Array<{ productId: string; reason: string }> = []; - let pulled = 0; - let pushed = 0; - const dryRun = args.dryRun ?? false; - const plannedWrites: Array<{ - productId: string; - step: string; - detail?: string; - }> = []; + const appIdStr = String(project.iosAppAppleId); - const appIdStr = String(project.iosAppAppleId); + // ── PULL: ASC → kit catalog ──────────────────────────────────── + if (direction === "pull" || direction === "both") { + await checkCancelled(); + await reportPhase("pull-iaps"); + const iaps = await client.listInAppPurchases(appIdStr).catch((error) => { + failures.push({ + productId: "(asc list iaps)", + reason: error instanceof Error ? error.message : String(error), + }); + return null; + }); + if (iaps) { + // Apple throttles ASC pretty aggressively (~50 req/min); + // concurrency=6 keeps the pull fast for catalogs with dozens + // of IAPs while staying well clear of 429 territory. Switching + // from a sequential await loop dropped a 30-IAP pull from + // ~30s to ~5s in local testing. + const iapResults = await mapWithConcurrency( + iaps.data, + 6, + async (item) => { + const productId = item.attributes.productId; + if (!productId) return null; + const type = mapAscIapType(item.attributes.inAppPurchaseType); + const pricePoint = await client.iapCurrentPrice(item.id); + return { item, productId, type, pricePoint }; + }, + ); + for (const result of iapResults) { + if (!result) continue; + const { item, productId, type, pricePoint } = result; + if (pricePoint instanceof Error) { + failures.push({ + productId: `${productId} (price lookup)`, + reason: pricePoint.message, + }); + } + const { priceAmountMicros, currency } = parseAssignedPrice( + pricePoint instanceof Error ? null : pricePoint, + "inAppPurchasePricePoint", + ); + // upsertFromStore runs serially — Convex coalesces writes + // anyway and parallel mutations on the same row would race + // on the (projectId, platform, productId) lookup. + await ctx.runMutation(internal.products.sync.upsertFromStore, { + projectId: project._id, + productId, + platform: "IOS", + type, + title: item.attributes.name ?? productId, + priceAmountMicros, + currency, + storeRef: item.id, + state: mapAscState(item.attributes.state), + }); + pulled += 1; + } + } - // ── PULL: ASC → kit catalog ──────────────────────────────────── - if (direction === "pull" || direction === "both") { - const iaps = await client.listInAppPurchases(appIdStr).catch((error) => { + await checkCancelled(); + await reportPhase("pull-subscriptions", { + current: pulled, + failuresCount: failures.length, + }); + const groups = await client + .listSubscriptionGroups(appIdStr) + .catch((error) => { failures.push({ - productId: "(asc list iaps)", + productId: "(asc list groups)", reason: error instanceof Error ? error.message : String(error), }); return null; }); - if (iaps) { - // Apple throttles ASC pretty aggressively (~50 req/min); - // concurrency=6 keeps the pull fast for catalogs with dozens - // of IAPs while staying well clear of 429 territory. Switching - // from a sequential await loop dropped a 30-IAP pull from - // ~30s to ~5s in local testing. - const iapResults = await mapWithConcurrency( - iaps.data, + if (groups) { + for (const group of groups.data) { + const subs = await client + .listSubscriptionsInGroup(group.id) + .catch((error) => { + failures.push({ + productId: `(asc list subs in group ${group.id})`, + reason: error instanceof Error ? error.message : String(error), + }); + return null; + }); + if (!subs) continue; + // Same parallelization as the IAP loop above. Within each + // sub, price lookup and intro-offer lookup are independent + // — fire them as a Promise.all to halve the per-item RTT + // before walking on to the upsert. + const subResults = await mapWithConcurrency( + subs.data, 6, - async (item) => { - const productId = item.attributes.productId; + async (sub) => { + const productId = sub.attributes.productId; if (!productId) return null; - const type = mapAscIapType(item.attributes.inAppPurchaseType); - const pricePoint = await client.iapCurrentPrice(item.id); - return { item, productId, type, pricePoint }; + const [pricePoint, introOffers] = await Promise.all([ + client.subCurrentPrice(sub.id), + client.subIntroductoryOffer(sub.id), + ]); + return { sub, productId, pricePoint, introOffers }; }, ); - for (const result of iapResults) { + for (const result of subResults) { if (!result) continue; - const { item, productId, type, pricePoint } = result; + const { sub, productId, pricePoint, introOffers } = result; if (pricePoint instanceof Error) { failures.push({ productId: `${productId} (price lookup)`, @@ -1041,560 +1195,494 @@ export const pushSyncProductsAppleIOS = action({ } const { priceAmountMicros, currency } = parseAssignedPrice( pricePoint instanceof Error ? null : pricePoint, - "inAppPurchasePricePoint", + "subscriptionPricePoint", + ); + if (introOffers instanceof Error) { + failures.push({ + productId: `${productId} (offers lookup)`, + reason: introOffers.message, + }); + } + const offers = parseIntroOffers( + introOffers instanceof Error ? null : introOffers, ); - // upsertFromStore runs serially — Convex coalesces writes - // anyway and parallel mutations on the same row would race - // on the (projectId, platform, productId) lookup. await ctx.runMutation(internal.products.sync.upsertFromStore, { projectId: project._id, productId, platform: "IOS", - type, - title: item.attributes.name ?? productId, + type: "Subscription", + title: sub.attributes.name ?? productId, priceAmountMicros, currency, - storeRef: item.id, - state: mapAscState(item.attributes.state), + storeRef: sub.id, + state: mapAscState(sub.attributes.state), + billingPeriod: coerceBillingPeriod( + mapAscOfferDurationToIso( + sub.attributes.subscriptionPeriod ?? undefined, + ), + ), + subscriptionGroupId: group.id, + subscriptionGroupName: group.attributes.referenceName, + offers: offers.length ? offers : undefined, }); pulled += 1; } } - - const groups = await client - .listSubscriptionGroups(appIdStr) - .catch((error) => { - failures.push({ - productId: "(asc list groups)", - reason: error instanceof Error ? error.message : String(error), - }); - return null; - }); - if (groups) { - for (const group of groups.data) { - const subs = await client - .listSubscriptionsInGroup(group.id) - .catch((error) => { - failures.push({ - productId: `(asc list subs in group ${group.id})`, - reason: error instanceof Error ? error.message : String(error), - }); - return null; - }); - if (!subs) continue; - // Same parallelization as the IAP loop above. Within each - // sub, price lookup and intro-offer lookup are independent - // — fire them as a Promise.all to halve the per-item RTT - // before walking on to the upsert. - const subResults = await mapWithConcurrency( - subs.data, - 6, - async (sub) => { - const productId = sub.attributes.productId; - if (!productId) return null; - const [pricePoint, introOffers] = await Promise.all([ - client.subCurrentPrice(sub.id), - client.subIntroductoryOffer(sub.id), - ]); - return { sub, productId, pricePoint, introOffers }; - }, - ); - for (const result of subResults) { - if (!result) continue; - const { sub, productId, pricePoint, introOffers } = result; - if (pricePoint instanceof Error) { - failures.push({ - productId: `${productId} (price lookup)`, - reason: pricePoint.message, - }); - } - const { priceAmountMicros, currency } = parseAssignedPrice( - pricePoint instanceof Error ? null : pricePoint, - "subscriptionPricePoint", - ); - if (introOffers instanceof Error) { - failures.push({ - productId: `${productId} (offers lookup)`, - reason: introOffers.message, - }); - } - const offers = parseIntroOffers( - introOffers instanceof Error ? null : introOffers, - ); - await ctx.runMutation(internal.products.sync.upsertFromStore, { - projectId: project._id, - productId, - platform: "IOS", - type: "Subscription", - title: sub.attributes.name ?? productId, - priceAmountMicros, - currency, - storeRef: sub.id, - state: mapAscState(sub.attributes.state), - billingPeriod: coerceBillingPeriod( - mapAscOfferDurationToIso( - sub.attributes.subscriptionPeriod ?? undefined, - ), - ), - subscriptionGroupId: group.id, - subscriptionGroupName: group.attributes.referenceName, - offers: offers.length ? offers : undefined, - }); - pulled += 1; - } - } - } } + } - // ── PUSH: kit → ASC for Draft rows ───────────────────────────── - // Each draft becomes a multi-step flow: create → localize → set - // price. The first step alone leaves the IAP/sub in an unsubmittable - // state because Apple requires both an en-US localization and a - // USA price schedule before the row can move past Draft. We do - // the whole chain here so a single Sync click takes the catalog - // from "kit-only" to "Ready to Submit" in App Store Connect. - // Submission itself (screenshot upload + inAppPurchaseSubmissions - // POST) is a follow-up because it needs a screenshot file and a - // dashboard upload slot we haven't built yet — see TODO below. - if (direction === "push" || direction === "both") { - const drafts = await ctx.runQuery( - internal.products.sync.listDraftIosProducts, - { projectId: project._id }, - ); - // Cache subscriptionGroup find-or-create results across the - // entire push pass so a project with multiple drafts in the - // same group (Premium Monthly + Premium Yearly + Premium - // Weekly all referencing groupName="Premium") only triggers - // one ASC listSubscriptionGroups round-trip — and never two - // concurrent create calls racing for the same name. + // ── PUSH: kit → ASC for Draft rows ───────────────────────────── + // Each draft becomes a multi-step flow: create → localize → set + // price. The first step alone leaves the IAP/sub in an unsubmittable + // state because Apple requires both an en-US localization and a + // USA price schedule before the row can move past Draft. We do + // the whole chain here so a single Sync click takes the catalog + // from "kit-only" to "Ready to Submit" in App Store Connect. + // Submission itself (screenshot upload + inAppPurchaseSubmissions + // POST) is a follow-up because it needs a screenshot file and a + // dashboard upload slot we haven't built yet — see TODO below. + if (direction === "push" || direction === "both") { + await checkCancelled(); + await reportPhase("push-drafts", { + current: pulled, + failuresCount: failures.length, + }); + const drafts = await ctx.runQuery( + internal.products.sync.listDraftIosProducts, + { projectId: project._id }, + ); + // Cache subscriptionGroup find-or-create results across the + // entire push pass so a project with multiple drafts in the + // same group (Premium Monthly + Premium Yearly + Premium + // Weekly all referencing groupName="Premium") only triggers + // one ASC listSubscriptionGroups round-trip — and never two + // concurrent create calls racing for the same name. + // + // Stores the in-flight promise (not the resolved id) so two + // drafts that hit the same name concurrently share one ASC + // round-trip. Without this the parallel push fan-out below + // could race two find-or-create calls for the same group, + // ending up with one of them returning a 409. + const groupIdCache = new Map>(); + // Dry-run uses a single up-front listSubscriptionGroups fetch + // (read-only) so the per-draft preview rendering doesn't + // re-list the groups for each Subscription row in drafts. + // Lazy: only fetched on the first Subscription draft we hit + // in dry-run, so projects without Sub drafts don't pay the + // call at all. + let dryRunGroupsCache: Awaited< + ReturnType + > | null = null; + const ensureDryRunGroups = async () => { + if (!dryRunGroupsCache) { + dryRunGroupsCache = await client.listSubscriptionGroups(appIdStr); + } + return dryRunGroupsCache; + }; + // Bounded-parallel push. ASC throttles aggressively on the + // mutation endpoints (createSubscription / createInAppPurchase / + // setPriceSchedule) so the previous sequential `for (const row + // of drafts)` loop was the safe-but-slow path; a project with + // 20 draft products waited 20× the per-draft round-trip. Run + // PUSH_CONCURRENCY drafts in parallel and trade some risk of a + // 429 (where ASC returns Retry-After we'd surface to the + // failures array) for an N× speedup. + // + // Each draft's create → localize → setPrice steps stay strictly + // sequential within `processOneDraft` — ASC rejects ordering + // races on a single resource (a localize call landing before + // the create propagates returns 409). Cross-draft parallelism + // is safe because each upstream resource is independent. The + // groupIdCache holds in-flight promises so concurrent drafts in + // the same subscription group still issue exactly one + // findOrCreate call. + // + // Concurrency=4 keeps us well under ASC's per-app rate limit + // (anecdotally ~10 writes/sec before 429s start) while + // delivering ~4× wall-clock improvement on typical catalogs. + // mapWithConcurrency preserves input order for the result + // array (we don't actually use it; failures + pushed are + // accumulated by mutation). + const PUSH_CONCURRENCY = 4; + const processOneDraft = async ( + row: (typeof drafts)[number], + ): Promise => { + // Track failures pushed *for this row* via a row-local flag. + // The previous `failuresAtStart = failures.length` snapshot + // worked when this loop was sequential, but with + // mapWithConcurrency (PUSH_CONCURRENCY=4) the shared + // `failures` array can grow because of OTHER concurrent + // drafts between the snapshot and the success-gate check — + // which would block this draft from calling markPushed even + // though every step for THIS row succeeded. // - // Stores the in-flight promise (not the resolved id) so two - // drafts that hit the same name concurrently share one ASC - // round-trip. Without this the parallel push fan-out below - // could race two find-or-create calls for the same group, - // ending up with one of them returning a 409. - const groupIdCache = new Map>(); - // Dry-run uses a single up-front listSubscriptionGroups fetch - // (read-only) so the per-draft preview rendering doesn't - // re-list the groups for each Subscription row in drafts. - // Lazy: only fetched on the first Subscription draft we hit - // in dry-run, so projects without Sub drafts don't pay the - // call at all. - let dryRunGroupsCache: Awaited< - ReturnType - > | null = null; - const ensureDryRunGroups = async () => { - if (!dryRunGroupsCache) { - dryRunGroupsCache = await client.listSubscriptionGroups(appIdStr); - } - return dryRunGroupsCache; + // Use a row-local boolean + a recordFailure helper so each + // draft's success gate is independent of cross-draft noise. + // A partial setup (create succeeded, localization failed) + // still leaves the row in Draft with a populated storeRef + // so the next sync resumes step 2 instead of re-creating + // the upstream resource. + let rowHadFailure = false; + const recordFailure = (failure: { + productId: string; + reason: string; + }) => { + rowHadFailure = true; + failures.push(failure); }; - // Bounded-parallel push. ASC throttles aggressively on the - // mutation endpoints (createSubscription / createInAppPurchase / - // setPriceSchedule) so the previous sequential `for (const row - // of drafts)` loop was the safe-but-slow path; a project with - // 20 draft products waited 20× the per-draft round-trip. Run - // PUSH_CONCURRENCY drafts in parallel and trade some risk of a - // 429 (where ASC returns Retry-After we'd surface to the - // failures array) for an N× speedup. - // - // Each draft's create → localize → setPrice steps stay strictly - // sequential within `processOneDraft` — ASC rejects ordering - // races on a single resource (a localize call landing before - // the create propagates returns 409). Cross-draft parallelism - // is safe because each upstream resource is independent. The - // groupIdCache holds in-flight promises so concurrent drafts in - // the same subscription group still issue exactly one - // findOrCreate call. - // - // Concurrency=4 keeps us well under ASC's per-app rate limit - // (anecdotally ~10 writes/sec before 429s start) while - // delivering ~4× wall-clock improvement on typical catalogs. - // mapWithConcurrency preserves input order for the result - // array (we don't actually use it; failures + pushed are - // accumulated by mutation). - const PUSH_CONCURRENCY = 4; - const processOneDraft = async ( - row: (typeof drafts)[number], - ): Promise => { - // Track failures pushed *for this row* via a row-local flag. - // The previous `failuresAtStart = failures.length` snapshot - // worked when this loop was sequential, but with - // mapWithConcurrency (PUSH_CONCURRENCY=4) the shared - // `failures` array can grow because of OTHER concurrent - // drafts between the snapshot and the success-gate check — - // which would block this draft from calling markPushed even - // though every step for THIS row succeeded. - // - // Use a row-local boolean + a recordFailure helper so each - // draft's success gate is independent of cross-draft noise. - // A partial setup (create succeeded, localization failed) - // still leaves the row in Draft with a populated storeRef - // so the next sync resumes step 2 instead of re-creating - // the upstream resource. - let rowHadFailure = false; - const recordFailure = (failure: { - productId: string; - reason: string; - }) => { - rowHadFailure = true; - failures.push(failure); - }; - try { - if (row.type === "Subscription") { - // Resolve the ASC subscriptionGroup from the operator-typed - // `subscriptionGroupName`. Find-or-create so the operator - // doesn't have to pre-create the group in ASC's web UI; if - // they don't pick a name we default to the productId so - // there's *some* group rather than a hard failure — but - // surface a non-fatal warning since per-product groups - // fragment the catalog and break StoreKit 2's - // upgrade/downgrade flow between Monthly and Yearly tiers - // (those need to share a group). In dry-run, list groups - // (read-only) and report which path the real run would - // take instead of creating anything. - // - // Skip both group-resolve and create when this row already - // has a storeRef from a prior partially-successful sync — - // re-creating would either duplicate or 409 against ASC. - const groupName = row.subscriptionGroupName ?? row.productId; - if (!row.subscriptionGroupName && !row.storeRef && dryRun) { - // Surface the per-product-group warning in dry-run only - // so operators see the recommendation while previewing - // (the most common time to fix the catalog), but a - // production sync isn't blocked or noisy. Pushing into - // `failures` would also trip the markPushed gate added - // for partial-failure resilience. + try { + if (row.type === "Subscription") { + // Resolve the ASC subscriptionGroup from the operator-typed + // `subscriptionGroupName`. Find-or-create so the operator + // doesn't have to pre-create the group in ASC's web UI; if + // they don't pick a name we default to the productId so + // there's *some* group rather than a hard failure — but + // surface a non-fatal warning since per-product groups + // fragment the catalog and break StoreKit 2's + // upgrade/downgrade flow between Monthly and Yearly tiers + // (those need to share a group). In dry-run, list groups + // (read-only) and report which path the real run would + // take instead of creating anything. + // + // Skip both group-resolve and create when this row already + // has a storeRef from a prior partially-successful sync — + // re-creating would either duplicate or 409 against ASC. + const groupName = row.subscriptionGroupName ?? row.productId; + if (!row.subscriptionGroupName && !row.storeRef && dryRun) { + // Surface the per-product-group warning in dry-run only + // so operators see the recommendation while previewing + // (the most common time to fix the catalog), but a + // production sync isn't blocked or noisy. Pushing into + // `failures` would also trip the markPushed gate added + // for partial-failure resilience. + plannedWrites.push({ + productId: row.productId, + step: "warning: no subscription group name set", + detail: + "Falling back to productId so this sub lands in its own group. Pick a shared name (e.g. 'Premium') for related tiers so StoreKit 2 upgrade/downgrade works.", + }); + } + let storeRef: string; + if (row.storeRef) { + storeRef = row.storeRef; + if (dryRun) { plannedWrites.push({ productId: row.productId, - step: "warning: no subscription group name set", - detail: - "Falling back to productId so this sub lands in its own group. Pick a shared name (e.g. 'Premium') for related tiers so StoreKit 2 upgrade/downgrade works.", + step: "skip create (resuming partial sync)", + detail: `existing storeRef=${storeRef}`, }); } - let storeRef: string; - if (row.storeRef) { - storeRef = row.storeRef; - if (dryRun) { - plannedWrites.push({ - productId: row.productId, - step: "skip create (resuming partial sync)", - detail: `existing storeRef=${storeRef}`, - }); - } + } else { + let groupId: string; + if (dryRun) { + const groups = await ensureDryRunGroups(); + const existing = groups.data.find( + (g) => g.attributes.referenceName === groupName, + ); + groupId = existing?.id ?? "(would-create)"; + plannedWrites.push({ + productId: row.productId, + step: existing + ? "use existing subscription group" + : "create subscription group", + detail: groupName, + }); + storeRef = "(would-create)"; + plannedWrites.push({ + productId: row.productId, + step: "create subscription", + detail: `${row.title} · ${mapBillingPeriodToAsc(row.billingPeriod)} · group=${groupName}`, + }); } else { - let groupId: string; - if (dryRun) { - const groups = await ensureDryRunGroups(); - const existing = groups.data.find( - (g) => g.attributes.referenceName === groupName, - ); - groupId = existing?.id ?? "(would-create)"; - plannedWrites.push({ - productId: row.productId, - step: existing - ? "use existing subscription group" - : "create subscription group", - detail: groupName, + let cached = groupIdCache.get(groupName); + if (!cached) { + cached = client.findOrCreateSubscriptionGroup({ + appId: appIdStr, + referenceName: groupName, }); - storeRef = "(would-create)"; - plannedWrites.push({ - productId: row.productId, - step: "create subscription", - detail: `${row.title} · ${mapBillingPeriodToAsc(row.billingPeriod)} · group=${groupName}`, - }); - } else { - let cached = groupIdCache.get(groupName); - if (!cached) { - cached = client.findOrCreateSubscriptionGroup({ - appId: appIdStr, - referenceName: groupName, - }); - groupIdCache.set(groupName, cached); - // If the in-flight call rejects, evict the cached - // promise so a follow-up draft can retry instead of - // permanently inheriting the failure. - cached.catch(() => { - if (groupIdCache.get(groupName) === cached) { - groupIdCache.delete(groupName); - } - }); - } - groupId = await cached; - const result = await client.createSubscription({ - groupId, - productId: row.productId, - name: row.title, - subscriptionPeriod: mapBillingPeriodToAsc(row.billingPeriod), - reviewNote: row.reviewNote, + groupIdCache.set(groupName, cached); + // If the in-flight call rejects, evict the cached + // promise so a follow-up draft can retry instead of + // permanently inheriting the failure. + cached.catch(() => { + if (groupIdCache.get(groupName) === cached) { + groupIdCache.delete(groupName); + } }); - storeRef = result.data.id; - // Persist the upstream id immediately so a subsequent - // step's failure doesn't lose the binding (and the - // next sync sees this row's storeRef populated and - // skips the create call above). - await ctx.runMutation(internal.products.sync.markStoreRef, { - projectId: project._id, - productId: row.productId, - platform: "IOS", - storeRef, + } + groupId = await cached; + const result = await client.createSubscription({ + groupId, + productId: row.productId, + name: row.title, + subscriptionPeriod: mapBillingPeriodToAsc(row.billingPeriod), + reviewNote: row.reviewNote, + }); + storeRef = result.data.id; + // Persist the upstream id immediately so a subsequent + // step's failure doesn't lose the binding (and the + // next sync sees this row's storeRef populated and + // skips the create call above). + await ctx.runMutation(internal.products.sync.markStoreRef, { + projectId: project._id, + productId: row.productId, + platform: "IOS", + storeRef, + }); + } + } + // Localize so reviewers see the human-readable name + + // description instead of just the productId. ASC requires + // at least one locale before submission — failing here + // doesn't unwind the create (Apple has no rollback) so we + // record a failure and let the operator retry / fix in + // ASC web. + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "create en-US localization", + detail: row.description ?? row.title, + }); + } else { + try { + await client.createSubLocalization({ + subId: storeRef, + name: row.title, + description: row.description ?? row.title, + }); + } catch (error) { + // 409 Conflict means the en-US localization already + // exists from a prior partial sync. That's a benign + // retry — fall through to the price-setting step + // instead of marking the whole product failed. + if (!(error instanceof AscApiError && error.status === 409)) { + recordFailure({ + productId: `${row.productId} (localization)`, + reason: + error instanceof Error ? error.message : String(error), }); } } - // Localize so reviewers see the human-readable name + - // description instead of just the productId. ASC requires - // at least one locale before submission — failing here - // doesn't unwind the create (Apple has no rollback) so we - // record a failure and let the operator retry / fix in - // ASC web. + } + // Set the USA price by resolving the operator's USD amount + // → Apple's nearest price-point id. We require currency = + // "USD" because the dashboard form lets them pick others + // but we only know the USA tier ladder here; non-USD prices + // are surfaced as an actionable failure rather than silently + // mis-priced. In dry-run, skip the lookup (the just-created + // subscription resource doesn't exist for read-back) and + // just record intent. + if ( + row.priceAmountMicros !== undefined && + (row.currency ?? "USD") === "USD" + ) { if (dryRun) { plannedWrites.push({ productId: row.productId, - step: "create en-US localization", - detail: row.description ?? row.title, + step: "set USA price", + detail: `USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)}`, }); } else { try { - await client.createSubLocalization({ - subId: storeRef, - name: row.title, - description: row.description ?? row.title, - }); + const pricePointId = await client.findSubUsaPricePointId( + storeRef, + row.priceAmountMicros, + ); + if (!pricePointId) { + recordFailure({ + productId: `${row.productId} (price)`, + reason: `No ASC price tier matches USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)} — pick a published tier amount.`, + }); + } else { + await client.setSubPriceSchedule({ + subId: storeRef, + pricePointId, + }); + } } catch (error) { - // 409 Conflict means the en-US localization already - // exists from a prior partial sync. That's a benign - // retry — fall through to the price-setting step - // instead of marking the whole product failed. + // 409 Conflict means a price schedule already exists + // for the (subscription, startDate=today) pair from a + // prior partial sync — Apple keys schedules by date, + // not by id. Treat as benign retry so the subsequent + // markPushed step still runs (PR #124 + // (https://github.com/hyodotdev/openiap/pull/124) + // review). if (!(error instanceof AscApiError && error.status === 409)) { recordFailure({ - productId: `${row.productId} (localization)`, + productId: `${row.productId} (price)`, reason: error instanceof Error ? error.message : String(error), }); } } } - // Set the USA price by resolving the operator's USD amount - // → Apple's nearest price-point id. We require currency = - // "USD" because the dashboard form lets them pick others - // but we only know the USA tier ladder here; non-USD prices - // are surfaced as an actionable failure rather than silently - // mis-priced. In dry-run, skip the lookup (the just-created - // subscription resource doesn't exist for read-back) and - // just record intent. - if ( - row.priceAmountMicros !== undefined && - (row.currency ?? "USD") === "USD" - ) { - if (dryRun) { - plannedWrites.push({ - productId: row.productId, - step: "set USA price", - detail: `USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)}`, - }); - } else { - try { - const pricePointId = await client.findSubUsaPricePointId( - storeRef, - row.priceAmountMicros, - ); - if (!pricePointId) { - recordFailure({ - productId: `${row.productId} (price)`, - reason: `No ASC price tier matches USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)} — pick a published tier amount.`, - }); - } else { - await client.setSubPriceSchedule({ - subId: storeRef, - pricePointId, - }); - } - } catch (error) { - // 409 Conflict means a price schedule already exists - // for the (subscription, startDate=today) pair from a - // prior partial sync — Apple keys schedules by date, - // not by id. Treat as benign retry so the subsequent - // markPushed step still runs (PR #124 - // (https://github.com/hyodotdev/openiap/pull/124) - // review). - if (!(error instanceof AscApiError && error.status === 409)) { - recordFailure({ - productId: `${row.productId} (price)`, - reason: - error instanceof Error ? error.message : String(error), - }); - } - } - } - } else if (row.currency && row.currency !== "USD") { - recordFailure({ - productId: `${row.productId} (price)`, - reason: `Non-USD pricing (${row.currency}) not supported in push yet — set USD on the catalog row or configure other territories in ASC web.`, - }); - } - // Only flip state to Ready when every follow-up step - // succeeded. Partial setups stay in Draft (with storeRef - // populated) so the next sync resumes the missing pieces. - if (!dryRun && !rowHadFailure) { - await ctx.runMutation(internal.products.sync.markPushed, { - projectId: project._id, + } else if (row.currency && row.currency !== "USD") { + recordFailure({ + productId: `${row.productId} (price)`, + reason: `Non-USD pricing (${row.currency}) not supported in push yet — set USD on the catalog row or configure other territories in ASC web.`, + }); + } + // Only flip state to Ready when every follow-up step + // succeeded. Partial setups stay in Draft (with storeRef + // populated) so the next sync resumes the missing pieces. + if (!dryRun && !rowHadFailure) { + await ctx.runMutation(internal.products.sync.markPushed, { + projectId: project._id, + productId: row.productId, + platform: "IOS", + storeRef, + }); + } + pushed += 1; + } else { + let storeRef: string; + if (row.storeRef) { + storeRef = row.storeRef; + if (dryRun) { + plannedWrites.push({ productId: row.productId, - platform: "IOS", - storeRef, + step: "skip create (resuming partial sync)", + detail: `existing storeRef=${storeRef}`, }); } - pushed += 1; + } else if (dryRun) { + storeRef = "(would-create)"; + plannedWrites.push({ + productId: row.productId, + step: "create in-app purchase", + detail: `${row.title} · ${row.type}`, + }); } else { - let storeRef: string; - if (row.storeRef) { - storeRef = row.storeRef; - if (dryRun) { - plannedWrites.push({ - productId: row.productId, - step: "skip create (resuming partial sync)", - detail: `existing storeRef=${storeRef}`, - }); - } - } else if (dryRun) { - storeRef = "(would-create)"; - plannedWrites.push({ - productId: row.productId, - step: "create in-app purchase", - detail: `${row.title} · ${row.type}`, - }); - } else { - const result = await client.createInAppPurchase({ - appId: appIdStr, - productId: row.productId, + const result = await client.createInAppPurchase({ + appId: appIdStr, + productId: row.productId, + name: row.title, + type: row.type === "Consumable" ? "CONSUMABLE" : "NON_CONSUMABLE", + reviewNote: row.reviewNote, + }); + storeRef = result.data.id; + // Same partial-sync resilience as the Subscription + // branch — persist the upstream id before the + // localization / price steps that may fail. + await ctx.runMutation(internal.products.sync.markStoreRef, { + projectId: project._id, + productId: row.productId, + platform: "IOS", + storeRef, + }); + } + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "create en-US localization", + detail: row.description ?? row.title, + }); + } else { + try { + await client.createIapLocalization({ + iapId: storeRef, name: row.title, - type: - row.type === "Consumable" ? "CONSUMABLE" : "NON_CONSUMABLE", - reviewNote: row.reviewNote, - }); - storeRef = result.data.id; - // Same partial-sync resilience as the Subscription - // branch — persist the upstream id before the - // localization / price steps that may fail. - await ctx.runMutation(internal.products.sync.markStoreRef, { - projectId: project._id, - productId: row.productId, - platform: "IOS", - storeRef, + description: row.description ?? row.title, }); + } catch (error) { + // Same 409-is-benign rationale as the subscription + // localization path — see PR #124 + // (https://github.com/hyodotdev/openiap/pull/124) review. + if (!(error instanceof AscApiError && error.status === 409)) { + recordFailure({ + productId: `${row.productId} (localization)`, + reason: + error instanceof Error ? error.message : String(error), + }); + } } + } + if ( + row.priceAmountMicros !== undefined && + (row.currency ?? "USD") === "USD" + ) { if (dryRun) { plannedWrites.push({ productId: row.productId, - step: "create en-US localization", - detail: row.description ?? row.title, + step: "set USA price", + detail: `USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)}`, }); } else { try { - await client.createIapLocalization({ - iapId: storeRef, - name: row.title, - description: row.description ?? row.title, - }); + const pricePointId = await client.findIapUsaPricePointId( + storeRef, + row.priceAmountMicros, + ); + if (!pricePointId) { + recordFailure({ + productId: `${row.productId} (price)`, + reason: `No ASC price tier matches USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)} — pick a published tier amount.`, + }); + } else { + await client.setIapPriceSchedule({ + iapId: storeRef, + pricePointId, + }); + } } catch (error) { // Same 409-is-benign rationale as the subscription - // localization path — see PR #124 - // (https://github.com/hyodotdev/openiap/pull/124) review. + // price schedule path above — Apple keys IAP price + // schedules by (iapId, startDate) so a same-day + // retry hits Conflict. Allow the row to proceed to + // markPushed instead of stalling in Draft. if (!(error instanceof AscApiError && error.status === 409)) { recordFailure({ - productId: `${row.productId} (localization)`, + productId: `${row.productId} (price)`, reason: error instanceof Error ? error.message : String(error), }); } } } - if ( - row.priceAmountMicros !== undefined && - (row.currency ?? "USD") === "USD" - ) { - if (dryRun) { - plannedWrites.push({ - productId: row.productId, - step: "set USA price", - detail: `USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)}`, - }); - } else { - try { - const pricePointId = await client.findIapUsaPricePointId( - storeRef, - row.priceAmountMicros, - ); - if (!pricePointId) { - recordFailure({ - productId: `${row.productId} (price)`, - reason: `No ASC price tier matches USD ${(row.priceAmountMicros / 1_000_000).toFixed(2)} — pick a published tier amount.`, - }); - } else { - await client.setIapPriceSchedule({ - iapId: storeRef, - pricePointId, - }); - } - } catch (error) { - // Same 409-is-benign rationale as the subscription - // price schedule path above — Apple keys IAP price - // schedules by (iapId, startDate) so a same-day - // retry hits Conflict. Allow the row to proceed to - // markPushed instead of stalling in Draft. - if (!(error instanceof AscApiError && error.status === 409)) { - recordFailure({ - productId: `${row.productId} (price)`, - reason: - error instanceof Error ? error.message : String(error), - }); - } - } - } - } else if (row.currency && row.currency !== "USD") { - recordFailure({ - productId: `${row.productId} (price)`, - reason: `Non-USD pricing (${row.currency}) not supported in push yet — set USD on the catalog row or configure other territories in ASC web.`, - }); - } - // Same gate as the Subscription branch — only flip Ready - // when no follow-up step recorded a failure for this row. - if (!dryRun && !rowHadFailure) { - await ctx.runMutation(internal.products.sync.markPushed, { - projectId: project._id, - productId: row.productId, - platform: "IOS", - storeRef, - }); - } - pushed += 1; + } else if (row.currency && row.currency !== "USD") { + recordFailure({ + productId: `${row.productId} (price)`, + reason: `Non-USD pricing (${row.currency}) not supported in push yet — set USD on the catalog row or configure other territories in ASC web.`, + }); } - // TODO(review-submit): once Settings has an upload slot for a - // project-level App Review screenshot - // (`apple_iap_review_screenshot` purpose), add a step here: - // 1. POST /v1/inAppPurchaseAppStoreReviewScreenshots (reserve) - // 2. PUT to the returned upload URL (binary) - // 3. PATCH ...screenshots/{id} with sourceFileChecksum - // 4. POST /v1/inAppPurchaseSubmissions - // Until then, the row stops at "Ready to Submit" in ASC and - // the operator hits Submit manually (or via next app version). - } catch (error) { - recordFailure({ - productId: row.productId, - reason: error instanceof Error ? error.message : String(error), - }); + // Same gate as the Subscription branch — only flip Ready + // when no follow-up step recorded a failure for this row. + if (!dryRun && !rowHadFailure) { + await ctx.runMutation(internal.products.sync.markPushed, { + projectId: project._id, + productId: row.productId, + platform: "IOS", + storeRef, + }); + } + pushed += 1; } - }; - await mapWithConcurrency(drafts, PUSH_CONCURRENCY, processOneDraft); - } - - return { - pulled, - pushed, - failures, - plannedWrites: dryRun ? plannedWrites : undefined, + // TODO(review-submit): once Settings has an upload slot for a + // project-level App Review screenshot + // (`apple_iap_review_screenshot` purpose), add a step here: + // 1. POST /v1/inAppPurchaseAppStoreReviewScreenshots (reserve) + // 2. PUT to the returned upload URL (binary) + // 3. PATCH ...screenshots/{id} with sourceFileChecksum + // 4. POST /v1/inAppPurchaseSubmissions + // Until then, the row stops at "Ready to Submit" in ASC and + // the operator hits Submit manually (or via next app version). + } catch (error) { + recordFailure({ + productId: row.productId, + reason: error instanceof Error ? error.message : String(error), + }); + } }; - }, -}); + await mapWithConcurrency(drafts, PUSH_CONCURRENCY, processOneDraft); + } + + return { + pulled, + pushed, + failures, + plannedWrites: dryRun ? plannedWrites : undefined, + }; +} // Lightweight read-only action so the dashboard can populate a // subscription-group autocomplete without the operator having to copy diff --git a/packages/kit/convex/products/jobs.test.ts b/packages/kit/convex/products/jobs.test.ts new file mode 100644 index 00000000..15164937 --- /dev/null +++ b/packages/kit/convex/products/jobs.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; + +import { + PRODUCT_SYNC_FAILURES_CAP, + PRODUCT_SYNC_FAILED_RETENTION_MS, + PRODUCT_SYNC_JOB_DEADLINE_MS, + PRODUCT_SYNC_REAPER_GRACE_MS, + PRODUCT_SYNC_SUCCEEDED_RETENTION_MS, + truncateFailures, +} from "./jobs"; + +describe("truncateFailures", () => { + it("returns the original array unchanged when under the cap", () => { + const failures = Array.from({ length: 10 }, (_, i) => ({ + productId: `p${i}`, + reason: "boom", + })); + const { items, truncated } = truncateFailures(failures); + expect(items).toBe(failures); + expect(truncated).toBe(false); + }); + + it("caps the array and flips the flag when over", () => { + const failures = Array.from( + { length: PRODUCT_SYNC_FAILURES_CAP + 50 }, + (_, i) => ({ productId: `p${i}`, reason: "boom" }), + ); + const { items, truncated } = truncateFailures(failures); + expect(items.length).toBe(PRODUCT_SYNC_FAILURES_CAP); + expect(items[0]?.productId).toBe("p0"); + expect(truncated).toBe(true); + }); + + it("preserves order when truncating", () => { + const failures = Array.from( + { length: PRODUCT_SYNC_FAILURES_CAP + 1 }, + (_, i) => ({ productId: `p${i}`, reason: "boom" }), + ); + const { items } = truncateFailures(failures); + for (let i = 0; i < items.length; i += 1) { + expect(items[i]?.productId).toBe(`p${i}`); + } + }); +}); + +describe("retention constants", () => { + // Sanity-check the bounds the reaper / pruner crons rely on. + // Without these the worker timeout is meaningless and the pruner + // could delete failed jobs before the operator can read them. + it("reaper grace is shorter than the worker deadline", () => { + expect(PRODUCT_SYNC_REAPER_GRACE_MS).toBeLessThan( + PRODUCT_SYNC_JOB_DEADLINE_MS, + ); + }); + + it("failed retention outlives succeeded retention", () => { + expect(PRODUCT_SYNC_FAILED_RETENTION_MS).toBeGreaterThan( + PRODUCT_SYNC_SUCCEEDED_RETENTION_MS, + ); + }); +}); diff --git a/packages/kit/convex/products/jobs.ts b/packages/kit/convex/products/jobs.ts new file mode 100644 index 00000000..e5349a58 --- /dev/null +++ b/packages/kit/convex/products/jobs.ts @@ -0,0 +1,487 @@ +import { v } from "convex/values"; +import { getAuthUserId } from "@convex-dev/auth/server"; + +import { + internalAction, + internalMutation, + internalQuery, + mutation, + query, + type QueryCtx, +} from "../_generated/server"; +import { internal } from "../_generated/api"; +import type { Doc, Id } from "../_generated/dataModel"; +import { ErrorCode, createError } from "../utils/errors"; + +// Per-job hard ceiling. Convex actions cap at ~10min; we allow 9min +// for the worker and rely on the reaper to mark anything still +// running 1min past that as failed. +export const PRODUCT_SYNC_JOB_DEADLINE_MS = 9 * 60 * 1_000; +export const PRODUCT_SYNC_REAPER_GRACE_MS = 60 * 1_000; +export const PRODUCT_SYNC_SUCCEEDED_RETENTION_MS = 7 * 24 * 60 * 60 * 1_000; +export const PRODUCT_SYNC_FAILED_RETENTION_MS = 30 * 24 * 60 * 60 * 1_000; +export const PRODUCT_SYNC_FAILURES_CAP = 200; + +// Cap the failures array stored on the job row so a runaway sync +// (every product fails for the same upstream config reason) doesn't +// blow past Convex's per-document size budget. The dashboard sees +// `failuresTruncated: true` and renders a notice; the operator can +// re-run after fixing the root cause. +export function truncateFailures< + T extends { productId: string; reason: string }, +>(failures: T[]): { items: T[]; truncated: boolean } { + if (failures.length <= PRODUCT_SYNC_FAILURES_CAP) { + return { items: failures, truncated: false }; + } + return { + items: failures.slice(0, PRODUCT_SYNC_FAILURES_CAP), + truncated: true, + }; +} + +const platformValidator = v.union(v.literal("IOS"), v.literal("Android")); +const directionValidator = v.union( + v.literal("pull"), + v.literal("push"), + v.literal("both"), + v.literal("purge-local"), +); + +async function requireProjectMember( + ctx: QueryCtx, + apiKey: string, +): Promise<{ project: Doc<"projects">; userId: Id<"users"> }> { + const userId = await getAuthUserId(ctx); + if (!userId) { + throw createError(ErrorCode.NOT_AUTHENTICATED); + } + const project = await ctx.db + .query("projects") + .withIndex("by_api_key", (q) => q.eq("apiKey", apiKey)) + .first(); + if (!project) { + throw createError(ErrorCode.PROJECT_NOT_FOUND); + } + const membership = await ctx.db + .query("organizationMembers") + .withIndex("by_org_and_user", (q) => + q.eq("organizationId", project.organizationId).eq("userId", userId), + ) + .first(); + if (!membership) { + throw createError(ErrorCode.NOT_ORGANIZATION_MEMBER); + } + return { project, userId }; +} + +async function requireJobAccess( + ctx: QueryCtx, + jobId: Id<"productSyncJobs">, +): Promise<{ job: Doc<"productSyncJobs">; userId: Id<"users"> }> { + const userId = await getAuthUserId(ctx); + if (!userId) { + throw createError(ErrorCode.NOT_AUTHENTICATED); + } + const job = await ctx.db.get(jobId); + if (!job) { + throw createError(ErrorCode.INVALID_INPUT, "Sync job not found"); + } + const project = await ctx.db.get(job.projectId); + if (!project) { + throw createError(ErrorCode.PROJECT_NOT_FOUND); + } + const membership = await ctx.db + .query("organizationMembers") + .withIndex("by_org_and_user", (q) => + q.eq("organizationId", project.organizationId).eq("userId", userId), + ) + .first(); + if (!membership) { + throw createError(ErrorCode.NOT_ORGANIZATION_MEMBER); + } + return { job, userId }; +} + +// Latest job (any status) for a project+platform — drives the +// dashboard's button state, progress, and last-result toast. +export const getActiveSyncJob = query({ + args: { + apiKey: v.string(), + platform: platformValidator, + }, + handler: async (ctx, args) => { + const { project } = await requireProjectMember(ctx, args.apiKey); + return await ctx.db + .query("productSyncJobs") + .withIndex("by_project_and_created", (q) => + q.eq("projectId", project._id), + ) + .filter((q) => q.eq(q.field("platform"), args.platform)) + .order("desc") + .first(); + }, +}); + +export const getSyncJobById = query({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + const { job } = await requireJobAccess(ctx, args.jobId); + return job; + }, +}); + +// Idempotent enqueue: if there's already a queued/running job for this +// (project, platform) we return the existing jobId so a double-tap or +// page reload doesn't fan out duplicate workers. +export const enqueueProductSync = mutation({ + args: { + apiKey: v.string(), + platform: platformValidator, + direction: v.optional(directionValidator), + dryRun: v.optional(v.boolean()), + }, + returns: v.object({ + jobId: v.id("productSyncJobs"), + deduped: v.boolean(), + }), + handler: async (ctx, args) => { + const { project, userId } = await requireProjectMember(ctx, args.apiKey); + const existingActive = await ctx.db + .query("productSyncJobs") + .withIndex("by_project_platform_status", (q) => + q + .eq("projectId", project._id) + .eq("platform", args.platform) + .eq("status", "queued"), + ) + .first(); + const existingRunning = await ctx.db + .query("productSyncJobs") + .withIndex("by_project_platform_status", (q) => + q + .eq("projectId", project._id) + .eq("platform", args.platform) + .eq("status", "running"), + ) + .first(); + const existing = existingActive ?? existingRunning; + if (existing) { + return { jobId: existing._id, deduped: true }; + } + const now = Date.now(); + const jobId = await ctx.db.insert("productSyncJobs", { + projectId: project._id, + platform: args.platform, + direction: args.direction ?? "both", + dryRun: args.dryRun ?? false, + status: "queued", + progress: { phase: "queued" }, + createdBy: userId, + createdAt: now, + }); + if (args.direction === "purge-local") { + // Purge runs in this module's V8-isolate runtime — no Apple + // credentials, no Play OAuth, no `"use node"` cost. Just a + // bounded delete loop against `products`. + await ctx.scheduler.runAfter( + 0, + internal.products.jobs.runProductSyncPurgeLocal, + { jobId }, + ); + } else if (args.platform === "IOS") { + await ctx.scheduler.runAfter(0, internal.products.asc.runProductSyncIOS, { + jobId, + }); + } else { + await ctx.scheduler.runAfter( + 0, + internal.products.play.runProductSyncAndroid, + { jobId }, + ); + } + return { jobId, deduped: false }; + }, +}); + +// Worker for `direction: "purge-local"`. Empties kit's local +// `products` rows for the (project, platform) in page-bounded +// batches; never touches App Store Connect or Play Console. The +// next regular sync re-pulls from the upstream store, so this is +// the recovery hatch when kit's cache drifts (manual store edits, +// failed partial pushes, stale prices). Cancel checks between +// pages so an operator can stop a runaway wipe within seconds. +export const runProductSyncPurgeLocal = internalAction({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args): Promise => { + const job = await ctx.runQuery(internal.products.jobs.getJobForWorker, { + jobId: args.jobId, + }); + if (!job) return; + if (job.status !== "queued") return; + await ctx.runMutation(internal.products.jobs.markJobRunning, { + jobId: args.jobId, + }); + try { + const PAGE = 100; + let total = 0; + // 200-page guard caps a runaway loop at 20k rows — far past + // any real project's catalog. The bounded `take(limit + 1)` + // inside `deletePlatformCatalog` decides `hasMore` from the + // overflow row, so we exit cleanly the moment the page returns + // short. + for (let page = 0; page < 200; page += 1) { + const cancelled = await ctx.runQuery( + internal.products.jobs.isCancelRequested, + { jobId: args.jobId }, + ); + if (cancelled) { + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: "Cancelled by operator", + }); + return; + } + await ctx.runMutation(internal.products.jobs.updateJobProgress, { + jobId: args.jobId, + phase: "purge-local", + current: total, + }); + const { deleted, hasMore } = await ctx.runMutation( + internal.products.sync.deletePlatformCatalog, + { + projectId: job.projectId, + platform: job.platform, + limit: PAGE, + }, + ); + total += deleted; + if (!hasMore) break; + } + await ctx.runMutation(internal.products.jobs.markJobSucceeded, { + jobId: args.jobId, + pulled: 0, + pushed: 0, + deleted: total, + failures: [], + }); + } catch (error) { + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: error instanceof Error ? error.message : String(error), + }); + } + }, +}); + +// Operator-initiated cancel. The worker checks `cancelRequested` at +// phase boundaries — granularity is per-phase, not per-product, but +// that's enough to stop a runaway sync within seconds on most paths. +export const cancelProductSync = mutation({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + const { job } = await requireJobAccess(ctx, args.jobId); + if (job.status !== "queued" && job.status !== "running") { + return { ok: false, reason: "not active" as const }; + } + await ctx.db.patch(job._id, { cancelRequested: true }); + return { ok: true as const }; + }, +}); + +// Soft-dismiss a finished job from the dashboard. Doesn't delete the +// row (the pruner handles retention) — just makes a future +// `getActiveSyncJob` skip it. +// +// Implemented via a tombstone field instead of a status change so +// audit-style queries can still see succeeded/failed history. +export const dismissCompletedJob = mutation({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + const { job } = await requireJobAccess(ctx, args.jobId); + if (job.status !== "succeeded" && job.status !== "failed") { + return { ok: false as const }; + } + // Reuse `cancelRequested` as a dismissal marker would muddle + // semantics; instead we shift `completedAt` into the past so the + // active-job query (ordered desc by createdAt) still sees newer + // jobs while the dashboard filter on the client treats this row + // as "dismissed" via `dismissed: true`. + await ctx.db.patch(job._id, { + progress: { ...job.progress, phase: "dismissed" }, + }); + return { ok: true as const }; + }, +}); + +// Worker-side helpers (internal — only the runner actions call these). + +export const getJobForWorker = internalQuery({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + return await ctx.db.get(args.jobId); + }, +}); + +export const isCancelRequested = internalQuery({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + const job = await ctx.db.get(args.jobId); + return job?.cancelRequested === true; + }, +}); + +export const markJobRunning = internalMutation({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args) => { + const job = await ctx.db.get(args.jobId); + if (!job) return; + if (job.status !== "queued") return; + const now = Date.now(); + await ctx.db.patch(args.jobId, { + status: "running", + startedAt: now, + expectedDeadline: now + PRODUCT_SYNC_JOB_DEADLINE_MS, + progress: { phase: "starting" }, + }); + }, +}); + +export const updateJobProgress = internalMutation({ + args: { + jobId: v.id("productSyncJobs"), + phase: v.string(), + current: v.optional(v.number()), + total: v.optional(v.number()), + failuresCount: v.optional(v.number()), + }, + handler: async (ctx, args) => { + await ctx.db.patch(args.jobId, { + progress: { + phase: args.phase, + current: args.current, + total: args.total, + failuresCount: args.failuresCount, + }, + }); + }, +}); + +export const markJobSucceeded = internalMutation({ + args: { + jobId: v.id("productSyncJobs"), + pulled: v.number(), + pushed: v.number(), + deleted: v.optional(v.number()), + failures: v.array(v.object({ productId: v.string(), reason: v.string() })), + plannedWrites: v.optional( + v.array( + v.object({ + productId: v.string(), + step: v.string(), + detail: v.optional(v.string()), + }), + ), + ), + }, + handler: async (ctx, args) => { + const { items: failures, truncated } = truncateFailures(args.failures); + await ctx.db.patch(args.jobId, { + status: "succeeded", + completedAt: Date.now(), + progress: { + phase: "completed", + failuresCount: args.failures.length, + }, + result: { + pulled: args.pulled, + pushed: args.pushed, + ...(args.deleted !== undefined ? { deleted: args.deleted } : {}), + failures, + ...(truncated ? { failuresTruncated: true } : {}), + ...(args.plannedWrites ? { plannedWrites: args.plannedWrites } : {}), + }, + }); + }, +}); + +export const markJobFailed = internalMutation({ + args: { + jobId: v.id("productSyncJobs"), + error: v.string(), + pulled: v.optional(v.number()), + pushed: v.optional(v.number()), + failures: v.optional( + v.array(v.object({ productId: v.string(), reason: v.string() })), + ), + }, + handler: async (ctx, args) => { + const rawFailures = args.failures ?? []; + const { items: failures, truncated } = truncateFailures(rawFailures); + await ctx.db.patch(args.jobId, { + status: "failed", + completedAt: Date.now(), + error: args.error, + progress: { + phase: "failed", + failuresCount: failures.length, + }, + result: { + pulled: args.pulled ?? 0, + pushed: args.pushed ?? 0, + failures, + ...(truncated ? { failuresTruncated: true } : {}), + }, + }); + }, +}); + +// Cron: flip `running` rows whose `expectedDeadline + grace` has +// passed to `failed("worker timed out")`. Without this, a crashed +// action permanently pins the project's "active job" slot. +export const reapStaleProductSyncJobs = internalMutation({ + args: {}, + handler: async (ctx) => { + const cutoff = Date.now() - PRODUCT_SYNC_REAPER_GRACE_MS; + const stale = await ctx.db + .query("productSyncJobs") + .withIndex("by_status_and_deadline", (q) => + q.eq("status", "running").lt("expectedDeadline", cutoff), + ) + .take(50); + for (const job of stale) { + await ctx.db.patch(job._id, { + status: "failed", + completedAt: Date.now(), + error: "Worker timed out — sync exceeded the 9-minute ceiling", + progress: { phase: "reaped" }, + }); + } + return { reaped: stale.length }; + }, +}); + +// Cron: drop succeeded jobs older than 7d, failed older than 30d. +export const pruneProductSyncJobs = internalMutation({ + args: {}, + handler: async (ctx) => { + const now = Date.now(); + const succeededCutoff = now - PRODUCT_SYNC_SUCCEEDED_RETENTION_MS; + const failedCutoff = now - PRODUCT_SYNC_FAILED_RETENTION_MS; + const succeeded = await ctx.db + .query("productSyncJobs") + .withIndex("by_status_and_completed", (q) => + q.eq("status", "succeeded").lt("completedAt", succeededCutoff), + ) + .take(100); + const failed = await ctx.db + .query("productSyncJobs") + .withIndex("by_status_and_completed", (q) => + q.eq("status", "failed").lt("completedAt", failedCutoff), + ) + .take(100); + for (const row of [...succeeded, ...failed]) { + await ctx.db.delete(row._id); + } + return { pruned: succeeded.length + failed.length }; + }, +}); diff --git a/packages/kit/convex/products/play.ts b/packages/kit/convex/products/play.ts index bc4ebe68..5ff02560 100644 --- a/packages/kit/convex/products/play.ts +++ b/packages/kit/convex/products/play.ts @@ -3,11 +3,17 @@ import { v } from "convex/values"; import { google } from "googleapis"; import type { androidpublisher_v3 } from "googleapis"; -import { action } from "../_generated/server"; +import { internalAction, type ActionCtx } from "../_generated/server"; import { internal } from "../_generated/api"; -import { getProjectByApiKey } from "../purchases/shared"; import { coerceBillingPeriod } from "./sync"; +class ProductSyncCancelledError extends Error { + constructor() { + super("Sync cancelled by operator"); + this.name = "ProductSyncCancelledError"; + } +} + /** * Per-product upstream rejection reported back to the dashboard. Used * inside `pushSyncProductsGoogle`'s `failures` array; extracted so the @@ -52,403 +58,513 @@ export interface ProductSyncFailure { * carrying per-product upstream rejection reasons so the * dashboard can render them. */ -export const pushSyncProductsGoogle = action({ - args: { - apiKey: v.string(), - direction: v.optional( - v.union(v.literal("pull"), v.literal("push"), v.literal("both")), - ), - }, - returns: v.object({ - pulled: v.number(), - pushed: v.number(), - failures: v.array(v.object({ productId: v.string(), reason: v.string() })), - }), - handler: async ( - ctx, - args, - ): Promise<{ - pulled: number; - pushed: number; - failures: ProductSyncFailure[]; - }> => { - const project = await getProjectByApiKey(ctx, args.apiKey); - if (!project.androidPackageName) { - throw new Error("Project androidPackageName is not configured"); - } - - const serviceAccountFile = await ctx.runQuery( - internal.files.internal.getGooglePlayFileByProjectInternal, - { projectId: project._id }, - ); - if (!serviceAccountFile) { - throw new Error( - "Google Play service account JSON not found — upload it before running push-sync", +// Worker that drives a single Google Play sync job. See the parallel +// docstring on `runProductSyncIOS` in `products/asc.ts` — same job +// lifecycle, same cancel-at-phase-boundary semantics. +export const runProductSyncAndroid = internalAction({ + args: { jobId: v.id("productSyncJobs") }, + handler: async (ctx, args): Promise => { + const job = await ctx.runQuery(internal.products.jobs.getJobForWorker, { + jobId: args.jobId, + }); + if (!job) return; + if (job.status !== "queued") return; + await ctx.runMutation(internal.products.jobs.markJobRunning, { + jobId: args.jobId, + }); + const checkCancelled = async () => { + const cancelled = await ctx.runQuery( + internal.products.jobs.isCancelRequested, + { jobId: args.jobId }, ); + if (cancelled) throw new ProductSyncCancelledError(); + }; + const reportPhase = async ( + phase: string, + extra?: { + current?: number; + total?: number; + failuresCount?: number; + }, + ) => { + await ctx.runMutation(internal.products.jobs.updateJobProgress, { + jobId: args.jobId, + phase, + current: extra?.current, + total: extra?.total, + failuresCount: extra?.failuresCount, + }); + }; + if (job.direction === "purge-local") { + // enqueue routes purge-local jobs to a different worker; this + // branch is unreachable in practice but narrows the type for + // the call below. + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: "purge-local routed to wrong worker", + }); + return; } - const fileContent = await ctx.runAction( - internal.files.internal.readFileAsText, - { fileId: serviceAccountFile._id }, - ); - if (!fileContent?.content) { - throw new Error("Service account JSON file is unreadable"); - } - // Wrap the parse so a malformed JSON upload yields an actionable - // config error ("Service account JSON is invalid") instead of a - // raw SyntaxError from JSON.parse, which surfaces as a generic - // 500 with no operator-friendly hint. - let credentials: Record; try { - credentials = JSON.parse(fileContent.content) as Record; - } catch { - throw new Error( - "Service account JSON is invalid — re-upload the file from Google Cloud Console", - ); + const result = await performAndroidSync(ctx, { + projectId: job.projectId, + direction: job.direction, + dryRun: job.dryRun, + checkCancelled, + reportPhase, + }); + await ctx.runMutation(internal.products.jobs.markJobSucceeded, { + jobId: args.jobId, + pulled: result.pulled, + pushed: result.pushed, + failures: result.failures, + plannedWrites: result.plannedWrites, + }); + } catch (error) { + const cancelled = error instanceof ProductSyncCancelledError; + const message = cancelled + ? "Cancelled by operator" + : error instanceof Error + ? error.message + : String(error); + await ctx.runMutation(internal.products.jobs.markJobFailed, { + jobId: args.jobId, + error: message, + }); } + }, +}); - const auth = new google.auth.GoogleAuth({ - credentials, - scopes: ["https://www.googleapis.com/auth/androidpublisher"], - }); - const androidpublisher = google.androidpublisher({ version: "v3", auth }); - const packageName = project.androidPackageName; - const direction = args.direction ?? "both"; - const failures: ProductSyncFailure[] = []; - let pulled = 0; - let pushed = 0; +async function performAndroidSync( + ctx: ActionCtx, + options: { + projectId: import("../_generated/dataModel").Id<"projects">; + direction: "pull" | "push" | "both"; + dryRun: boolean; + checkCancelled: () => Promise; + reportPhase: ( + phase: string, + extra?: { + current?: number; + total?: number; + failuresCount?: number; + }, + ) => Promise; + }, +): Promise<{ + pulled: number; + pushed: number; + failures: ProductSyncFailure[]; + plannedWrites?: Array<{ productId: string; step: string; detail?: string }>; +}> { + const project = await ctx.runQuery( + internal.projects.internal.getProjectById, + { projectId: options.projectId }, + ); + if (!project) { + throw new Error("Project not found for sync job"); + } + if (!project.androidPackageName) { + throw new Error("Project androidPackageName is not configured"); + } + const args = { direction: options.direction }; + const { checkCancelled, reportPhase } = options; - // ── PULL: Play → kit ───────────────────────────────────────── - if (direction === "pull" || direction === "both") { - // One-time products. Play has TWO catalog APIs and apps live in - // different ones depending on when/how they were set up: - // - // - `inappproducts.list` — legacy v1 endpoint. Apps created - // before the new monetization framework store products here. - // - `monetization.onetimeproducts.list` — new endpoint. Apps - // onboarded under "Manage products" in the modern Play - // Console store products HERE and `inappproducts.list` - // silently returns empty for them. (This is why "Sync with - // Play Console" was only pulling subscriptions for accounts - // using the new console — the one-time products were - // invisible to the legacy endpoint.) - // - // We hit both, dedupe by SKU, and keep going on either failing - // — that way an account that lives entirely in one or the other - // still gets a complete pull instead of failing on the missing - // half. - // - // ORDER MATTERS: hit the new monetization API first so its - // USD-preferred regional price wins. The legacy - // `inappproducts.list` only exposes a single `defaultPrice` - // (whatever currency the merchant set in Play Console — often - // their home currency) which made products like - // `dev.hyo.martie.10bulbs` show up as "AED 3.89" on the - // dashboard for an operator using a Korean Play Console where - // AED happens to be a regional override. New endpoint runs - // first; legacy only fills in skus the new endpoint missed. - const seenOneTimeSkus = new Set(); - try { - // Defensive guard: the new monetization API isn't surfaced in - // any typed shape by `googleapis` yet, so we cast through - // `unknown` and read the (possibly-missing) `onetimeproducts` - // property. `androidpublisher.monetization` is documented but - // could change shape in a future SDK release; failing soft - // (treating it as "no monetization endpoint here") lets the - // legacy `inappproducts.list` path below still pull what it - // can instead of bailing the entire pull half-done. The - // outer try/catch records the failure in the per-product - // `failures` array so the operator sees something happened. - const monetizationApi = androidpublisher.monetization as - | { onetimeproducts?: unknown } - | undefined; - const onetime = ( - monetizationApi as unknown as { - onetimeproducts?: { - list: (params: { - packageName: string; - pageToken?: string; - }) => Promise<{ - data: { - oneTimeProducts?: Array<{ - productId?: string; - listings?: Array<{ - languageCode?: string; - title?: string; - description?: string; - }>; - purchaseOptions?: Array<{ - state?: string; - purchaseOptionId?: string; - buyOption?: { legacyCompatible?: boolean }; - rentOption?: unknown; - // Pricing lives DIRECTLY on the purchaseOption, - // NOT nested inside buyOption. The earlier shape - // (buyOption.regionalPricingAndAvailabilityConfigs) - // was wrong — every one-time product surfaced - // with no price because the lookup never matched. - regionalPricingAndAvailabilityConfigs?: Array<{ - regionCode?: string; - // Google's enum: "AVAILABLE", - // "NO_LONGER_AVAILABLE", "AVAILABLE_IF_RELEASED". - // Stale rows from removed regions still ship - // back with a price attached, so without this - // field we'd happily display a price the - // operator turned off years ago. - availability?: string; - price?: { - currencyCode?: string; - units?: string; - nanos?: number; - }; - }>; + const serviceAccountFile = await ctx.runQuery( + internal.files.internal.getGooglePlayFileByProjectInternal, + { projectId: project._id }, + ); + if (!serviceAccountFile) { + throw new Error( + "Google Play service account JSON not found — upload it before running push-sync", + ); + } + const fileContent = await ctx.runAction( + internal.files.internal.readFileAsText, + { fileId: serviceAccountFile._id }, + ); + if (!fileContent?.content) { + throw new Error("Service account JSON file is unreadable"); + } + // Wrap the parse so a malformed JSON upload yields an actionable + // config error ("Service account JSON is invalid") instead of a + // raw SyntaxError from JSON.parse, which surfaces as a generic + // 500 with no operator-friendly hint. + let credentials: Record; + try { + credentials = JSON.parse(fileContent.content) as Record; + } catch { + throw new Error( + "Service account JSON is invalid — re-upload the file from Google Cloud Console", + ); + } + + const auth = new google.auth.GoogleAuth({ + credentials, + scopes: ["https://www.googleapis.com/auth/androidpublisher"], + }); + const androidpublisher = google.androidpublisher({ version: "v3", auth }); + const packageName = project.androidPackageName; + const direction = args.direction ?? "both"; + const dryRun = options.dryRun; + const failures: ProductSyncFailure[] = []; + // Same shape as the iOS sync — read-only preview accumulator. Each + // upstream write the PUSH branch would have made gets pushed onto + // this list instead, then surfaced through the toast / result + // banner so the operator can verify base-plan duration, region + // pricing, etc. before committing. + const plannedWrites: Array<{ + productId: string; + step: string; + detail?: string; + }> = []; + let pulled = 0; + let pushed = 0; + + // ── PULL: Play → kit ───────────────────────────────────────── + if (direction === "pull" || direction === "both") { + await checkCancelled(); + await reportPhase("pull-products"); + // One-time products. Play has TWO catalog APIs and apps live in + // different ones depending on when/how they were set up: + // + // - `inappproducts.list` — legacy v1 endpoint. Apps created + // before the new monetization framework store products here. + // - `monetization.onetimeproducts.list` — new endpoint. Apps + // onboarded under "Manage products" in the modern Play + // Console store products HERE and `inappproducts.list` + // silently returns empty for them. (This is why "Sync with + // Play Console" was only pulling subscriptions for accounts + // using the new console — the one-time products were + // invisible to the legacy endpoint.) + // + // We hit both, dedupe by SKU, and keep going on either failing + // — that way an account that lives entirely in one or the other + // still gets a complete pull instead of failing on the missing + // half. + // + // ORDER MATTERS: hit the new monetization API first so its + // USD-preferred regional price wins. The legacy + // `inappproducts.list` only exposes a single `defaultPrice` + // (whatever currency the merchant set in Play Console — often + // their home currency) which made products like + // `dev.hyo.martie.10bulbs` show up as "AED 3.89" on the + // dashboard for an operator using a Korean Play Console where + // AED happens to be a regional override. New endpoint runs + // first; legacy only fills in skus the new endpoint missed. + const seenOneTimeSkus = new Set(); + try { + // Defensive guard: the new monetization API isn't surfaced in + // any typed shape by `googleapis` yet, so we cast through + // `unknown` and read the (possibly-missing) `onetimeproducts` + // property. `androidpublisher.monetization` is documented but + // could change shape in a future SDK release; failing soft + // (treating it as "no monetization endpoint here") lets the + // legacy `inappproducts.list` path below still pull what it + // can instead of bailing the entire pull half-done. The + // outer try/catch records the failure in the per-product + // `failures` array so the operator sees something happened. + const monetizationApi = androidpublisher.monetization as + | { onetimeproducts?: unknown } + | undefined; + const onetime = ( + monetizationApi as unknown as { + onetimeproducts?: { + list: (params: { + packageName: string; + pageToken?: string; + }) => Promise<{ + data: { + oneTimeProducts?: Array<{ + productId?: string; + listings?: Array<{ + languageCode?: string; + title?: string; + description?: string; + }>; + purchaseOptions?: Array<{ + state?: string; + purchaseOptionId?: string; + buyOption?: { legacyCompatible?: boolean }; + rentOption?: unknown; + // Pricing lives DIRECTLY on the purchaseOption, + // NOT nested inside buyOption. The earlier shape + // (buyOption.regionalPricingAndAvailabilityConfigs) + // was wrong — every one-time product surfaced + // with no price because the lookup never matched. + regionalPricingAndAvailabilityConfigs?: Array<{ + regionCode?: string; + // Google's enum: "AVAILABLE", + // "NO_LONGER_AVAILABLE", "AVAILABLE_IF_RELEASED". + // Stale rows from removed regions still ship + // back with a price attached, so without this + // field we'd happily display a price the + // operator turned off years ago. + availability?: string; + price?: { + currencyCode?: string; + units?: string; + nanos?: number; + }; }>; }>; - nextPageToken?: string; - }; - }>; - }; - } - ).onetimeproducts; - if (onetime?.list) { - let token: string | undefined; - let pageCount = 0; - do { - const resp = await onetime.list({ - packageName, - ...(token ? { pageToken: token } : {}), - }); - for (const product of resp.data.oneTimeProducts ?? []) { - if (!product.productId) continue; - if (seenOneTimeSkus.has(product.productId)) continue; - seenOneTimeSkus.add(product.productId); - const listing = product.listings?.[0]; - // Walk every purchaseOption × regionalPricingAndAvailabilityConfig - // (pricing lives on the option, not inside buyOption). - // Two filters before ranking: - // - drop regions explicitly NO_LONGER_AVAILABLE so we - // don't surface stale pricing the operator removed. - // - require the price to have a `units` field — Google - // ships zero-priced placeholder rows for some regions - // and they'd outrank real prices alphabetically. - // Ranking: regionCode === "US" first (canonical kit - // display currency, deterministically maps to USD), - // then any USD-currency region (covers operators who - // override the US region price into a non-USD currency - // — rare but possible), then the first remaining region - // alphabetically by currency for a stable result. - const priceCandidates: Array<{ - regionCode?: string; - currencyCode?: string; - units?: string; - nanos?: number; - }> = []; - for (const opt of product.purchaseOptions ?? []) { - for (const region of opt.regionalPricingAndAvailabilityConfigs ?? - []) { - if (region.availability === "NO_LONGER_AVAILABLE") continue; - if (region.price && typeof region.price.units === "string") { - priceCandidates.push({ - regionCode: region.regionCode, - currencyCode: region.price.currencyCode, - units: region.price.units, - nanos: region.price.nanos, - }); - } - } - } - priceCandidates.sort((a, b) => - (a.currencyCode ?? "").localeCompare(b.currencyCode ?? ""), - ); - const preferred = - priceCandidates.find((p) => p.regionCode === "US") ?? - priceCandidates.find((p) => p.currencyCode === "USD") ?? - priceCandidates[0]; - const priceAmountMicros = preferred - ? moneyToMicros({ - units: preferred.units, - nanos: preferred.nanos, - }) - : undefined; - await ctx.runMutation(internal.products.sync.upsertFromStore, { - projectId: project._id, - productId: product.productId, - platform: "Android", - // The new API doesn't carry a "consumable vs. - // non-consumable" distinction the same way — Play - // tracks consumption at purchase time. Default to - // NonConsumable; operators can edit on the kit side. - type: "NonConsumable", - title: listing?.title ?? product.productId, - description: listing?.description ?? undefined, - priceAmountMicros, - currency: preferred?.currencyCode ?? undefined, - storeRef: product.productId, - state: "Active", - }); - pulled += 1; - } - token = resp.data.nextPageToken ?? undefined; - pageCount += 1; - if (pageCount > 50) break; - } while (token); - } - } catch (error) { - failures.push({ - productId: "(play list onetimeproducts)", - reason: error instanceof Error ? error.message : String(error), - }); - } - - // Legacy `inappproducts.list` runs SECOND so any sku already - // surfaced by the new endpoint (with USD-preferred pricing) wins - // via the dedupe set. Only skus invisible to the new endpoint - // get filled in here with whatever `defaultPrice` the merchant - // set in Play Console. - try { - let token: string | undefined; - let pageCount = 0; - do { - const oneTimes = await androidpublisher.inappproducts.list({ - packageName, - ...(token ? { token } : {}), - }); - for (const product of oneTimes.data.inappproduct ?? []) { - if (!product.sku) continue; - if (seenOneTimeSkus.has(product.sku)) continue; - seenOneTimeSkus.add(product.sku); - if (product.purchaseType === "subscription") continue; - await ctx.runMutation(internal.products.sync.upsertFromStore, { - projectId: project._id, - productId: product.sku, - platform: "Android", - type: mapPlayOneTimeType(product), - title: pickPlayTitle(product) ?? product.sku, - description: pickPlayDescription(product), - priceAmountMicros: parsePlayPriceMicros(product), - currency: pickPlayCurrency(product), - storeRef: product.sku, - state: mapPlayStatus(product.status), - }); - pulled += 1; - } - token = oneTimes.data.tokenPagination?.nextPageToken ?? undefined; - pageCount += 1; - if (pageCount > 50) break; - } while (token); - } catch (error) { - // The legacy `inappproducts.list` endpoint is deprecated for - // newer Play Console accounts and Google now responds with - // "Please migrate to the new publishing API". That message is - // expected — the new `monetization.onetimeproducts.list` call - // above already covers this account — and surfacing it as a - // failure produces a noisy red toast every Sync. Suppress it - // when seen; surface anything else. - const reason = error instanceof Error ? error.message : String(error); - if (!/migrate to the new publishing API/i.test(reason)) { - failures.push({ - productId: "(play list inappproducts)", - reason, - }); + }>; + nextPageToken?: string; + }; + }>; + }; } - } - - try { + ).onetimeproducts; + if (onetime?.list) { let token: string | undefined; let pageCount = 0; do { - const subs = await androidpublisher.monetization.subscriptions.list({ + const resp = await onetime.list({ packageName, ...(token ? { pageToken: token } : {}), }); - for (const sub of subs.data.subscriptions ?? []) { - if (!sub.productId) continue; - const { priceAmountMicros, currency, basePlanId } = - pickSubBasePlanPrice(sub); - const offers = collectPlaySubscriptionOffers(sub); - // Pick the billingPeriod from the *same* base plan whose - // price we just selected (`basePlanId` returned by - // pickSubBasePlanPrice). If we can't find that exact plan - // in `offers`, fall back to the first BasePlan row — but - // this fallback only triggers when basePlanId is missing, - // which means the subscription has no price at all. - // Without the basePlanId match, mixed monthly + yearly - // products would pair the yearly USD price with the - // monthly duration and break MRR normalization. - const billingPeriod = ( - basePlanId - ? offers.find( - (o) => o.kind === "BasePlan" && o.id === basePlanId, - ) - : offers.find((o) => o.kind === "BasePlan") - )?.duration; + for (const product of resp.data.oneTimeProducts ?? []) { + if (!product.productId) continue; + if (seenOneTimeSkus.has(product.productId)) continue; + seenOneTimeSkus.add(product.productId); + const listing = product.listings?.[0]; + // Walk every purchaseOption × regionalPricingAndAvailabilityConfig + // (pricing lives on the option, not inside buyOption). + // Two filters before ranking: + // - drop regions explicitly NO_LONGER_AVAILABLE so we + // don't surface stale pricing the operator removed. + // - require the price to have a `units` field — Google + // ships zero-priced placeholder rows for some regions + // and they'd outrank real prices alphabetically. + // Ranking: regionCode === "US" first (canonical kit + // display currency, deterministically maps to USD), + // then any USD-currency region (covers operators who + // override the US region price into a non-USD currency + // — rare but possible), then the first remaining region + // alphabetically by currency for a stable result. + const priceCandidates: Array<{ + regionCode?: string; + currencyCode?: string; + units?: string; + nanos?: number; + }> = []; + for (const opt of product.purchaseOptions ?? []) { + for (const region of opt.regionalPricingAndAvailabilityConfigs ?? + []) { + if (region.availability === "NO_LONGER_AVAILABLE") continue; + if (region.price && typeof region.price.units === "string") { + priceCandidates.push({ + regionCode: region.regionCode, + currencyCode: region.price.currencyCode, + units: region.price.units, + nanos: region.price.nanos, + }); + } + } + } + priceCandidates.sort((a, b) => + (a.currencyCode ?? "").localeCompare(b.currencyCode ?? ""), + ); + const preferred = + priceCandidates.find((p) => p.regionCode === "US") ?? + priceCandidates.find((p) => p.currencyCode === "USD") ?? + priceCandidates[0]; + const priceAmountMicros = preferred + ? moneyToMicros({ + units: preferred.units, + nanos: preferred.nanos, + }) + : undefined; await ctx.runMutation(internal.products.sync.upsertFromStore, { projectId: project._id, - productId: sub.productId, + productId: product.productId, platform: "Android", - type: "Subscription", - title: sub.listings?.[0]?.title ?? sub.productId, - description: sub.listings?.[0]?.description ?? undefined, + // The new API doesn't carry a "consumable vs. + // non-consumable" distinction the same way — Play + // tracks consumption at purchase time. Default to + // NonConsumable; operators can edit on the kit side. + type: "NonConsumable", + title: listing?.title ?? product.productId, + description: listing?.description ?? undefined, priceAmountMicros, - currency, - storeRef: sub.productId, + currency: preferred?.currencyCode ?? undefined, + storeRef: product.productId, state: "Active", - billingPeriod: coerceBillingPeriod(billingPeriod), - // Play has no first-class subscription "group" — base - // plans on a single subscription product play that role, - // and we surface them as `offers[].kind === "BasePlan"` - // rows. Leave the ASC-only group fields unset. - offers: offers.length ? offers : undefined, }); pulled += 1; } - token = subs.data.nextPageToken ?? undefined; + token = resp.data.nextPageToken ?? undefined; pageCount += 1; if (pageCount > 50) break; } while (token); - } catch (error) { + } + } catch (error) { + failures.push({ + productId: "(play list onetimeproducts)", + reason: error instanceof Error ? error.message : String(error), + }); + } + + // Legacy `inappproducts.list` runs SECOND so any sku already + // surfaced by the new endpoint (with USD-preferred pricing) wins + // via the dedupe set. Only skus invisible to the new endpoint + // get filled in here with whatever `defaultPrice` the merchant + // set in Play Console. + try { + let token: string | undefined; + let pageCount = 0; + do { + const oneTimes = await androidpublisher.inappproducts.list({ + packageName, + ...(token ? { token } : {}), + }); + for (const product of oneTimes.data.inappproduct ?? []) { + if (!product.sku) continue; + if (seenOneTimeSkus.has(product.sku)) continue; + seenOneTimeSkus.add(product.sku); + if (product.purchaseType === "subscription") continue; + await ctx.runMutation(internal.products.sync.upsertFromStore, { + projectId: project._id, + productId: product.sku, + platform: "Android", + type: mapPlayOneTimeType(product), + title: pickPlayTitle(product) ?? product.sku, + description: pickPlayDescription(product), + priceAmountMicros: parsePlayPriceMicros(product), + currency: pickPlayCurrency(product), + storeRef: product.sku, + state: mapPlayStatus(product.status), + }); + pulled += 1; + } + token = oneTimes.data.tokenPagination?.nextPageToken ?? undefined; + pageCount += 1; + if (pageCount > 50) break; + } while (token); + } catch (error) { + // The legacy `inappproducts.list` endpoint is deprecated for + // newer Play Console accounts and Google now responds with + // "Please migrate to the new publishing API". That message is + // expected — the new `monetization.onetimeproducts.list` call + // above already covers this account — and surfacing it as a + // failure produces a noisy red toast every Sync. Suppress it + // when seen; surface anything else. + const reason = error instanceof Error ? error.message : String(error); + if (!/migrate to the new publishing API/i.test(reason)) { failures.push({ - productId: "(play list subscriptions)", - reason: error instanceof Error ? error.message : String(error), + productId: "(play list inappproducts)", + reason, }); } } - // ── PUSH: kit → Play for Draft rows ────────────────────────── - if (direction === "push" || direction === "both") { - const drafts = await ctx.runQuery( - internal.products.sync.listDraftAndroidProducts, - { projectId: project._id }, - ); - for (const row of drafts) { - try { - // When this row already has a storeRef from a prior partial - // sync, run the appropriate update endpoint instead of the - // create endpoint — Play returns 409 Conflict on - // create-with-existing-productId and ASC's parity step - // (asc.ts) does the same patch flow. Listings + price are - // both safe to re-push idempotently. Without this, kit-side - // edits made after the initial push would silently never - // reach Play (PR #124 + try { + let token: string | undefined; + let pageCount = 0; + do { + const subs = await androidpublisher.monetization.subscriptions.list({ + packageName, + ...(token ? { pageToken: token } : {}), + }); + for (const sub of subs.data.subscriptions ?? []) { + if (!sub.productId) continue; + const { priceAmountMicros, currency, basePlanId } = + pickSubBasePlanPrice(sub); + const offers = collectPlaySubscriptionOffers(sub); + // Pick the billingPeriod from the *same* base plan whose + // price we just selected (`basePlanId` returned by + // pickSubBasePlanPrice). If we can't find that exact plan + // in `offers`, fall back to the first BasePlan row — but + // this fallback only triggers when basePlanId is missing, + // which means the subscription has no price at all. + // Without the basePlanId match, mixed monthly + yearly + // products would pair the yearly USD price with the + // monthly duration and break MRR normalization. + const billingPeriod = ( + basePlanId + ? offers.find((o) => o.kind === "BasePlan" && o.id === basePlanId) + : offers.find((o) => o.kind === "BasePlan") + )?.duration; + await ctx.runMutation(internal.products.sync.upsertFromStore, { + projectId: project._id, + productId: sub.productId, + platform: "Android", + type: "Subscription", + title: sub.listings?.[0]?.title ?? sub.productId, + description: sub.listings?.[0]?.description ?? undefined, + priceAmountMicros, + currency, + storeRef: sub.productId, + state: "Active", + billingPeriod: coerceBillingPeriod(billingPeriod), + // Play has no first-class subscription "group" — base + // plans on a single subscription product play that role, + // and we surface them as `offers[].kind === "BasePlan"` + // rows. Leave the ASC-only group fields unset. + offers: offers.length ? offers : undefined, + }); + pulled += 1; + } + token = subs.data.nextPageToken ?? undefined; + pageCount += 1; + if (pageCount > 50) break; + } while (token); + } catch (error) { + failures.push({ + productId: "(play list subscriptions)", + reason: error instanceof Error ? error.message : String(error), + }); + } + } + + // ── PUSH: kit → Play for Draft rows ────────────────────────── + if (direction === "push" || direction === "both") { + await checkCancelled(); + await reportPhase("push-drafts", { + current: pulled, + failuresCount: failures.length, + }); + const drafts = await ctx.runQuery( + internal.products.sync.listDraftAndroidProducts, + { projectId: project._id }, + ); + for (const row of drafts) { + try { + // When this row already has a storeRef from a prior partial + // sync, run the appropriate update endpoint instead of the + // create endpoint — Play returns 409 Conflict on + // create-with-existing-productId and ASC's parity step + // (asc.ts) does the same patch flow. Listings + price are + // both safe to re-push idempotently. Without this, kit-side + // edits made after the initial push would silently never + // reach Play (PR #124 + // (https://github.com/hyodotdev/openiap/pull/124) review). + if (row.storeRef) { + // Track whether the patch step succeeded — only flip to + // Ready when the upstream actually accepted our changes, + // otherwise the row stays Draft and surfaces in the next + // sync's drafts list for retry (PR #124 // (https://github.com/hyodotdev/openiap/pull/124) review). - if (row.storeRef) { - // Track whether the patch step succeeded — only flip to - // Ready when the upstream actually accepted our changes, - // otherwise the row stays Draft and surfaces in the next - // sync's drafts list for retry (PR #124 - // (https://github.com/hyodotdev/openiap/pull/124) review). - let patchOk = true; - if (row.type === "Subscription") { - // Subscriptions: patch the listing via - // monetization.subscriptions.patch (en-US listing only — - // multi-language sync is a future feature). Base-plan - // price changes have to go through a separate - // monetization.subscriptions.basePlans endpoint, so we - // intentionally don't try to mutate price here; that - // requires a deactivate+recreate flow Play doesn't allow - // in a single call. The dashboard surfaces a hint when - // the kit-side row has a different price than the - // pulled row so the operator knows to do that step - // manually. + let patchOk = true; + if (row.type === "Subscription") { + // Subscriptions: patch the listing via + // monetization.subscriptions.patch (en-US listing only — + // multi-language sync is a future feature). Base-plan + // price changes have to go through a separate + // monetization.subscriptions.basePlans endpoint, so we + // intentionally don't try to mutate price here; that + // requires a deactivate+recreate flow Play doesn't allow + // in a single call. The dashboard surfaces a hint when + // the kit-side row has a different price than the + // pulled row so the operator knows to do that step + // manually. + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "patch subscription listing", + detail: `${row.title} (en-US, storeRef=${row.storeRef})`, + }); + } else { try { await androidpublisher.monetization.subscriptions.patch({ packageName, @@ -485,10 +601,22 @@ export const pushSyncProductsGoogle = action({ error instanceof Error ? error.message : String(error), }); } + } + } else { + // One-time product: patch listings + price via the + // legacy inappproducts.patch endpoint, which accepts a + // partial body and merges it. + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "patch in-app product", + detail: + `${row.title} (en-US, storeRef=${row.storeRef})` + + (row.priceAmountMicros !== undefined && row.currency + ? ` · ${row.currency} ${(row.priceAmountMicros / 1_000_000).toFixed(2)}` + : ""), + }); } else { - // One-time product: patch listings + price via the - // legacy inappproducts.patch endpoint, which accepts a - // partial body and merges it. try { await androidpublisher.inappproducts.patch({ packageName, @@ -522,30 +650,47 @@ export const pushSyncProductsGoogle = action({ }); } } - if (patchOk) { - await ctx.runMutation(internal.products.sync.markPushed, { - projectId: project._id, - productId: row.productId, - platform: "Android", - storeRef: row.storeRef, - }); - pushed += 1; - } - continue; } - if (row.type === "Subscription") { - // Reject subscription creates that would land on Play with - // no base plan: such a subscription is created in a draft - // state that the Play app cannot purchase, which silently - // breaks the SDK's `requestPurchase` flow downstream. The - // operator must provide both a price and currency at - // minimum so we can synthesize a base plan. - if (!row.priceAmountMicros || !row.currency) { - throw new Error( - "Subscription requires priceAmountMicros + currency to mint a Play base plan; otherwise the product will not be purchasable.", - ); - } - const basePlanId = basePlanIdForPeriod(row.billingPeriod); + if (patchOk && !dryRun) { + await ctx.runMutation(internal.products.sync.markPushed, { + projectId: project._id, + productId: row.productId, + platform: "Android", + storeRef: row.storeRef, + }); + pushed += 1; + } else if (patchOk && dryRun) { + pushed += 1; + } + continue; + } + if (row.type === "Subscription") { + // Reject subscription creates that would land on Play with + // no base plan: such a subscription is created in a draft + // state that the Play app cannot purchase, which silently + // breaks the SDK's `requestPurchase` flow downstream. The + // operator must provide both a price and currency at + // minimum so we can synthesize a base plan. Validation + // applies to dry-run too — we want the operator to see + // this error before attempting a real sync. + if (!row.priceAmountMicros || !row.currency) { + throw new Error( + "Subscription requires priceAmountMicros + currency to mint a Play base plan; otherwise the product will not be purchasable.", + ); + } + const basePlanId = basePlanIdForPeriod(row.billingPeriod); + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "create subscription", + detail: `${row.title} · base plan ${basePlanId} · ${row.billingPeriod ?? "P1M"} · ${row.currency} ${(row.priceAmountMicros / 1_000_000).toFixed(2)} (US)`, + }); + plannedWrites.push({ + productId: row.productId, + step: "activate base plan", + detail: basePlanId, + }); + } else { await androidpublisher.monetization.subscriptions.create({ packageName, productId: row.productId, @@ -611,6 +756,18 @@ export const pushSyncProductsGoogle = action({ }, }, ); + } + } else { + if (dryRun) { + plannedWrites.push({ + productId: row.productId, + step: "create in-app product", + detail: + `${row.title} · ${row.type}` + + (row.priceAmountMicros !== undefined && row.currency + ? ` · ${row.currency} ${(row.priceAmountMicros / 1_000_000).toFixed(2)}` + : " · no price set"), + }); } else { await androidpublisher.inappproducts.insert({ packageName, @@ -642,6 +799,8 @@ export const pushSyncProductsGoogle = action({ }, }); } + } + if (!dryRun) { // Persist storeRef immediately after the create returns, // BEFORE flipping state to Ready via markPushed. If the // action times out / crashes between create and markPushed, @@ -662,19 +821,24 @@ export const pushSyncProductsGoogle = action({ platform: "Android", storeRef: row.productId, }); - pushed += 1; - } catch (error) { - failures.push({ - productId: row.productId, - reason: error instanceof Error ? error.message : String(error), - }); } + pushed += 1; + } catch (error) { + failures.push({ + productId: row.productId, + reason: error instanceof Error ? error.message : String(error), + }); } } + } - return { pulled, pushed, failures }; - }, -}); + return { + pulled, + pushed, + failures, + plannedWrites: dryRun ? plannedWrites : undefined, + }; +} function mapPlayOneTimeType( product: androidpublisher_v3.Schema$InAppProduct, diff --git a/packages/kit/convex/products/sync.ts b/packages/kit/convex/products/sync.ts index 30501863..06c37886 100644 --- a/packages/kit/convex/products/sync.ts +++ b/packages/kit/convex/products/sync.ts @@ -346,3 +346,34 @@ export const listDraftAndroidProducts = internalQuery({ })); }, }); + +// Bounded delete used by the `purge-local` sync direction. Deletes +// the project's kit-side product rows for one platform and returns +// `{ deleted, hasMore }` so the worker can loop until empty without +// blowing past Convex's per-mutation document budget. Does NOT touch +// App Store Connect / Play Console — purging upstream is intentionally +// out of scope (Apple can't fully delete IAPs via API; Play archive +// risks live-billing breakage). The operator does that in the +// store's web console if they really need it. +export const deletePlatformCatalog = internalMutation({ + args: { + projectId: v.id("projects"), + platform: platformValidator, + limit: v.number(), + }, + returns: v.object({ deleted: v.number(), hasMore: v.boolean() }), + handler: async (ctx, args) => { + const page = await ctx.db + .query("products") + .withIndex("by_project_and_platform", (q) => + q.eq("projectId", args.projectId).eq("platform", args.platform), + ) + .take(args.limit + 1); + const hasMore = page.length > args.limit; + const toDelete = hasMore ? page.slice(0, args.limit) : page; + for (const row of toDelete) { + await ctx.db.delete(row._id); + } + return { deleted: toDelete.length, hasMore }; + }, +}); diff --git a/packages/kit/convex/projects/internal.ts b/packages/kit/convex/projects/internal.ts index 3777e76a..d8a02e22 100644 --- a/packages/kit/convex/projects/internal.ts +++ b/packages/kit/convex/projects/internal.ts @@ -3,7 +3,7 @@ import { v } from "convex/values"; import { internalQuery } from "../_generated/server"; import { Doc } from "../_generated/dataModel"; import { getApiKeyByKey } from "../apiKeys/helpers"; -import { getProjectById } from "./helpers"; +import { getProjectById as getProjectByIdFromDb } from "./helpers"; export const getProjectByApiKey = internalQuery({ args: { @@ -19,7 +19,7 @@ export const getProjectByApiKey = internalQuery({ if (apiKey.isActive === false) { return null; } - return getProjectById(ctx, apiKey.projectId); + return getProjectByIdFromDb(ctx, apiKey.projectId); } // Legacy fallback: early projects — or anything created before the @@ -35,3 +35,14 @@ export const getProjectByApiKey = internalQuery({ return legacyProject ?? null; }, }); + +// Direct id-based lookup. Used by the product-sync workers, which +// resolve the project from a job row's `projectId` instead of the +// apiKey path so the worker can run without re-entering the api-key +// table on every poll. +export const getProjectById = internalQuery({ + args: { projectId: v.id("projects") }, + handler: async (ctx, args): Promise | null> => { + return await ctx.db.get(args.projectId); + }, +}); diff --git a/packages/kit/convex/schema.ts b/packages/kit/convex/schema.ts index f576aaa4..c6aa3dce 100644 --- a/packages/kit/convex/schema.ts +++ b/packages/kit/convex/schema.ts @@ -834,6 +834,86 @@ const schema = defineSchema({ "productId", ]) .index("by_project_and_platform", ["projectId", "platform"]), + + // Async job rows for the product-sync workers + // (`runProductSyncIOS` / `runProductSyncAndroid`). Replaces the + // original synchronous `pushSyncProductsAppleIOS` / `pushSyncProductsGoogle` + // public actions — those held the browser fetch open for the entire + // sync (minutes for catalogs >30 SKUs), so iOS Safari aborted with + // `TypeError: Load failed` on cellular / backgrounded tabs. The + // dashboard now enqueues a job (returns immediately) and subscribes + // via `useQuery(getActiveSyncJob)`; the worker runs as a scheduled + // internalAction and writes progress / result back to this row. + // + // Retention: succeeded rows pruned after 7d, failed after 30d + // (so operators still have a record after the weekend). + // Reaper: `reapStaleProductSyncJobs` cron flips running rows past + // `expectedDeadline` to failed("worker timed out") so a crashed + // action can't pin the project's "active job" slot forever. + productSyncJobs: defineTable({ + projectId: v.id("projects"), + platform: v.union(v.literal("IOS"), v.literal("Android")), + direction: v.union( + v.literal("pull"), + v.literal("push"), + v.literal("both"), + // Empty kit's local catalog for the platform without touching + // the upstream store. The next regular sync re-pulls, so this + // is the "blow away the cache" affordance used to recover from + // stale state after manual store edits or a bad config push. + v.literal("purge-local"), + ), + dryRun: v.boolean(), + status: v.union( + v.literal("queued"), + v.literal("running"), + v.literal("succeeded"), + v.literal("failed"), + ), + progress: v.object({ + phase: v.string(), + current: v.optional(v.number()), + total: v.optional(v.number()), + failuresCount: v.optional(v.number()), + }), + result: v.optional( + v.object({ + pulled: v.number(), + pushed: v.number(), + // Number of kit-side rows the purge-local direction removed. + // Always 0 (or unset) for pull/push/both directions. + deleted: v.optional(v.number()), + failures: v.array( + v.object({ productId: v.string(), reason: v.string() }), + ), + failuresTruncated: v.optional(v.boolean()), + plannedWrites: v.optional( + v.array( + v.object({ + productId: v.string(), + step: v.string(), + detail: v.optional(v.string()), + }), + ), + ), + }), + ), + error: v.optional(v.string()), + cancelRequested: v.optional(v.boolean()), + expectedDeadline: v.optional(v.number()), + createdBy: v.optional(v.id("users")), + startedAt: v.optional(v.number()), + completedAt: v.optional(v.number()), + createdAt: v.number(), + }) + // Active job lookup per (project, platform, status). Backs the + // dashboard's `getActiveSyncJob` subscription and the + // double-enqueue guard inside `enqueueProductSync`. + .index("by_project_platform_status", ["projectId", "platform", "status"]) + .index("by_project_and_created", ["projectId", "createdAt"]) + // Reaper / pruner scans. + .index("by_status_and_deadline", ["status", "expectedDeadline"]) + .index("by_status_and_completed", ["status", "completedAt"]), }); export default schema; diff --git a/packages/kit/server/api/v1/products.ts b/packages/kit/server/api/v1/products.ts index 717d264a..c613be94 100644 --- a/packages/kit/server/api/v1/products.ts +++ b/packages/kit/server/api/v1/products.ts @@ -1,6 +1,7 @@ import { Hono } from "hono"; import { api } from "@/convex"; +import type { Id } from "@/convex"; import { client } from "../../convex"; // Catalog read/write surface mirroring onesub's @onesub/providers @@ -147,13 +148,20 @@ products.post("/:apiKey/state", async (c) => { } }); +// Enqueue an async sync job. Returns `{ jobId, deduped }` +// immediately. The caller polls `GET .../sync/jobs/:jobId` until +// `status` is `succeeded` or `failed`. The previous synchronous +// endpoint held the HTTP connection open for the entire sync, which +// iOS Safari aborted on cellular / backgrounded tabs as +// `TypeError: Load failed`. products.post("/:apiKey/sync/:platform", async (c) => { const apiKey = c.req.param("apiKey"); - const platform = c.req.param("platform"); + const platformParam = c.req.param("platform"); const direction = (c.req.query("direction") as "pull" | "push" | "both" | undefined) ?? "both"; - if (platform !== "ios" && platform !== "android") { + const dryRun = c.req.query("dryRun") === "true"; + if (platformParam !== "ios" && platformParam !== "android") { return c.json( { errors: [ @@ -163,24 +171,78 @@ products.post("/:apiKey/sync/:platform", async (c) => { 400, ); } + const platform: "IOS" | "Android" = + platformParam === "ios" ? "IOS" : "Android"; try { - const result = - platform === "ios" - ? await client.action(api.products.asc.pushSyncProductsAppleIOS, { - apiKey, - direction, - }) - : await client.action(api.products.play.pushSyncProductsGoogle, { - apiKey, - direction, - }); + const result = await client.mutation(api.products.jobs.enqueueProductSync, { + apiKey, + platform, + direction, + dryRun, + }); + return c.json(result, 202); + } catch (error) { + return c.json( + { + errors: [ + { + code: "PRODUCT_SYNC_ENQUEUE_FAILED", + message: error instanceof Error ? error.message : String(error), + }, + ], + }, + 400, + ); + } +}); + +// Poll the job state. Clients should backoff (e.g. 3s) between +// polls; the typical sync finishes in tens of seconds, larger +// catalogs in 1-2 min. SSE is a future option; polling kept simple +// for v1. +products.get("/:apiKey/sync/jobs/:jobId", async (c) => { + const jobId = c.req.param("jobId"); + try { + const job = await client.query(api.products.jobs.getSyncJobById, { + jobId: jobId as Id<"productSyncJobs">, + }); + if (!job) { + return c.json( + { errors: [{ code: "NOT_FOUND", message: "Sync job not found" }] }, + 404, + ); + } + return c.json(job); + } catch (error) { + return c.json( + { + errors: [ + { + code: "PRODUCT_SYNC_LOOKUP_FAILED", + message: error instanceof Error ? error.message : String(error), + }, + ], + }, + 400, + ); + } +}); + +// Operator-initiated cancel. The worker checks `cancelRequested` +// at phase boundaries. +products.post("/:apiKey/sync/jobs/:jobId/cancel", async (c) => { + const jobId = c.req.param("jobId"); + try { + const result = await client.mutation(api.products.jobs.cancelProductSync, { + jobId: jobId as Id<"productSyncJobs">, + }); return c.json(result); } catch (error) { return c.json( { errors: [ { - code: "PRODUCT_SYNC_FAILED", + code: "PRODUCT_SYNC_CANCEL_FAILED", message: error instanceof Error ? error.message : String(error), }, ], diff --git a/packages/kit/src/components/AuthTransition.tsx b/packages/kit/src/components/AuthTransition.tsx index a6460d80..ca068dff 100644 --- a/packages/kit/src/components/AuthTransition.tsx +++ b/packages/kit/src/components/AuthTransition.tsx @@ -82,7 +82,7 @@ export function AuthTransition() { - + @@ -93,7 +93,7 @@ export function AuthTransition() { - + ); diff --git a/packages/kit/src/components/Tooltip.tsx b/packages/kit/src/components/Tooltip.tsx new file mode 100644 index 00000000..82e9077b --- /dev/null +++ b/packages/kit/src/components/Tooltip.tsx @@ -0,0 +1,96 @@ +import { ReactNode, useState } from "react"; +import { cn } from "@/lib/utils"; + +type Side = "top" | "bottom" | "left" | "right"; +type Align = "start" | "center" | "end"; + +// Lightweight hover/focus tooltip. Self-contained — no Radix / +// floating-ui dep — because the only thing we need is a positioned +// popover that opens on `mouseenter` / `focusin` and closes on +// `mouseleave` / `focusout`. Anchors against the trigger element via +// the wrapping `relative` div, so the trigger has to be a single +// inline-block element (a button, a span); avoid wrapping block +// elements that fight with `absolute` positioning. +// +// Use a real component (not a `title=` attribute or inline JSX) so +// the hover affordance is consistent across the app: same delay, same +// 320px max-width, same border/padding tokens. +export function Tooltip({ + children, + content, + side = "bottom", + align = "end", + widthClass = "w-80", +}: { + children: ReactNode; + content: ReactNode; + side?: Side; + align?: Align; + widthClass?: string; +}) { + const [open, setOpen] = useState(false); + return ( +
setOpen(true)} + onMouseLeave={() => setOpen(false)} + onFocus={() => setOpen(true)} + onBlur={() => setOpen(false)} + > + {children} + {open ? ( +
+ {content} +
+ ) : null} +
+ ); +} + +function sidePosition(side: Side): string { + switch (side) { + case "top": + return "bottom-full mb-2"; + case "bottom": + return "top-full mt-2"; + case "left": + return "right-full mr-2"; + case "right": + return "left-full ml-2"; + } +} + +function alignPosition(side: Side, align: Align): string { + if (side === "top" || side === "bottom") { + switch (align) { + case "start": + return "left-0"; + case "center": + return "left-1/2 -translate-x-1/2"; + case "end": + return "right-0"; + } + } + switch (align) { + case "start": + return "top-0"; + case "center": + return "top-1/2 -translate-y-1/2"; + case "end": + return "bottom-0"; + } +} diff --git a/packages/kit/src/lib/sentry.ts b/packages/kit/src/lib/sentry.ts index 5d344a53..e3acc385 100644 --- a/packages/kit/src/lib/sentry.ts +++ b/packages/kit/src/lib/sentry.ts @@ -40,6 +40,30 @@ if (typeof dsn === "string" && dsn.length > 0 && onAllowedHost) { sendDefaultPii: true, environment: mode, tracesSampleRate, + // The Convex client emits its own promise rejection when the + // websocket / fetch backing a query/action layer transiently + // fails (browser sleep, network change, server redeploy). The + // user-facing path catches these inline (toast, retry), but the + // internal rejection still surfaces to `window.onunhandledrejection` + // and floods Sentry as `TypeError: Load failed` noise that drowns + // out real bugs. Tag those events so triage can filter them out + // (or downsample) instead of treating each as a fresh signal. + beforeSend: (event, hint) => { + const exception = hint?.originalException; + const message = + (exception instanceof Error ? exception.message : String(exception)) ?? + ""; + const isFetchLoadFailed = + /Load failed|Failed to fetch|NetworkError/i.test(message); + const looksConvex = /convex\.cloud|\/api\/(action|query|mutation)/i.test( + message + " " + (event.request?.url ?? ""), + ); + if (isFetchLoadFailed && looksConvex) { + event.tags = { ...(event.tags ?? {}), source: "convex-reconnect" }; + event.fingerprint = ["convex-reconnect-load-failed"]; + } + return event; + }, }); } diff --git a/packages/kit/src/pages/auth/organization/project/products.tsx b/packages/kit/src/pages/auth/organization/project/products.tsx index a933b044..2955e3b0 100644 --- a/packages/kit/src/pages/auth/organization/project/products.tsx +++ b/packages/kit/src/pages/auth/organization/project/products.tsx @@ -1,15 +1,28 @@ -import { useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useOutletContext } from "react-router-dom"; import { toast } from "sonner"; import { useMutation, useQuery, useAction } from "convex/react"; -import { Layers, Loader2, Plus, RefreshCw, ChevronDown } from "lucide-react"; +import { + Layers, + Loader2, + Plus, + RefreshCw, + ChevronDown, + X, + Trash2, + AlertTriangle, + Info, + ExternalLink, +} from "lucide-react"; import type { Doc } from "@/convex"; import { api } from "@/convex"; import { PageLoading } from "@/components/LoadingSpinner"; +import { Tooltip } from "@/components/Tooltip"; import { Badge, PlatformBadge } from "../../../../components/Badge"; type ProjectContext = { project: Doc<"projects"> }; +type SyncJob = Doc<"productSyncJobs">; export default function ProjectProducts() { const { project } = useOutletContext(); @@ -17,8 +30,17 @@ export default function ProjectProducts() { apiKey: project.apiKey, }); const upsert = useMutation(api.products.mutation.upsertProduct); - const syncApple = useAction(api.products.asc.pushSyncProductsAppleIOS); - const syncGoogle = useAction(api.products.play.pushSyncProductsGoogle); + const enqueueSync = useMutation(api.products.jobs.enqueueProductSync); + const cancelSync = useMutation(api.products.jobs.cancelProductSync); + const dismissJob = useMutation(api.products.jobs.dismissCompletedJob); + const iosJob = useQuery(api.products.jobs.getActiveSyncJob, { + apiKey: project.apiKey, + platform: "IOS", + }); + const androidJob = useQuery(api.products.jobs.getActiveSyncJob, { + apiKey: project.apiKey, + platform: "Android", + }); const listAscGroups = useAction( api.products.asc.listSubscriptionGroupsAppleIOS, ); @@ -29,12 +51,16 @@ export default function ProjectProducts() { const [ascGroupNames, setAscGroupNames] = useState(null); const [ascGroupLoadFailed, setAscGroupLoadFailed] = useState(false); - // Per-platform sync state — the button shows a spinner instead of a - // separate page-level banner so the affordance lives next to the - // action the operator triggered. - const [syncingPlatform, setSyncingPlatform] = useState< - "IOS" | "Android" | null - >(null); + // Sync state now lives in `productSyncJobs` and is read reactively + // via `getActiveSyncJob` per platform — the worker writes progress + // back to the row, so the dashboard re-renders without polling. The + // local `lastShownJobId*` refs gate the completion toast so a + // succeeded job only toasts once even if the row updates again + // (e.g. on dismiss). + const lastShownJobIdRef = useRef>({ + IOS: null, + Android: null, + }); // The draft form holds every field the push-sync flow consumes. // Optional fields are stored as empty strings here and converted to // `undefined` on submit so an unfilled price doesn't end up @@ -59,6 +85,57 @@ export default function ProjectProducts() { }; }, [products]); + // Show success/failure toast exactly once when a job transitions to + // a terminal state. The lastShownJobIdRef guard prevents the toast + // from re-firing on subsequent reactive updates of the same row. + useEffect(() => { + for (const platform of ["IOS", "Android"] as const) { + const job = platform === "IOS" ? iosJob : androidJob; + if (!job) continue; + const terminal = job.status === "succeeded" || job.status === "failed"; + if (!terminal) continue; + if (lastShownJobIdRef.current[platform] === job._id) continue; + lastShownJobIdRef.current[platform] = job._id; + const label = platform === "IOS" ? "App Store Connect" : "Play Console"; + const result = job.result; + if (job.status === "succeeded" && result) { + const summary = + result.deleted !== undefined + ? `Deleted ${result.deleted} row${result.deleted === 1 ? "" : "s"}` + : `Pulled ${result.pulled}, pushed ${result.pushed}`; + const plannedLines = result.plannedWrites?.length + ? result.plannedWrites + .map( + (p) => + `${p.productId} → ${p.step}${p.detail ? ": " + p.detail : ""}`, + ) + .join("\n") + : undefined; + if (result.failures.length) { + toast.error(`${label} sync — ${summary}`, { + description: + (plannedLines ? `Planned writes:\n${plannedLines}\n\n` : "") + + result.failures + .map((f) => `${f.productId}: ${f.reason}`) + .join("\n"), + duration: 12_000, + }); + } else if (plannedLines) { + toast.success(`${label} dry-run — ${summary} (no writes performed)`, { + description: plannedLines, + duration: 12_000, + }); + } else { + toast.success(`${label} sync — ${summary}`); + } + } else if (job.status === "failed") { + toast.error(`${label} sync failed: ${job.error ?? "Unknown error"}`, { + duration: 12_000, + }); + } + } + }, [iosJob, androidJob]); + if (products === undefined) { return ; } @@ -112,89 +189,70 @@ export default function ProjectProducts() { platform: "IOS" | "Android", options?: { dryRun?: boolean }, ) => { - if (syncingPlatform) return; - setSyncingPlatform(platform); + const activeForPlatform = platform === "IOS" ? iosJob : androidJob; + const isActive = + activeForPlatform?.status === "queued" || + activeForPlatform?.status === "running"; + if (isActive) return; const label = platform === "IOS" ? "App Store Connect" : "Play Console"; const dryRun = options?.dryRun === true; try { - // Dry-run is iOS-only for now — Android push doesn't have an - // equivalent skip-writes path yet (we'd need to mirror the same - // gating across play.ts). When operator hits "Dry-run" on - // Android we just route to a regular sync and surface a notice - // explaining the limitation. - if (dryRun && platform === "Android") { - toast.message( - "Dry-run not yet supported for Play Console — running real sync instead.", - { duration: 6_000 }, - ); - } - const result = - platform === "IOS" - ? await syncApple({ - apiKey: project.apiKey, - direction: "both", - ...(dryRun ? { dryRun: true } : {}), - }) - : await syncGoogle({ - apiKey: project.apiKey, - direction: "both", - }); - const summary = `Pulled ${result.pulled}, pushed ${result.pushed}`; - const planned: - | Array<{ - productId: string; - step: string; - detail?: string; - }> - | undefined = - "plannedWrites" in result && - Array.isArray((result as { plannedWrites?: unknown }).plannedWrites) - ? ( - result as { - plannedWrites: Array<{ - productId: string; - step: string; - detail?: string; - }>; - } - ).plannedWrites - : undefined; - const plannedLines = planned?.length - ? planned - .map( - (p) => - `${p.productId} → ${p.step}${p.detail ? ": " + p.detail : ""}`, - ) - .join("\n") - : undefined; - if (result.failures.length) { - const failureLines = result.failures - .map((f) => `${f.productId}: ${f.reason}`) - .join("\n"); - toast.error( - `${label} ${dryRun && platform === "IOS" ? "dry-run" : "sync"} — ${summary}`, - { - description: - (plannedLines ? `Planned writes:\n${plannedLines}\n\n` : "") + - failureLines, - duration: 12_000, - }, - ); - } else if (plannedLines) { - toast.success(`${label} dry-run — ${summary} (no writes performed)`, { - description: plannedLines, - duration: 12_000, - }); + const { deduped } = await enqueueSync({ + apiKey: project.apiKey, + platform, + direction: "both", + ...(dryRun ? { dryRun: true } : {}), + }); + if (deduped) { + toast.message(`${label} sync already running`, { duration: 4_000 }); } else { - toast.success(`${label} sync — ${summary}`); + toast.message(`${label} sync queued`, { duration: 3_000 }); } } catch (error) { toast.error( `${label} sync failed: ${error instanceof Error ? error.message : String(error)}`, { duration: 12_000 }, ); - } finally { - setSyncingPlatform(null); + } + }; + + const onPurge = async (platform: "IOS" | "Android") => { + const activeForPlatform = platform === "IOS" ? iosJob : androidJob; + const isActive = + activeForPlatform?.status === "queued" || + activeForPlatform?.status === "running"; + if (isActive) return; + const label = platform === "IOS" ? "App Store Connect" : "Play Console"; + try { + const { deduped } = await enqueueSync({ + apiKey: project.apiKey, + platform, + direction: "purge-local", + }); + if (deduped) { + toast.message(`${label} reset already running`, { duration: 4_000 }); + } else { + toast.message(`${label} catalog reset queued`, { duration: 3_000 }); + } + } catch (error) { + toast.error( + `${label} reset failed: ${error instanceof Error ? error.message : String(error)}`, + { duration: 12_000 }, + ); + } + }; + + const onCancel = async (jobId: SyncJob["_id"], label: string) => { + try { + await cancelSync({ jobId }); + toast.message(`${label} sync — cancellation requested`, { + duration: 4_000, + }); + } catch (error) { + toast.error( + `Cancel failed: ${error instanceof Error ? error.message : String(error)}`, + { duration: 8_000 }, + ); } }; @@ -213,6 +271,8 @@ export default function ProjectProducts() {

+ {project.horizonEnabled ? : null} +
@@ -381,21 +441,42 @@ export default function ProjectProducts() { { void onSync("IOS"); }} onDryRun={() => { void onSync("IOS", { dryRun: true }); }} + onPurge={() => { + void onPurge("IOS"); + }} + onCancel={(jobId) => { + void onCancel(jobId, "App Store Connect"); + }} + onDismiss={(jobId) => { + void dismissJob({ jobId }); + }} /> { void onSync("Android"); }} + onDryRun={() => { + void onSync("Android", { dryRun: true }); + }} + onPurge={() => { + void onPurge("Android"); + }} + onCancel={(jobId) => { + void onCancel(jobId, "Play Console"); + }} + onDismiss={(jobId) => { + void dismissJob({ jobId }); + }} />
); @@ -427,6 +508,37 @@ type ProductRow = { updatedAt: number; }; +// Map a job's `progress.phase` to the button label. Adds counts for +// the pull phases when available so the operator sees forward motion +// instead of a static "Syncing…" while the worker walks dozens of +// products. +function formatJobPhaseLabel(job: SyncJob | null, storeLabel: string): string { + if (!job) return `Syncing with ${storeLabel}…`; + const phase = job.progress.phase; + const current = job.progress.current; + switch (phase) { + case "queued": + case "starting": + return `Queued for ${storeLabel}…`; + case "pull-iaps": + return `Pulling IAPs from ${storeLabel}…`; + case "pull-subscriptions": + return `Pulling subscriptions${ + current !== undefined ? ` (${current})` : "" + }…`; + case "pull-products": + return `Pulling from ${storeLabel}…`; + case "push-drafts": + return `Pushing drafts to ${storeLabel}…`; + case "purge-local": + return `Resetting kit catalog${ + current !== undefined ? ` (${current} deleted)` : "" + }…`; + default: + return `Syncing with ${storeLabel}…`; + } +} + // Render a human-readable label for a single offer row. The dashboard // shows these as small badges under the subscription title so the // operator can see at a glance which plans/intros a sub carries @@ -528,19 +640,245 @@ function groupRowsByHierarchy( return out; } +// Dry-run skips every POST/PATCH against the upstream store and +// returns a `plannedWrites` list the toast/banner renders so the +// operator can verify group, price tier, base plan, billing +// period, etc. before committing. Apple's ASC catalog can't be +// fully deleted (Removed != gone from catalog); Play archive is +// reversible but still affects live billing. Either way, previewing +// is the safer first step. +function DryRunButton({ + onDryRun, + disabled, + platform, +}: { + onDryRun: () => void; + disabled: boolean; + platform: "IOS" | "Android"; +}) { + const storeLabel = platform === "IOS" ? "App Store Connect" : "Play Console"; + const constraint = + platform === "IOS" + ? "Apple won't let you fully delete an IAP once it's in the catalog, so this is the safety check." + : "Play archive is reversible but still affects live billing, so this is the safety check."; + return ( + +
Read-only preview
+

+ Walks {storeLabel} like a real sync but{" "} + skips every write (no products created, no + listings, no price changes, no base plan activations). Returns a + list of writes the real run would have made so you can verify them + before committing — {constraint} +

+ + } + > + + + ); +} + +// Meta Horizon doesn't expose a catalog REST API — only +// `verify_entitlement` (purchase check) and `consume_entitlement` +// (consumable burn-down) are reachable from the server side. SKU +// definitions live exclusively in Meta Quest Developer Hub. We +// surface the constraint here so a Horizon-enabled project's +// operator doesn't keep looking for a missing "Sync with Meta" +// button — kit handles entitlements (receipt verification + +// 6-hour reconciliation cron) but cannot mirror the catalog. +function HorizonCatalogNotice() { + return ( +
+ +
+
Horizon catalog is upstream-only
+

+ Meta doesn't expose a catalog API — manage Quest / Horizon SKUs + in Meta Quest Developer Hub. kit verifies Horizon receipts and + reconciles subscription entitlements every 6 hours, but the SKU list + itself can't be synced. +

+ + Open Meta Quest Developer Hub + + +
+
+ ); +} + +// Trigger for the local-purge confirm dialog. Tooltip explains the +// scope (kit cache only, never store) so the operator isn't scared +// off by the destructive icon. Disabled state covers both "sync in +// progress" (would race with the worker) and "nothing to purge". +function ResetCatalogButton({ + onClick, + disabled, +}: { + onClick: () => void; + disabled: boolean; +}) { + return ( + +
Reset local catalog
+

+ Deletes kit's local rows for this platform.{" "} + Does not modify App Store Connect / Play Console.{" "} + Use this when kit's cache has drifted from the store and you + want a clean re-pull. Run Sync after to re-import. +

+ + } + > + + + ); +} + +// Lightweight confirm dialog. Inline (not a shared ) because +// (a) it's the only confirm dialog in this page and (b) the +// AuthModal helper isn't exported from `components/`. The warning +// list here intentionally calls out the two real risks of +// purge-then-sync: unpushed local edits get overwritten on re-pull, +// and kit-only Draft rows that never made it to the store +// disappear permanently. +function PurgeConfirmDialog({ + open, + platform, + rowCount, + onClose, + onConfirm, +}: { + open: boolean; + platform: "IOS" | "Android"; + rowCount: number; + onClose: () => void; + onConfirm: () => void; +}) { + useEffect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") onClose(); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [open, onClose]); + if (!open) return null; + const storeLabel = platform === "IOS" ? "App Store Connect" : "Play Console"; + return ( +
+
+
+
+
+
+ +
+
+
+ Reset local {storeLabel} catalog? +
+

+ Deletes all {rowCount} kit-side{" "} + {platform === "IOS" ? "iOS" : "Android"} row + {rowCount === 1 ? "" : "s"}. {storeLabel} itself is not + modified. +

+
+
+
+
+ +

+ Local edits not yet pushed to {storeLabel} (price changes, + review notes, titles) will be lost when the + next Sync re-pulls the upstream copy. +

+
+
+ +

+ Draft rows that were never pushed upstream will be{" "} + permanently deleted — they don't exist on{" "} + {storeLabel} so Sync can't recover them. +

+
+
+
+ + +
+
+
+
+ ); +} + function ProductGroup({ platform, rows, - syncing, + job, onSync, onDryRun, + onPurge, + onCancel, + onDismiss, }: { platform: "IOS" | "Android"; rows: Array; - syncing: boolean; + job: SyncJob | null; onSync: () => void; onDryRun?: () => void; + onPurge: () => void; + onCancel: (jobId: SyncJob["_id"]) => void; + onDismiss: (jobId: SyncJob["_id"]) => void; }) { + const storeLabel = platform === "IOS" ? "App Store Connect" : "Play Console"; + const isActive = job?.status === "queued" || job?.status === "running"; + const isTerminal = job?.status === "succeeded" || job?.status === "failed"; + const dismissed = job?.progress.phase === "dismissed"; + const showResult = isTerminal && !dismissed; + const [purgeOpen, setPurgeOpen] = useState(false); return (
@@ -552,31 +890,89 @@ function ProductGroup({
{onDryRun && ( + + )} + setPurgeOpen(true)} + /> + {isActive && job ? ( - )} + ) : null}
+ setPurgeOpen(false)} + onConfirm={() => { + setPurgeOpen(false); + onPurge(); + }} + /> + {showResult && job ? ( +
+
+ {job.status === "succeeded" && job.result ? ( +
+ {job.result.deleted !== undefined + ? `Reset — deleted ${job.result.deleted} row${ + job.result.deleted === 1 ? "" : "s" + }` + : `Last sync — pulled ${job.result.pulled}, pushed ${job.result.pushed}`} + {job.result.failures.length + ? `, ${job.result.failures.length} failure${ + job.result.failures.length === 1 ? "" : "s" + }` + : ""} + {job.result.failuresTruncated ? " (truncated)" : ""} +
+ ) : ( +
Last sync failed — {job.error ?? "Unknown error"}
+ )} +
+ +
+ ) : null} From acaaa8e8f2e97b1d6cd6bc0f6317014dc7500340 Mon Sep 17 00:00:00 2001 From: hyochan Date: Tue, 5 May 2026 19:02:13 +0900 Subject: [PATCH 2/3] fix(kit): address Copilot/Gemini/CodeRabbit review on async product sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/kit/convex/files/internal.ts | 17 +- packages/kit/convex/products/asc.ts | 47 +++--- packages/kit/convex/products/jobs.ts | 153 +++++++++++------- packages/kit/convex/products/play.ts | 46 +++--- packages/kit/convex/products/sync.ts | 9 ++ packages/kit/convex/schema.ts | 11 ++ packages/kit/server/api/v1/products.ts | 12 +- packages/kit/src/lib/sentry.ts | 20 ++- .../auth/organization/project/products.tsx | 14 +- 9 files changed, 219 insertions(+), 110 deletions(-) diff --git a/packages/kit/convex/files/internal.ts b/packages/kit/convex/files/internal.ts index 40ba9dac..7e28e5b3 100644 --- a/packages/kit/convex/files/internal.ts +++ b/packages/kit/convex/files/internal.ts @@ -186,18 +186,23 @@ export const readFileAsBase64 = internalAction({ // the dashboard's "download .p8" failing). Encode via `btoa` on // chunked binary strings so the path stays portable to either // runtime; chunking keeps the call-stack bound below the - // String.fromCharCode argument limit for files of any size. + // `String.fromCharCode` argument limit for files of any size, + // and accumulating the chunks in an array before `join("")` + // avoids the O(n²) string-concatenation behavior of `binary +=` + // on multi-megabyte uploads (Copilot review on PR #127). const arrayBuffer = await blob.arrayBuffer(); const bytes = new Uint8Array(arrayBuffer); const CHUNK = 0x8000; - let binary = ""; + const chunks: string[] = []; for (let i = 0; i < bytes.length; i += CHUNK) { - binary += String.fromCharCode.apply( - null, - bytes.subarray(i, i + CHUNK) as unknown as number[], + chunks.push( + String.fromCharCode.apply( + null, + bytes.subarray(i, i + CHUNK) as unknown as number[], + ), ); } - const base64 = btoa(binary); + const base64 = btoa(chunks.join("")); return { fileId: file._id, diff --git a/packages/kit/convex/products/asc.ts b/packages/kit/convex/products/asc.ts index 06cdc532..a1f47782 100644 --- a/packages/kit/convex/products/asc.ts +++ b/packages/kit/convex/products/asc.ts @@ -1010,28 +1010,39 @@ export const runProductSyncIOS = internalAction({ }, }); -async function performIosSync( - ctx: ActionCtx, - options: { - projectId: import("../_generated/dataModel").Id<"projects">; - direction: "pull" | "push" | "both"; - dryRun: boolean; - checkCancelled: () => Promise; - reportPhase: ( - phase: string, - extra?: { - current?: number; - total?: number; - failuresCount?: number; - }, - ) => Promise; - }, -): Promise<{ +// Shared shape for the per-phase progress callback the worker +// passes into both `performIosSync` and `performAndroidSync`. Pulled +// out so the two function signatures stay readable when extended +// (Gemini review on PR #127). +interface SyncProgressUpdate { + current?: number; + total?: number; + failuresCount?: number; +} +type SyncProgressReporter = ( + phase: string, + extra?: SyncProgressUpdate, +) => Promise; + +interface IosSyncOptions { + projectId: import("../_generated/dataModel").Id<"projects">; + direction: "pull" | "push" | "both"; + dryRun: boolean; + checkCancelled: () => Promise; + reportPhase: SyncProgressReporter; +} + +interface SyncResult { pulled: number; pushed: number; failures: Array<{ productId: string; reason: string }>; plannedWrites?: Array<{ productId: string; step: string; detail?: string }>; -}> { +} + +async function performIosSync( + ctx: ActionCtx, + options: IosSyncOptions, +): Promise { const project = await ctx.runQuery( internal.projects.internal.getProjectById, { projectId: options.projectId }, diff --git a/packages/kit/convex/products/jobs.ts b/packages/kit/convex/products/jobs.ts index e5349a58..41336ed1 100644 --- a/packages/kit/convex/products/jobs.ts +++ b/packages/kit/convex/products/jobs.ts @@ -21,6 +21,13 @@ export const PRODUCT_SYNC_REAPER_GRACE_MS = 60 * 1_000; export const PRODUCT_SYNC_SUCCEEDED_RETENTION_MS = 7 * 24 * 60 * 60 * 1_000; export const PRODUCT_SYNC_FAILED_RETENTION_MS = 30 * 24 * 60 * 60 * 1_000; export const PRODUCT_SYNC_FAILURES_CAP = 200; +// Batch size for the reaper / pruner crons. Sized to comfortably +// fit Convex's per-mutation document budget — enough rows that one +// tick can clear a typical backlog, but not so many that a +// pathological deployment with thousands of stuck jobs blows past +// the budget mid-run. +export const PRODUCT_SYNC_REAPER_BATCH = 50; +export const PRODUCT_SYNC_PRUNER_BATCH = 100; // Cap the failures array stored on the job row so a runaway sync // (every product fails for the same upstream config reason) doesn't @@ -47,14 +54,19 @@ const directionValidator = v.union( v.literal("purge-local"), ); -async function requireProjectMember( +// Auth gate: presence of a valid `apiKey` is sufficient (matches the +// existing pattern in `products/mutation.ts upsertProduct` and the +// HTTP routes in `server/api/v1/products.ts`). The dashboard call +// path is also authenticated via Convex Auth — when a logged-in +// user makes the call we record their `userId` on `createdBy` for +// audit trail, but a missing user (server-to-server HTTP path) +// must NOT block the call. Earlier versions required +// `getAuthUserId` and broke the documented HTTP API entirely +// (Copilot review on PR #127). +async function resolveProjectByApiKey( ctx: QueryCtx, apiKey: string, -): Promise<{ project: Doc<"projects">; userId: Id<"users"> }> { - const userId = await getAuthUserId(ctx); - if (!userId) { - throw createError(ErrorCode.NOT_AUTHENTICATED); - } +): Promise<{ project: Doc<"projects">; userId: Id<"users"> | null }> { const project = await ctx.db .query("projects") .withIndex("by_api_key", (q) => q.eq("apiKey", apiKey)) @@ -62,44 +74,51 @@ async function requireProjectMember( if (!project) { throw createError(ErrorCode.PROJECT_NOT_FOUND); } - const membership = await ctx.db - .query("organizationMembers") - .withIndex("by_org_and_user", (q) => - q.eq("organizationId", project.organizationId).eq("userId", userId), - ) - .first(); - if (!membership) { - throw createError(ErrorCode.NOT_ORGANIZATION_MEMBER); + let userId: Id<"users"> | null = null; + try { + userId = await getAuthUserId(ctx); + } catch { + userId = null; + } + // Best-effort membership confirmation when a user IS present + // — refuse to honor a logged-in user calling another org's + // apiKey. Without a logged-in user we trust the apiKey as the + // sole credential (server-side caller, MCP tool, SDK). + if (userId) { + const membership = await ctx.db + .query("organizationMembers") + .withIndex("by_org_and_user", (q) => + q.eq("organizationId", project.organizationId).eq("userId", userId), + ) + .first(); + if (!membership) { + throw createError(ErrorCode.NOT_ORGANIZATION_MEMBER); + } } return { project, userId }; } -async function requireJobAccess( +// Authenticate `(apiKey, jobId)` together: resolve the project from +// the apiKey, then verify the job belongs to that project. This +// ensures the apiKey acts as a per-project capability — a stolen +// jobId from one project can't be cancelled / read by another +// project's apiKey. +async function resolveJobByApiKey( ctx: QueryCtx, + apiKey: string, jobId: Id<"productSyncJobs">, -): Promise<{ job: Doc<"productSyncJobs">; userId: Id<"users"> }> { - const userId = await getAuthUserId(ctx); - if (!userId) { - throw createError(ErrorCode.NOT_AUTHENTICATED); - } +): Promise<{ job: Doc<"productSyncJobs">; project: Doc<"projects"> }> { + const { project } = await resolveProjectByApiKey(ctx, apiKey); const job = await ctx.db.get(jobId); if (!job) { throw createError(ErrorCode.INVALID_INPUT, "Sync job not found"); } - const project = await ctx.db.get(job.projectId); - if (!project) { - throw createError(ErrorCode.PROJECT_NOT_FOUND); - } - const membership = await ctx.db - .query("organizationMembers") - .withIndex("by_org_and_user", (q) => - q.eq("organizationId", project.organizationId).eq("userId", userId), - ) - .first(); - if (!membership) { - throw createError(ErrorCode.NOT_ORGANIZATION_MEMBER); + if (job.projectId !== project._id) { + // Treat cross-project lookups as not-found rather than 403 to + // avoid leaking job existence across projects. + throw createError(ErrorCode.INVALID_INPUT, "Sync job not found"); } - return { job, userId }; + return { job, project }; } // Latest job (any status) for a project+platform — drives the @@ -110,22 +129,29 @@ export const getActiveSyncJob = query({ platform: platformValidator, }, handler: async (ctx, args) => { - const { project } = await requireProjectMember(ctx, args.apiKey); + const { project } = await resolveProjectByApiKey(ctx, args.apiKey); + // Composite index `by_project_platform_created` narrows the + // index range to just this (project, platform) — replaces the + // earlier `by_project_and_created` + in-memory `.filter()` + // which scanned every job for the project before discarding + // the wrong-platform rows (Gemini review). return await ctx.db .query("productSyncJobs") - .withIndex("by_project_and_created", (q) => - q.eq("projectId", project._id), + .withIndex("by_project_platform_created", (q) => + q.eq("projectId", project._id).eq("platform", args.platform), ) - .filter((q) => q.eq(q.field("platform"), args.platform)) .order("desc") .first(); }, }); export const getSyncJobById = query({ - args: { jobId: v.id("productSyncJobs") }, + args: { + apiKey: v.string(), + jobId: v.id("productSyncJobs"), + }, handler: async (ctx, args) => { - const { job } = await requireJobAccess(ctx, args.jobId); + const { job } = await resolveJobByApiKey(ctx, args.apiKey, args.jobId); return job; }, }); @@ -145,7 +171,7 @@ export const enqueueProductSync = mutation({ deduped: v.boolean(), }), handler: async (ctx, args) => { - const { project, userId } = await requireProjectMember(ctx, args.apiKey); + const { project, userId } = await resolveProjectByApiKey(ctx, args.apiKey); const existingActive = await ctx.db .query("productSyncJobs") .withIndex("by_project_platform_status", (q) => @@ -176,7 +202,7 @@ export const enqueueProductSync = mutation({ dryRun: args.dryRun ?? false, status: "queued", progress: { phase: "queued" }, - createdBy: userId, + ...(userId ? { createdBy: userId } : {}), createdAt: now, }); if (args.direction === "purge-local") { @@ -277,9 +303,12 @@ export const runProductSyncPurgeLocal = internalAction({ // phase boundaries — granularity is per-phase, not per-product, but // that's enough to stop a runaway sync within seconds on most paths. export const cancelProductSync = mutation({ - args: { jobId: v.id("productSyncJobs") }, + args: { + apiKey: v.string(), + jobId: v.id("productSyncJobs"), + }, handler: async (ctx, args) => { - const { job } = await requireJobAccess(ctx, args.jobId); + const { job } = await resolveJobByApiKey(ctx, args.apiKey, args.jobId); if (job.status !== "queued" && job.status !== "running") { return { ok: false, reason: "not active" as const }; } @@ -288,24 +317,23 @@ export const cancelProductSync = mutation({ }, }); -// Soft-dismiss a finished job from the dashboard. Doesn't delete the -// row (the pruner handles retention) — just makes a future -// `getActiveSyncJob` skip it. -// -// Implemented via a tombstone field instead of a status change so -// audit-style queries can still see succeeded/failed history. +// Soft-dismiss a finished job from the dashboard. Doesn't delete +// the row (the pruner handles retention) — sets +// `progress.phase = "dismissed"` so the dashboard's result-banner +// gate (`progress.phase === "dismissed"`, see products.tsx +// `` renderer) and the toast effect both hide the +// row. We avoid `cancelRequested` here so the worker's cancel +// semantics stay distinct from operator dismiss. export const dismissCompletedJob = mutation({ - args: { jobId: v.id("productSyncJobs") }, + args: { + apiKey: v.string(), + jobId: v.id("productSyncJobs"), + }, handler: async (ctx, args) => { - const { job } = await requireJobAccess(ctx, args.jobId); + const { job } = await resolveJobByApiKey(ctx, args.apiKey, args.jobId); if (job.status !== "succeeded" && job.status !== "failed") { return { ok: false as const }; } - // Reuse `cancelRequested` as a dismissal marker would muddle - // semantics; instead we shift `completedAt` into the past so the - // active-job query (ordered desc by createdAt) still sees newer - // jobs while the dashboard filter on the client treats this row - // as "dismissed" via `dismissed: true`. await ctx.db.patch(job._id, { progress: { ...job.progress, phase: "dismissed" }, }); @@ -423,7 +451,12 @@ export const markJobFailed = internalMutation({ error: args.error, progress: { phase: "failed", - failuresCount: failures.length, + // Match `markJobSucceeded`'s "true count" semantics — + // `progress.failuresCount` is the raw upstream count, not + // the truncated slice's length, so analytics readers see + // the same number on both terminal states (CodeRabbit + // review on PR #127). + failuresCount: rawFailures.length, }, result: { pulled: args.pulled ?? 0, @@ -447,7 +480,7 @@ export const reapStaleProductSyncJobs = internalMutation({ .withIndex("by_status_and_deadline", (q) => q.eq("status", "running").lt("expectedDeadline", cutoff), ) - .take(50); + .take(PRODUCT_SYNC_REAPER_BATCH); for (const job of stale) { await ctx.db.patch(job._id, { status: "failed", @@ -472,13 +505,13 @@ export const pruneProductSyncJobs = internalMutation({ .withIndex("by_status_and_completed", (q) => q.eq("status", "succeeded").lt("completedAt", succeededCutoff), ) - .take(100); + .take(PRODUCT_SYNC_PRUNER_BATCH); const failed = await ctx.db .query("productSyncJobs") .withIndex("by_status_and_completed", (q) => q.eq("status", "failed").lt("completedAt", failedCutoff), ) - .take(100); + .take(PRODUCT_SYNC_PRUNER_BATCH); for (const row of [...succeeded, ...failed]) { await ctx.db.delete(row._id); } diff --git a/packages/kit/convex/products/play.ts b/packages/kit/convex/products/play.ts index 5ff02560..00c117b0 100644 --- a/packages/kit/convex/products/play.ts +++ b/packages/kit/convex/products/play.ts @@ -135,28 +135,38 @@ export const runProductSyncAndroid = internalAction({ }, }); -async function performAndroidSync( - ctx: ActionCtx, - options: { - projectId: import("../_generated/dataModel").Id<"projects">; - direction: "pull" | "push" | "both"; - dryRun: boolean; - checkCancelled: () => Promise; - reportPhase: ( - phase: string, - extra?: { - current?: number; - total?: number; - failuresCount?: number; - }, - ) => Promise; - }, -): Promise<{ +// See parallel definitions in `products/asc.ts`. Kept inline here +// (rather than imported across modules) because both files declare +// `"use node"` and Convex treats the module boundary as the runtime +// boundary; importing this from a non-`"use node"` module would +// pull `googleapis` into the V8 isolate runtime (Gemini review on +// PR #127). +interface AndroidSyncProgressUpdate { + current?: number; + total?: number; + failuresCount?: number; +} +interface AndroidSyncOptions { + projectId: import("../_generated/dataModel").Id<"projects">; + direction: "pull" | "push" | "both"; + dryRun: boolean; + checkCancelled: () => Promise; + reportPhase: ( + phase: string, + extra?: AndroidSyncProgressUpdate, + ) => Promise; +} +interface AndroidSyncResult { pulled: number; pushed: number; failures: ProductSyncFailure[]; plannedWrites?: Array<{ productId: string; step: string; detail?: string }>; -}> { +} + +async function performAndroidSync( + ctx: ActionCtx, + options: AndroidSyncOptions, +): Promise { const project = await ctx.runQuery( internal.projects.internal.getProjectById, { projectId: options.projectId }, diff --git a/packages/kit/convex/products/sync.ts b/packages/kit/convex/products/sync.ts index 06c37886..2264ee9d 100644 --- a/packages/kit/convex/products/sync.ts +++ b/packages/kit/convex/products/sync.ts @@ -363,6 +363,15 @@ export const deletePlatformCatalog = internalMutation({ }, returns: v.object({ deleted: v.number(), hasMore: v.boolean() }), handler: async (ctx, args) => { + // Guard against `limit <= 0` — Convex would happily run + // `.take(1)` (limit + 1 = 1) and the worker loop reads + // `hasMore` to keep iterating, which combined with `deleted = 0` + // traps the purge worker in a non-progressing loop until the + // 9-min reaper kills it (CodeRabbit critical finding on + // PR #127). + if (!Number.isInteger(args.limit) || args.limit < 1) { + throw new Error("limit must be a positive integer"); + } const page = await ctx.db .query("products") .withIndex("by_project_and_platform", (q) => diff --git a/packages/kit/convex/schema.ts b/packages/kit/convex/schema.ts index c6aa3dce..beff8890 100644 --- a/packages/kit/convex/schema.ts +++ b/packages/kit/convex/schema.ts @@ -910,6 +910,17 @@ const schema = defineSchema({ // dashboard's `getActiveSyncJob` subscription and the // double-enqueue guard inside `enqueueProductSync`. .index("by_project_platform_status", ["projectId", "platform", "status"]) + // Composite (projectId, platform, createdAt) — backs + // `getActiveSyncJob` so the latest job per platform is a + // direct index range scan instead of "fetch every job for the + // project, then in-memory filter by platform". Without this, + // the query degraded with the table size on multi-platform + // projects (Gemini review on PR #127). + .index("by_project_platform_created", [ + "projectId", + "platform", + "createdAt", + ]) .index("by_project_and_created", ["projectId", "createdAt"]) // Reaper / pruner scans. .index("by_status_and_deadline", ["status", "expectedDeadline"]) diff --git a/packages/kit/server/api/v1/products.ts b/packages/kit/server/api/v1/products.ts index c613be94..1d567691 100644 --- a/packages/kit/server/api/v1/products.ts +++ b/packages/kit/server/api/v1/products.ts @@ -158,8 +158,12 @@ products.post("/:apiKey/sync/:platform", async (c) => { const apiKey = c.req.param("apiKey"); const platformParam = c.req.param("platform"); const direction = - (c.req.query("direction") as "pull" | "push" | "both" | undefined) ?? - "both"; + (c.req.query("direction") as + | "pull" + | "push" + | "both" + | "purge-local" + | undefined) ?? "both"; const dryRun = c.req.query("dryRun") === "true"; if (platformParam !== "ios" && platformParam !== "android") { return c.json( @@ -201,9 +205,11 @@ products.post("/:apiKey/sync/:platform", async (c) => { // catalogs in 1-2 min. SSE is a future option; polling kept simple // for v1. products.get("/:apiKey/sync/jobs/:jobId", async (c) => { + const apiKey = c.req.param("apiKey"); const jobId = c.req.param("jobId"); try { const job = await client.query(api.products.jobs.getSyncJobById, { + apiKey, jobId: jobId as Id<"productSyncJobs">, }); if (!job) { @@ -231,9 +237,11 @@ products.get("/:apiKey/sync/jobs/:jobId", async (c) => { // Operator-initiated cancel. The worker checks `cancelRequested` // at phase boundaries. products.post("/:apiKey/sync/jobs/:jobId/cancel", async (c) => { + const apiKey = c.req.param("apiKey"); const jobId = c.req.param("jobId"); try { const result = await client.mutation(api.products.jobs.cancelProductSync, { + apiKey, jobId: jobId as Id<"productSyncJobs">, }); return c.json(result); diff --git a/packages/kit/src/lib/sentry.ts b/packages/kit/src/lib/sentry.ts index e3acc385..3e710c5d 100644 --- a/packages/kit/src/lib/sentry.ts +++ b/packages/kit/src/lib/sentry.ts @@ -50,9 +50,23 @@ if (typeof dsn === "string" && dsn.length > 0 && onAllowedHost) { // (or downsample) instead of treating each as a fresh signal. beforeSend: (event, hint) => { const exception = hint?.originalException; - const message = - (exception instanceof Error ? exception.message : String(exception)) ?? - ""; + // Sentry events occasionally arrive with no `originalException` + // (programmatic captureMessage, late-bound rejections that + // lost their cause), only an `event.message` / + // `logentry.message`. Build the classifier input from every + // available source so reconnect noise gets tagged regardless + // of where the message lives (CodeRabbit review on PR #127). + const message = [ + exception instanceof Error + ? exception.message + : typeof exception === "string" + ? exception + : "", + event.message ?? "", + event.logentry?.message ?? "", + ] + .filter(Boolean) + .join(" "); const isFetchLoadFailed = /Load failed|Failed to fetch|NetworkError/i.test(message); const looksConvex = /convex\.cloud|\/api\/(action|query|mutation)/i.test( diff --git a/packages/kit/src/pages/auth/organization/project/products.tsx b/packages/kit/src/pages/auth/organization/project/products.tsx index 2955e3b0..44494ace 100644 --- a/packages/kit/src/pages/auth/organization/project/products.tsx +++ b/packages/kit/src/pages/auth/organization/project/products.tsx @@ -94,6 +94,14 @@ export default function ProjectProducts() { if (!job) continue; const terminal = job.status === "succeeded" || job.status === "failed"; if (!terminal) continue; + // Once the operator has dismissed a terminal job, the row's + // `progress.phase` flips to "dismissed". The result banner + // already gates on this; the toast effect needs the same + // gate or it re-fires the success/failure toast on every + // subsequent page reload (because `lastShownJobIdRef` is + // in-memory and resets) until the pruner deletes the row + // (CodeRabbit review on PR #127). + if (job.progress.phase === "dismissed") continue; if (lastShownJobIdRef.current[platform] === job._id) continue; lastShownJobIdRef.current[platform] = job._id; const label = platform === "IOS" ? "App Store Connect" : "Play Console"; @@ -244,7 +252,7 @@ export default function ProjectProducts() { const onCancel = async (jobId: SyncJob["_id"], label: string) => { try { - await cancelSync({ jobId }); + await cancelSync({ apiKey: project.apiKey, jobId }); toast.message(`${label} sync — cancellation requested`, { duration: 4_000, }); @@ -455,7 +463,7 @@ export default function ProjectProducts() { void onCancel(jobId, "App Store Connect"); }} onDismiss={(jobId) => { - void dismissJob({ jobId }); + void dismissJob({ apiKey: project.apiKey, jobId }); }} /> { - void dismissJob({ jobId }); + void dismissJob({ apiKey: project.apiKey, jobId }); }} /> From 3c9421f17ccc01f3a57a39d67847e619ee6ba176 Mon Sep 17 00:00:00 2001 From: hyochan Date: Tue, 5 May 2026 19:25:41 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix(kit):=20round=202=20review=20=E2=80=94?= =?UTF-8?q?=20atomic=20dedup,=20USD=20guard,=20Modal=20a11y,=20contrast?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/kit/convex/products/jobs.ts | 93 +++++++--- packages/kit/convex/products/play.ts | 15 ++ packages/kit/convex/schema.ts | 15 ++ .../kit/src/components/AuthModal/index.tsx | 2 +- .../src/components/{AuthModal => }/Modal.tsx | 38 +++- packages/kit/src/components/Tooltip.tsx | 68 ++++++- .../auth/organization/project/products.tsx | 167 +++++++++--------- 7 files changed, 278 insertions(+), 120 deletions(-) rename packages/kit/src/components/{AuthModal => }/Modal.tsx (75%) diff --git a/packages/kit/convex/products/jobs.ts b/packages/kit/convex/products/jobs.ts index 41336ed1..37ed84ff 100644 --- a/packages/kit/convex/products/jobs.ts +++ b/packages/kit/convex/products/jobs.ts @@ -172,27 +172,31 @@ export const enqueueProductSync = mutation({ }), handler: async (ctx, args) => { const { project, userId } = await resolveProjectByApiKey(ctx, args.apiKey); - const existingActive = await ctx.db - .query("productSyncJobs") - .withIndex("by_project_platform_status", (q) => - q - .eq("projectId", project._id) - .eq("platform", args.platform) - .eq("status", "queued"), - ) - .first(); - const existingRunning = await ctx.db - .query("productSyncJobs") - .withIndex("by_project_platform_status", (q) => - q - .eq("projectId", project._id) - .eq("platform", args.platform) - .eq("status", "running"), - ) - .first(); - const existing = existingActive ?? existingRunning; - if (existing) { - return { jobId: existing._id, deduped: true }; + // Atomic dedup via the project's `activeSyncJobIds` lock field. + // Reading and writing the project doc lets Convex's OCC collapse + // two concurrent enqueue mutations onto the same job: both read + // the project, both try to patch, only one commit wins; the + // loser retries, sees the lock, and returns the deduped jobId + // (Copilot review on PR #127). + // + // Index-only dedup (the prior implementation) wasn't atomic + // because the two queries returned `null` for both concurrent + // callers, then both inserted separate `productSyncJobs` rows + // and scheduled separate workers — fanning out conflicting + // upstream writes. + const lockedJobId = project.activeSyncJobIds?.[args.platform]; + if (lockedJobId) { + const lockedJob = await ctx.db.get(lockedJobId); + if ( + lockedJob && + (lockedJob.status === "queued" || lockedJob.status === "running") + ) { + return { jobId: lockedJob._id, deduped: true }; + } + // Lock points at a stale (terminal) row — fall through to + // claim a fresh slot. The worker should have cleared it; this + // path covers a crashed worker or a job marked failed via the + // reaper. } const now = Date.now(); const jobId = await ctx.db.insert("productSyncJobs", { @@ -205,6 +209,12 @@ export const enqueueProductSync = mutation({ ...(userId ? { createdBy: userId } : {}), createdAt: now, }); + await ctx.db.patch(project._id, { + activeSyncJobIds: { + ...(project.activeSyncJobIds ?? {}), + [args.platform]: jobId, + }, + }); if (args.direction === "purge-local") { // Purge runs in this module's V8-isolate runtime — no Apple // credentials, no Play OAuth, no `"use node"` cost. Just a @@ -429,6 +439,22 @@ export const markJobSucceeded = internalMutation({ ...(args.plannedWrites ? { plannedWrites: args.plannedWrites } : {}), }, }); + // Clear the project's lock so the next enqueue can claim the + // slot. Guard against a stale lock pointing at a different job + // (e.g. a manual db.patch races with a finished worker) so we + // never overwrite a fresh lock with `undefined`. + const job = await ctx.db.get(args.jobId); + if (job) { + const project = await ctx.db.get(job.projectId); + if (project && project.activeSyncJobIds?.[job.platform] === args.jobId) { + await ctx.db.patch(project._id, { + activeSyncJobIds: { + ...project.activeSyncJobIds, + [job.platform]: undefined, + }, + }); + } + } }, }); @@ -465,6 +491,20 @@ export const markJobFailed = internalMutation({ ...(truncated ? { failuresTruncated: true } : {}), }, }); + // Mirror `markJobSucceeded` — clear the lock on terminal + // failure so the next enqueue isn't blocked by a stuck slot. + const job = await ctx.db.get(args.jobId); + if (job) { + const project = await ctx.db.get(job.projectId); + if (project && project.activeSyncJobIds?.[job.platform] === args.jobId) { + await ctx.db.patch(project._id, { + activeSyncJobIds: { + ...project.activeSyncJobIds, + [job.platform]: undefined, + }, + }); + } + } }, }); @@ -488,6 +528,17 @@ export const reapStaleProductSyncJobs = internalMutation({ error: "Worker timed out — sync exceeded the 9-minute ceiling", progress: { phase: "reaped" }, }); + // Clear the project's lock so the next enqueue isn't pinned + // by the dead worker. + const project = await ctx.db.get(job.projectId); + if (project && project.activeSyncJobIds?.[job.platform] === job._id) { + await ctx.db.patch(project._id, { + activeSyncJobIds: { + ...project.activeSyncJobIds, + [job.platform]: undefined, + }, + }); + } } return { reaped: stale.length }; }, diff --git a/packages/kit/convex/products/play.ts b/packages/kit/convex/products/play.ts index 00c117b0..6f61f117 100644 --- a/packages/kit/convex/products/play.ts +++ b/packages/kit/convex/products/play.ts @@ -688,6 +688,21 @@ async function performAndroidSync( "Subscription requires priceAmountMicros + currency to mint a Play base plan; otherwise the product will not be purchasable.", ); } + // Play's `regionalConfigs` requires the `currencyCode` to + // be the local currency of the `regionCode` it's paired + // with — pushing `regionCode: "US"` with a non-USD price + // returns a generic 400 from the API and leaves the + // operator chasing a confusing error message. Match the + // iOS path's currency-validation pattern (asc.ts intro + // offer push) and surface an actionable failure here so + // the row stays in Draft and the dashboard reports + // exactly which SKU failed and why (Gemini review on + // PR #127). + if (row.currency !== "USD") { + throw new Error( + `Subscription "${row.productId}" has currency "${row.currency}" but the kit→Play push currently only supports the US region (USD). Set the price in USD on the dashboard, or pre-create the subscription in Play Console with your preferred regional pricing and let the next pull-sync mirror it back into kit.`, + ); + } const basePlanId = basePlanIdForPeriod(row.billingPeriod); if (dryRun) { plannedWrites.push({ diff --git a/packages/kit/convex/schema.ts b/packages/kit/convex/schema.ts index beff8890..fbe3806a 100644 --- a/packages/kit/convex/schema.ts +++ b/packages/kit/convex/schema.ts @@ -219,6 +219,21 @@ const schema = defineSchema({ horizonAppId: v.optional(v.union(v.string(), v.null())), horizonAppSecret: v.optional(v.union(v.string(), v.null())), + // Per-platform "active product-sync job" lock. Read-and-patched + // inside `enqueueProductSync` so Convex's optimistic concurrency + // control collapses two concurrent enqueue mutations onto the + // same job — without this, both readers see an empty + // `productSyncJobs` index range, both insert separate queued + // rows, and the scheduler fans out two competing workers + // (Copilot review on PR #127). Cleared by the worker's + // success/failure mutation so the next enqueue can claim it. + activeSyncJobIds: v.optional( + v.object({ + IOS: v.optional(v.id("productSyncJobs")), + Android: v.optional(v.id("productSyncJobs")), + }), + ), + createdAt: v.number(), updatedAt: v.number(), }) diff --git a/packages/kit/src/components/AuthModal/index.tsx b/packages/kit/src/components/AuthModal/index.tsx index 0dea31ac..3e8e9f46 100644 --- a/packages/kit/src/components/AuthModal/index.tsx +++ b/packages/kit/src/components/AuthModal/index.tsx @@ -5,7 +5,7 @@ import { Loader2, Check, AlertCircle, AtSign, Send } from "lucide-react"; import { SiGithub } from "@icons-pack/react-simple-icons"; import { useNavigate } from "react-router-dom"; import { api } from "@/convex"; -import { Modal } from "./Modal"; +import { Modal } from "@/components/Modal"; interface AuthModalProps { isOpen: boolean; diff --git a/packages/kit/src/components/AuthModal/Modal.tsx b/packages/kit/src/components/Modal.tsx similarity index 75% rename from packages/kit/src/components/AuthModal/Modal.tsx rename to packages/kit/src/components/Modal.tsx index 4c7a6c5d..72c02c23 100644 --- a/packages/kit/src/components/AuthModal/Modal.tsx +++ b/packages/kit/src/components/Modal.tsx @@ -2,12 +2,27 @@ import React, { useEffect, useRef } from "react"; import { createPortal } from "react-dom"; import { X } from "lucide-react"; +// Shared modal primitive. Originally co-located in `AuthModal/Modal.tsx`; +// promoted here so other dialogs (e.g. the destructive +// PurgeConfirmDialog on the products page) can reuse the focus +// trap, escape handling, scroll lock, and focus-restore behavior +// instead of each rebuilding the same a11y plumbing inline. interface ModalProps { isOpen: boolean; onClose: () => void; children: React.ReactNode; className?: string; ariaLabel?: string; + // Hide the built-in close (X) button when the consumer supplies + // its own dismissal affordance (e.g. a Cancel button in a confirm + // dialog). The close button is otherwise nice for the auth + // modal's "tap-X" muscle memory. + showCloseButton?: boolean; + // Override the inner content wrapper's padding. Defaults to + // `p-6` to match AuthModal's existing layout; confirm dialogs + // typically want a tighter `p-5` or no padding so they can render + // their own structured spacing. + contentClassName?: string; } export function Modal({ @@ -16,6 +31,8 @@ export function Modal({ children, className = "", ariaLabel = "Dialog", + showCloseButton = true, + contentClassName = "p-6", }: ModalProps) { const modalRef = useRef(null); const previouslyFocusedRef = useRef(null); @@ -116,17 +133,20 @@ export function Modal({ tabIndex={-1} className={`relative w-full max-w-md bg-white dark:bg-[#18181f] rounded-2xl shadow-2xl animate-scale-in focus:outline-none ${className}`} > - {/* Close button */} - + {showCloseButton ? ( + + ) : null} {/* Content */} -
+
{children}
diff --git a/packages/kit/src/components/Tooltip.tsx b/packages/kit/src/components/Tooltip.tsx index 82e9077b..6f4de92c 100644 --- a/packages/kit/src/components/Tooltip.tsx +++ b/packages/kit/src/components/Tooltip.tsx @@ -1,4 +1,13 @@ -import { ReactNode, useState } from "react"; +import { + Children, + cloneElement, + isValidElement, + ReactNode, + useId, + useState, + type FocusEvent, + type ReactElement, +} from "react"; import { cn } from "@/lib/utils"; type Side = "top" | "bottom" | "left" | "right"; @@ -7,14 +16,22 @@ type Align = "start" | "center" | "end"; // Lightweight hover/focus tooltip. Self-contained — no Radix / // floating-ui dep — because the only thing we need is a positioned // popover that opens on `mouseenter` / `focusin` and closes on -// `mouseleave` / `focusout`. Anchors against the trigger element via -// the wrapping `relative` div, so the trigger has to be a single -// inline-block element (a button, a span); avoid wrapping block -// elements that fight with `absolute` positioning. +// `mouseleave` / `focusout`. Anchors against the trigger element +// via the wrapping `relative` div. +// +// A11y plumbing (Copilot review on PR #127): +// - The trigger element gets `aria-describedby` pointing at a +// stable id on the tooltip popover (via `useId` + `cloneElement`), +// so screen readers announce the tooltip text alongside the +// trigger's own label. +// - Focus close uses `onBlurCapture` with a `relatedTarget` +// containment check — the tooltip stays open while focus moves +// between descendants of the wrapper instead of closing the +// moment any inner element is blurred. // // Use a real component (not a `title=` attribute or inline JSX) so -// the hover affordance is consistent across the app: same delay, same -// 320px max-width, same border/padding tokens. +// the hover affordance is consistent across the app: same delay, +// same widths, same border/padding tokens. export function Tooltip({ children, content, @@ -29,17 +46,50 @@ export function Tooltip({ widthClass?: string; }) { const [open, setOpen] = useState(false); + const tooltipId = useId(); + + // Wire `aria-describedby` onto the first valid React element so + // assistive tech announces the tooltip text. Falls through + // unchanged when the consumer passes a fragment / text node / + // multiple siblings — we don't want to silently swallow those + // shapes, the user just gets a tooltip without the SR linkage. + const trigger = + Children.count(children) === 1 && isValidElement(children) + ? cloneElement( + children as ReactElement<{ "aria-describedby"?: string }>, + { + "aria-describedby": tooltipId, + }, + ) + : children; + + const handleBlur = (event: FocusEvent) => { + // `relatedTarget` is the element receiving focus next. When it + // stays inside the wrapper (e.g. focus moving between two + // buttons in the trigger), keep the tooltip open. Closing on + // every bubbled blur was prematurely hiding the popover the + // moment focus shifted across descendants. + if ( + event.relatedTarget instanceof Node && + event.currentTarget.contains(event.relatedTarget) + ) { + return; + } + setOpen(false); + }; + return (
setOpen(true)} onMouseLeave={() => setOpen(false)} onFocus={() => setOpen(true)} - onBlur={() => setOpen(false)} + onBlurCapture={handleBlur} > - {children} + {trigger} {open ? ( +
+
+ +

+ Local edits not yet pushed to {storeLabel} (price changes, review + notes, titles) will be lost when the next Sync + re-pulls the upstream copy. +

+
+
+ +

+ Draft rows that were never pushed upstream will be{" "} + permanently deleted — they don't exist on{" "} + {storeLabel} so Sync can't recover them. +

+
+ + +
- + ); } @@ -945,13 +947,18 @@ function ProductGroup({ }} /> {showResult && job ? ( + // Theme-aware text colors — `-700` for the light surface, + // `-200` for the dark surface. The earlier dark-only + // palette (`text-rose-200` etc.) was unreadable in light + // mode against the bg-tint background (Gemini review on + // PR #127).