feat: kit hosted backend (webhooks + state + products sync + MCP + dashboard)#124
Conversation
Introduce `webhook.graphql` defining the normalized cross-store lifecycle event surface that kit will emit to clients. This is the foundation for removing the need for users to run their own server: kit ingests Apple ASN v2 and Google RTDN, normalizes them into one shape, and streams them to authenticated clients via a GraphQL Subscription transport. - 15 unified WebhookEventType values covering subscription lifecycle (started/renewed/expired/grace/retry/recovered/canceled/uncanceled/ revoked/price-change/product-changed/paused/resumed) plus refunds, consumption requests, and test notifications. - WebhookEventSource discriminator (ASN v2 vs RTDN) and Environment (Production/Sandbox/Xcode). - WebhookEvent payload with idempotency `id`, occurredAt/receivedAt epoch-ms timestamps, cross-platform `purchaseToken`, optional subscription state and price snapshot, plus `rawSignedPayload` escape hatch. - `Subscription.webhookEvent` for live streaming and `Query.webhookEventsSince` for reconnection backfill. - ASN v2 ↔ RTDN ↔ openiap mapping table in `knowledge/external/webhook-mapping.md` (SSOT for the kit receivers shipping in the next PR). Codegen verified across all 5 target languages; types synced to all 7 SDK files (apple, google, rn-iap, expo-iap, flutter, godot, kmp). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires kit as the server-side webhook receiver so consumers can drop
their own backend. Apple App Store Server Notifications v2 and Google
Play Real-Time Developer Notifications are now verified, normalized
into the unified `WebhookEvent` shape from PR #1, and persisted with
idempotency for replay/dedup.
What's in:
- Convex schema: `webhookEvents` (normalized event log, indexed by
project / purchase-token / received-at) plus `webhookIdempotencyKeys`
(`(source, sourceNotificationId)` unique guard).
- `convex/webhooks/shared.ts`: pure normalizers `normalizeAppleAsn` and
`normalizeGoogleRtdn` mapping the 30+ upstream notification types and
subtypes to the 16-value openiap vocabulary defined in
`webhook.graphql`. Translates Apple's millicent pricing into Google-
compatible micros and Apple `expirationIntent` / Google `cancelReason`
into `WebhookCancellationReason`.
- `convex/webhooks/internal.ts`: `recordWebhookEvent` does the
dedup-first insert; Apple's transient-5xx retries and Pub/Sub's
at-least-once delivery both collapse into `deduped: true` with a
200 ACK. `pruneWebhookEvents` daily-prunes events past the 30-day
retention window so `webhookEventsSince` stays bounded.
- `convex/webhooks/apple.ts`: action that decodes the signed payload,
verifies it via Apple's `SignedDataVerifier` against the project's
bundle ID + cached root certificates, decodes the embedded
transaction + renewal JWS, then normalizes and inserts.
- `convex/webhooks/google.ts`: action that takes the parsed Pub/Sub
RTDN body, optionally enriches via `purchases.subscriptionsv2.get`
for state/expiry/price (graceful fallback when the project hasn't
uploaded service-account credentials yet), then normalizes and
inserts.
- `convex/webhooks/query.ts`: `webhookEventsSince` query used by SDKs
on reconnect to backfill events delivered while their WebSocket was
closed. Capped at 500 per page, ascending by `receivedAt`.
- `server/api/v1/webhooks.ts`: Hono routes mounted at
`/v1/webhooks/apple/{apiKey}` and `/v1/webhooks/google/{apiKey}`.
Apple route validates the signedPayload envelope before dispatching;
Google route verifies Pub/Sub OIDC bearer tokens against the
configured `GOOGLE_PUBSUB_PUSH_AUDIENCE` and decodes base64
`message.data` before dispatching. Unsupported notification types
ACK 200 + `dropped: true` so the upstream stops retrying — Apple
and Google ship new types ahead of the spec.
Verification:
- 29 new vitest tests in `shared.test.ts` cover every spec event type
plus the cancellation-reason translations and rejection paths
(missing UUID / unsupported type / missing purchaseToken). Total kit
suite: 254/254 passing.
- `bun run lint` clean (0 errors); `bun run smoke:server` passes
(server compiles + boots, all probes green); `bun run audit:docs`
no new failures.
What's NOT in (handled in follow-ups):
- GraphQL Subscription endpoint that streams `webhookEvents` to clients
over WebSocket (Phase 1 PR #3).
- Apple/Google sandbox dry-run end-to-end test — needs live credentials
and is out of scope for an autonomous run; the receiver is wired so a
maintainer can register the URL in App Store Connect / Pub/Sub and
verify with a TEST notification.
- 5 SDK transport integrations (Phase 1 PR #4-8).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end webhook support: GraphQL/Webhook schema and language bindings, server SSE stream with backlog/replay and Last-Event-ID, Apple ASN v2 and Google RTDN normalizers, idempotent Convex recording/pruning and cursoring, subscription state machine and stats, multi-platform SSE clients/hooks, product sync, kit HTTP clients, dashboard pages, MCP CLI, tests, crons, and CI/tooling updates. ChangesWebhooks & Cross-platform SDKs (single DAG)
Sequence Diagram(s)sequenceDiagram
participant Store as Store (Apple / Google)
participant Server as Kit Server
participant Convex as Convex Backend
participant Client as SDK Client
Store->>Server: POST /v1/webhooks/:apiKey (ASN v2 or RTDN)
Server->>Server: detect payload, verify (JWS or OIDC)
Server->>Convex: ingestAppleAsnIOS / ingestGoogleRtdn
Convex->>Convex: normalizeWebhookEvent
Convex->>Convex: recordWebhookEvent (idempotent)
Convex->>Convex: applySubscriptionEvent (state machine + stats)
Convex-->>Server: { eventId, type, deduped }
Server-->>Store: 200 OK
Client->>Server: GET /v1/webhooks/stream/:apiKey (Last-Event-ID)
Server->>Convex: webhookEventsSince(...) (backfill)
Convex-->>Server: events[]
Server-->>Client: SSE ready + backlog events...
Convex->>Server: onUpdate (live)
Server-->>Client: SSE live event
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Code Review
This pull request implements a unified lifecycle webhook system for Apple App Store Server Notifications (ASN) v2 and Google Play Real-Time Developer Notifications (RTDN). It introduces new database tables for event storage and idempotency, normalization logic for both platforms, and Hono routes for ingestion. Feedback includes renaming the Apple ingestion action to follow naming conventions for iOS-specific functions, using ConvexError for better error mapping in the API layer, preventing precision loss in Google price calculations by using BigInt, and ensuring the _creationTime field is included in queries to support reliable SDK checkpointing.
Completes Phase 1 in this PR by adding the SSE streaming endpoint,
the prune cron, the shared TS webhook client, per-SDK integrations
for react-native-iap, expo-iap, flutter_inapp_purchase, kmp-iap,
and godot-iap, plus a docs page tying it all together.
Server-side (kit):
- Hourly `pruneWebhookEvents` cron registered in crons.ts so the
30-day retention window in `webhookEventsSince` stays bounded.
- `/v1/webhooks/stream/{apiKey}` SSE endpoint that polls
`webhookEventsSince` every 1.5s and emits new events as
`id: <sourceNotificationId>` / `event: <type>` / `data: <json>`.
EventSource clients reconnect with `Last-Event-ID`; kit looks up
the named event's `receivedAt` and resumes so events fired during
a closed connection are delivered in order.
Shared TS client:
- `packages/gql/src/webhook-client.ts` — transport-agnostic
`connectWebhookStream({apiKey, onEvent, onError})` plus a pure
`parseWebhookEventData` helper. Default factory uses the global
`EventSource`; consumers can inject a polyfill.
- 11 vitest tests cover heartbeats, stream-control envelopes,
malformed JSON, missing required fields, transport errors,
trailing-slash trimming, and apiKey URL-encoding.
- `./webhook-client` export added to `packages/gql/package.json`
and `sync-to-platforms.mjs` copies the canonical implementation
into `libraries/react-native-iap/src/` and
`libraries/expo-iap/src/` during normal type sync.
Per-SDK integrations:
- react-native-iap: synced `webhook-client.ts` + a
`useWebhookEvents` hook returning `{events, lastError, isConnected}`.
Re-exports the helpers from `index.ts`. 4 jest tests; full suite
276/276 passes.
- expo-iap: same surface (`webhook-client.ts`, `useWebhookEvents.ts`).
Re-exported from `index.ts`. 2 jest tests; full suite 46/46 passes.
- flutter_inapp_purchase: `lib/webhook_client.dart` — typed
`WebhookEvent`, `connectWebhookStream(apiKey:)` returning a
`WebhookListener` with `events` / `errors` streams, plus a pure
`parseWebhookEventData` helper. 3 flutter tests pass.
- kmp-iap: `WebhookClient.kt` in commonMain — `WebhookEventTypeName`
enum, `WebhookEvent` data class, `WebhookEventParser.parse()`,
and `webhookStreamUrl()` URL builder. Transport intentionally not
in commonMain so the module avoids pulling Ktor; consumers wire a
per-target HTTP client and feed JSON frames to the parser.
4 commonTest tests added.
- godot-iap: `addons/godot-iap/webhook_client.gd` —
`OpenIapWebhookClient` Node running an HTTPClient SSE loop with
`Last-Event-ID` reconnect, emitting `event_received(Dictionary)`,
`connected_to_stream()`, `stream_error(code, message)` signals.
Docs:
- `packages/docs/src/pages/docs/webhooks.tsx` routed at
`/docs/webhooks` — architecture overview, event-shape reference
linking to `webhook.graphql` + the mapping doc, per-SDK usage in
TypeScript / Dart / Kotlin / GDScript, and the reconnect-and-replay
contract.
Verification:
- kit: lint clean (0 errors), 254/254 vitest, smoke probes green.
- gql: 11/11 webhook-client vitest tests.
- react-native-iap: 276/276 jest, typecheck clean for src/.
- expo-iap: 46/46 jest, typecheck clean for src/.
- flutter_inapp_purchase: 3/3 flutter test.
- docs: tsc clean, audit:docs no new failures.
- kmp-iap: tests written but a gradle run is out of scope for this
autonomous turn — parser is pure and mirrors the TS / Dart logic
that vitest + flutter test already validate.
- godot-iap: GDScript needs the Godot editor for runtime tests;
the SSE parser mirrors the Dart implementation tested above.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…esub parity)
Closes the gap with onesub by adding the four feature areas the
comparison table called out as missing: subscription state machine
(Phase 2), revenue metrics summary (Phase 2), product catalog CRUD
(Phase 3 — kit-side cache; ASC/Play push-sync stays as a follow-up),
hosted paywalls (Phase 4), and a stdio MCP server (Phase 5).
Phase 2 — subscriptions + entitlements + metrics:
- New `subscriptions` and `revenueMetricsDaily` Convex tables.
- `subscriptions/stateMachine.ts` — pure state-machine that maps each
WebhookEventType to the next persistent row, the entitlement
decision, and a transition tag for analytics. 17 vitest cases cover
every transition.
- `applySubscriptionEvent` internal mutation runs after every
non-deduped webhook, idempotent on `lastEventId`.
- `subscriptions/query.ts` — `subscriptionStatus` (onesub
`/onesub/status` parity), `entitlements`, `listSubscriptions`,
`metricsSummary`.
- `subscriptions/mutation.ts` — public `bindUser` for SDKs to attach a
userId after a successful verifyReceipt.
- Hono routes at
`/v1/subscriptions/{status,entitlements,list,metrics,bind-user}/{apiKey}`.
Phase 3 — product catalog:
- New `products` Convex table.
- `products/{mutation,query}.ts` for upsert / soft-remove / list.
- Hono routes at `/v1/products/{apiKey}`. Phase 3 follow-up will
layer App Store Connect + Play Developer API push-sync on top.
Phase 4 — hosted paywalls:
- New `paywalls` table with slug uniqueness per project.
- `paywalls/{mutation,query}.ts` and `/v1/paywalls/{apiKey}` routes
for CRUD plus a hosted HTML renderer at
`/v1/paywalls/{apiKey}/{slug}`. The HTML emits a
`{ openiap: 'purchase', productId }` message via
`ReactNativeWebView.postMessage` / `flutter_inappwebview` /
`window.parent.postMessage` so any of the 5 SDK WebViews can
dispatch the actual purchase.
Phase 5 — MCP server (`@hyodotdev/openiap-mcp-server`):
- New `packages/mcp-server` with 11 tools mirroring the onesub MCP
surface: `openiap_setup`, `openiap_add_paywall`,
`openiap_check_status`, `openiap_troubleshoot`,
`openiap_create_product`, `openiap_list_products`,
`openiap_view_subscribers`, `openiap_simulate_purchase`,
`openiap_simulate_webhook`, `openiap_inspect_state`,
`openiap_manage_product`. Driven by stdio so it plugs into Claude
Desktop / Cursor / Codex without additional infra.
SDK helpers:
- New `packages/gql/src/kit-api.ts` — typed `kitApi({apiKey})` wrapper
around `/v1/subscriptions/...` + `/v1/paywalls/...`. 5 vitest cases
cover URL encoding, error mapping, and the bind-user POST shape.
- Synced into `libraries/react-native-iap/src/kit-api.ts` and
`libraries/expo-iap/src/kit-api.ts` via the existing
`sync-to-platforms.mjs`, and re-exported from each library's
`index.ts`.
Verification:
- kit: lint clean (0 errors), 271/271 vitest, smoke probes green.
- gql: 16/16 vitest (11 webhook-client + 5 kit-api).
- react-native-iap: 276/276 jest.
- expo-iap: 46/46 jest.
- audit:docs: no new failures.
Out of scope (follow-ups):
- App Store Connect + Play Developer API push-sync (the `products`
row carries `storeRef` so this is additive).
- Convex-native realtime subscription stream (currently SSE polls
`webhookEventsSince` every 1.5s).
- KMP per-target SSE transport adapters; Godot runtime tests.
- Live sandbox E2E (needs ASC / RTDN credentials a maintainer holds).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ard UX Closes the remaining "out of scope" items from the previous commits. App Store Connect push-sync (Phase 3 completion): - `convex/products/jwt.ts` — pure ES256 JWT minter (DER → JWS r||s). 4 vitest cases including a cryptographic round-trip. - `convex/products/asc.ts` — `AscClient` + `pushSyncProductsApple` action that pulls IAPs / subscriptions from ASC, upserts kit's `products`, and pushes Draft kit rows to ASC writing the upstream id back as `storeRef`. Google Play push-sync: - `convex/products/play.ts` — `pushSyncProductsGoogle` action reusing the per-project service-account JSON. Lists `inappproducts` and `monetization.subscriptions`, then pushes Draft rows back. - `convex/products/sync.ts` — internal mutations / queries shared by both push-sync actions. Convex realtime SSE upgrade: - `/v1/webhooks/stream/:apiKey` no longer polls. Per-connection `ConvexClient.onUpdate(...)` subscription dedupes by id and emits the moment Convex commits. 25s heartbeat keeps proxies happy. KMP per-target SSE transports: - `commonMain/.../WebhookTransport.kt` — `expect class` with an `events(lastEventId)` Flow surface. - `androidMain` — `HttpURLConnection`-based actual. - `iosMain` — NSURLSession + `NSURLSessionDataDelegate` cinterop. - Build adds `-Xexpect-actual-classes` to silence Kotlin 2.x. End-to-end conformance harness: - `convex/webhooks/conformance.test.ts` — 6 multi-step lifecycle scenarios driving the full receiver → state machine → entitlement pipeline. 6/6 passing. Dashboard UX (`packages/kit/src/pages/auth/organization/project/`): - `subscriptions.tsx` — metrics summary + filterable table. - `products.tsx` — catalog editor with Sync buttons + per-product failure surfacing. - `paywalls.tsx` — paywall CRUD + hosted-URL preview. - `webhooks.tsx` — copy URLs + curl recipe. - All four mounted in the project tab strip. Docs: - `packages/docs/src/pages/docs/kit-backend.tsx` at `/docs/kit-backend` — surface map, dashboard tour, per-SDK entitlement check, paywall WebView bridge contract, push-sync direction matrix, MCP config. Verification: - kit lint clean (0 errors); 281/281 vitest; smoke green. - gql 16/16 vitest; rn-iap 276/276 jest; expo-iap 46/46 jest; docs tsc clean; audit:docs no new failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds “Phase 1” kit backend surfaces for lifecycle webhooks + subscription state, plus client/runtime helpers across JS (gql + RN/Expo), KMP, Flutter, and Godot to consume the normalized webhook stream and new kit endpoints.
Changes:
- Adds new kit
/v1routes for subscriptions, products, paywalls, and webhooks (plus Convex tables/mutations/queries/state machine + cron pruning). - Adds cross-SDK webhook stream clients (JS + RN/Expo hook, KMP transports/parser, Godot node, Flutter parser tests) and a small kit HTTP API wrapper (JS + MCP).
- Adds dashboard pages for Subscriptions / Products / Paywalls / Webhooks and wires them into the authenticated router.
Review notes (requires changes):
packages/kit/convex/products/play.ts: Playinappproducts.insertsetspurchaseTypeto"managedUser"for both Consumable and NonConsumable; consumables should not be created as managed.packages/kit/server/api/v1/subscriptions.tsandpackages/kit/server/api/v1/products.ts: query params (state,direction) are passed through without validation (using casts), which can turn bad input into Convex validator errors and 4xx/5xx mismatches.libraries/kmp-iap/.../WebhookClient.kt:webhookStreamUrldoes not URL-encodeapiKey, diverging from the TS/RN behavior and tests.packages/gql/package.json: exports./webhook-clientbut not./kit-api, even thoughkit-api.tsis added alongside it.
Reviewed changes
Copilot reviewed 70 out of 72 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp-server/tsconfig.json | Adds TS build config for the MCP server package. |
| packages/mcp-server/src/kit-client.ts | Adds MCP-side HTTP wrapper for kit /v1 endpoints. |
| packages/mcp-server/package.json | Declares MCP server package metadata and build scripts. |
| packages/kit/src/pages/auth/organization/project/webhooks.tsx | Adds project UI for webhook receiver/stream URLs + test curl snippet. |
| packages/kit/src/pages/auth/organization/project/subscriptions.tsx | Adds subscriptions dashboard page (filters + metrics). |
| packages/kit/src/pages/auth/organization/project/paywalls.tsx | Adds paywalls CRUD UI in the kit dashboard. |
| packages/kit/src/pages/auth/organization/project/index.tsx | Adds new project tabs (subscriptions/products/paywalls/webhooks). |
| packages/kit/src/pages/auth/index.tsx | Wires new project routes into authenticated routing. |
| packages/kit/src/convex.ts | Re-exports Doc type from Convex generated model. |
| packages/kit/server/convex.ts | Exposes Convex URL for realtime client usage (SSE stream). |
| packages/kit/server/api/v1/subscriptions.ts | Adds /v1/subscriptions/* routes (status/entitlements/list/metrics/bind-user). |
| packages/kit/server/api/v1/routes.ts | Registers new /webhooks, /subscriptions, /paywalls, /products route groups. |
| packages/kit/server/api/v1/products.ts | Adds /v1/products/* CRUD + push-sync trigger routes. |
| packages/kit/server/api/v1/paywalls.ts | Adds paywall CRUD plus hosted HTML renderer for WebViews. |
| packages/kit/package.json | Adds google-auth-library dependency for Google auth flows. |
| packages/kit/convex/webhooks/validators.ts | Centralizes webhook enum validators. |
| packages/kit/convex/webhooks/query.ts | Adds webhookEventsSince backfill query. |
| packages/kit/convex/webhooks/internal.ts | Adds idempotent event recording + pruning mutations. |
| packages/kit/convex/webhooks/google.ts | Adds RTDN ingest action + optional subscriptionsv2 enrichment. |
| packages/kit/convex/subscriptions/stateMachine.ts | Adds pure subscription state transition logic. |
| packages/kit/convex/subscriptions/stateMachine.test.ts | Adds unit tests for subscription transitions/entitlement rules. |
| packages/kit/convex/subscriptions/query.ts | Adds public subscription queries (status/entitlements/list/metrics). |
| packages/kit/convex/subscriptions/mutation.ts | Adds public mutation to bind purchaseToken → userId. |
| packages/kit/convex/subscriptions/internal.ts | Adds internal mutation to apply webhook events to subscriptions. |
| packages/kit/convex/schema.ts | Adds Convex tables for webhook events/idempotency, subscriptions, products, paywalls, metrics rollups. |
| packages/kit/convex/products/sync.ts | Adds internal sync helpers (upsertFromStore, markPushed, list drafts). |
| packages/kit/convex/products/query.ts | Adds product catalog query by apiKey (optional platform filter). |
| packages/kit/convex/products/play.ts | Adds Play Developer API push/pull sync action. |
| packages/kit/convex/products/mutation.ts | Adds public product upsert/remove mutations. |
| packages/kit/convex/products/jwt.ts | Adds minimal ASC ES256 JWT minter. |
| packages/kit/convex/products/jwt.test.ts | Adds tests for ASC JWT and DER→JOSE signature conversion. |
| packages/kit/convex/paywalls/query.ts | Adds paywall queries (get/list). |
| packages/kit/convex/paywalls/mutation.ts | Adds paywall mutations (upsert/delete). |
| packages/kit/convex/crons.ts | Registers hourly webhook event pruning cron. |
| packages/kit/convex/_generated/api.d.ts | Updates generated Convex API typings for new modules. |
| packages/gql/src/webhook-client.ts | Adds transport-agnostic JS SSE webhook client + parser. |
| packages/gql/src/webhook-client.test.ts | Adds unit tests for webhook SSE parsing + connection wrapper behavior. |
| packages/gql/src/kit-api.ts | Adds JS fetch wrapper for kit subscription/paywall endpoints. |
| packages/gql/src/kit-api.test.ts | Adds unit tests for kit-api wrapper behavior. |
| packages/gql/scripts/sync-to-platforms.mjs | Syncs webhook-client + kit-api to react-native-iap and expo-iap. |
| packages/gql/package.json | Exports webhook-client as a subpath export. |
| packages/docs/src/pages/docs/webhooks.tsx | Adds docs page explaining webhook stream architecture + usage snippets. |
| packages/docs/src/pages/docs/index.tsx | Adds docs routes for /docs/webhooks and /docs/kit-backend. |
| libraries/react-native-iap/src/webhook-client.ts | Adds RN copy of webhook-client (synced from packages/gql). |
| libraries/react-native-iap/src/kit-api.ts | Adds RN copy of kit-api (synced from packages/gql). |
| libraries/react-native-iap/src/index.ts | Re-exports webhook/kit helpers and new hook. |
| libraries/react-native-iap/src/hooks/useWebhookEvents.ts | Adds React hook that manages SSE stream + ring buffer. |
| libraries/react-native-iap/src/tests/hooks/useWebhookEvents.test.ts | Adds tests for the React hook behavior. |
| libraries/expo-iap/src/webhook-client.ts | Adds Expo copy of webhook-client (synced from packages/gql). |
| libraries/expo-iap/src/useWebhookEvents.ts | Adds Expo hook wrapper for webhook SSE stream. |
| libraries/expo-iap/src/kit-api.ts | Adds Expo copy of kit-api (synced from packages/gql). |
| libraries/expo-iap/src/index.ts | Re-exports webhook/kit helpers and new hook. |
| libraries/expo-iap/src/tests/useWebhookEvents.test.ts | Adds tests for Expo hook behavior. |
| libraries/kmp-iap/library/src/commonMain/.../WebhookTransport.kt | Adds expect/actual transport API for KMP SSE streaming. |
| libraries/kmp-iap/library/src/commonMain/.../WebhookClient.kt | Adds KMP webhook event parser + URL builder. |
| libraries/kmp-iap/library/src/androidMain/.../WebhookTransport.android.kt | Implements Android SSE transport via HttpURLConnection. |
| libraries/kmp-iap/library/src/iosMain/.../WebhookTransport.ios.kt | Implements iOS SSE transport via NSURLSession delegate. |
| libraries/kmp-iap/library/src/commonTest/.../WebhookClientTest.kt | Adds KMP parser + URL builder tests. |
| libraries/kmp-iap/library/build.gradle.kts | Sets -Xexpect-actual-classes compiler flag for KMP build cleanliness. |
| libraries/godot-iap/addons/godot-iap/webhook_client.gd | Adds Godot webhook stream client node with reconnect + Last-Event-ID. |
| libraries/flutter_inapp_purchase/test/webhook_client_test.dart | Adds Flutter webhook parser tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…he icon has padding instead of crowding the text
… + setup status
UX simplification + Horizon completion + clearer error signaling.
Unified webhook endpoint:
- New `POST /v1/webhooks/{apiKey}` route auto-detects Apple ASN v2
(`{signedPayload}`) vs Google Pub/Sub (`{message:{data,messageId}}`)
by inspecting the body shape and dispatches to the same Convex
action either platform-specific path uses. Operators paste one URL
into BOTH App Store Connect and Google Pub/Sub — whichever platform
isn't configured simply produces no traffic.
- Legacy `/apple/{apiKey}` and `/google/{apiKey}` aliases kept for
back-compat; both route through the same handler.
- Pre-dispatch setup check: if an Apple notification arrives but the
project is missing iosBundleId / iosAppAppleId / issuerId / keyId,
kit returns 412 with `code: "IOS_NOT_CONFIGURED"` listing the
missing fields. Same for Android (`ANDROID_NOT_CONFIGURED`). No
more silent drops — operators see exactly what's missing.
Setup status:
- New `convex/projects/setupStatus.ts::getSetupStatus(apiKey)` query
returns `{ found, ios, android, horizon }` with per-platform
`{ configured, missing[] }`. Used by the dashboard, the unified
webhook handler's pre-check, and downstream SDK error surfacing.
Horizon polling reconciler:
- `convex/subscriptions/horizon.ts` — Meta Horizon Store has no
webhook system, only the synchronous `verify_entitlement` Graph
API. The cron action walks every Horizon-enabled project's
Active / InGracePeriod / Paused subscriptions every 6h, calls
Meta Graph for each (userId, sku), and feeds deltas through the
same `applySubscriptionTransition` pipeline Apple/Google use.
- `horizonInternal.ts` exposes the queries / mutations the action
needs without dragging node-only imports into the Convex runtime.
- `reconcileHorizonNow` action + cron registration in `crons.ts`
(6h interval).
Dashboard Webhooks page:
- Single "Lifecycle webhook URL" card with copy-to-clipboard; paste
into both App Store Connect and Google Pub/Sub.
- Per-platform setup badges (iOS / Android / Horizon-polling) with
green check / amber warning + missing-field list.
- "Advanced — platform-specific URLs (legacy)" collapsible section
for operators with existing wiring.
- Live test curl recipe now points at the unified URL.
Verification:
- kit lint clean (0 errors); 281/281 vitest; smoke probes green.
- typecheck clean across kit + docs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ice units + Promise return types The closed PR #123 had 12 inline review comments from gemini-code-assist and coderabbitai. Fixing the substantive correctness issues here: RTDN numeric codes were swapped/incorrect: - Code 1 = SUBSCRIPTION_RECOVERED, code 4 = SUBSCRIPTION_PURCHASED (earlier draft had them reversed). Fixed in `convex/webhooks/shared.ts::GOOGLE_SUB_TYPE_MAP`, the unit-test expectations, the conformance scenarios, and `knowledge/external/webhook-mapping.md`. - Code 7 = SUBSCRIPTION_RESTARTED was incorrectly mapped to `SubscriptionRecovered`. RTDN docs define it as auto-renew re-enabled while the period is still active — that matches `SubscriptionUncanceled` semantics. Fixed in the map and added an explicit test case. - Code 11 = SUBSCRIPTION_PAUSE_SCHEDULE_CHANGED had its enum documentation under `SubscriptionResumed`. RTDN actually fires this when the pause schedule is updated; the real resume comes back as RECOVERED (1). Moved the doc under `SubscriptionPaused` and updated `webhook.graphql` + the mapping table. - Code 19 = SUBSCRIPTION_PRICE_CHANGE_UPDATED added as alias for the existing PRICE_CHANGE_CONFIRMED. Apple price unit terminology was wrong: - Apple's `signedTransactionInfo.price` is in **milliunits** (1/1000 of a currency unit), not "millicents". $9.99 is 9990 milliunits. Multiplier to micros is 1000×, not 10×. - Fixed `normalizeAppleAsn` (price * 10 → price * 1000), the terminology + link comment, the test fixture (999_000 → 9_990), and the `webhook-mapping.md` formula. `webhookEventsSince` query missing Promise<> wrap: - `Query.webhookEventsSince` was generating as `webhookEventsSince: WebhookEvent[]` instead of `Promise<WebhookEvent[]>`. The TS post-processor only wraps fields marked `# Future` in the schema and only scanned `api*.graphql` — `webhook.graphql` was excluded. - Added `# Future` comment in `webhook.graphql` and added `webhook.graphql` to `fix-generated-types.mjs`'s `schemaFiles`. Out of scope for this commit (deferred to follow-up): - Required-field fail-fast in generated `fromJson` / `from_dict` for Kotlin / Dart / GDScript / Swift. The codegen plugins currently default missing required fields to empty strings / zero / first enum value, which review correctly flagged as contract-violation hiding. Fixing requires plugin changes in `packages/gql/codegen/plugins/` for all four languages. Verification: - kit lint clean (0 errors); 281/281 vitest; smoke green. - gql 16/16 vitest; rn-iap 276/276 jest; expo-iap 46/46 jest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (26)
packages/kit/convex/products/jwt.ts-35-39 (1)
35-39:⚠️ Potential issue | 🟠 MajorValidate
ttlSecondsto prevent minting invalid ASC JWTs.Line 35 accepts any TTL without bounds checking. Values ≤ 0 or > 1200 violate App Store Connect API constraints (max 20 minutes) and will cause bearer token auth failures. Add validation before constructing the JWT.
Suggested fix
export function mintAscJwt(opts: AscJwtOptions): string { const ttl = opts.ttlSeconds ?? 600; + if (!Number.isInteger(ttl) || ttl <= 0 || ttl > 1200) { + throw new Error("ttlSeconds must be an integer between 1 and 1200"); + } const now = opts.nowSeconds ? opts.nowSeconds() : Math.floor(Date.now() / 1000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/products/jwt.ts` around lines 35 - 39, The code sets ttl = opts.ttlSeconds ?? 600 without bounds checking, so validate opts.ttlSeconds (and the computed ttl) before constructing the JWT: ensure ttl is a positive integer > 0 and ≤ 1200 (20 minutes) and throw a clear RangeError if it is out of range; reference the ttl variable and opts.ttlSeconds/opts.nowSeconds in your validation and perform this check early in the function that builds the ASC JWT so invalid TTLs are rejected rather than producing a token the App Store will refuse.packages/kit/src/pages/auth/organization/project/paywalls.tsx-41-70 (1)
41-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle mutation failures explicitly instead of fire-and-forget.
Both save and delete flows drop mutation promises from click handlers. If Convex rejects, this can surface as unhandled rejections and leaves users without clear failure feedback.
Suggested fix
+ const [actionError, setActionError] = useState<string | null>(null); + const onSubmit = async () => { + setActionError(null); const productIds = draft.productIds .split(",") .map((s) => s.trim()) .filter(Boolean); if ( !draft.slug || !draft.title || !draft.headline || productIds.length === 0 ) { return; } - await upsert({ - apiKey: project.apiKey, - slug: draft.slug, - title: draft.title, - layout: draft.layout, - productIds, - headline: draft.headline, - cta: draft.cta, - }); - setDraft({ - ...draft, - slug: "", - title: "", - productIds: "", - headline: "", - }); + try { + await upsert({ + apiKey: project.apiKey, + slug: draft.slug, + title: draft.title, + layout: draft.layout, + productIds, + headline: draft.headline, + cta: draft.cta, + }); + setDraft({ + ...draft, + slug: "", + title: "", + productIds: "", + headline: "", + }); + } catch { + setActionError("Failed to save paywall. Please try again."); + } };- <button - onClick={() => { - void remove({ - apiKey: project.apiKey, - slug: paywall.slug, - }); - }} + <button + onClick={async () => { + setActionError(null); + try { + await remove({ + apiKey: project.apiKey, + slug: paywall.slug, + }); + } catch { + setActionError("Failed to delete paywall. Please try again."); + } + }}Also applies to: 159-161, 205-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/pages/auth/organization/project/paywalls.tsx` around lines 41 - 70, The onSubmit handler currently fire-and-forgets the upsert mutation (and similarly the save/delete handlers referenced) which can produce unhandled rejections and no user feedback; wrap the mutation calls (e.g., the upsert call in onSubmit and the corresponding delete/save mutation calls) in async try/catch blocks, await the promise, and handle errors by setting local error state or showing a user-facing message/notification before returning, while preserving the existing success behavior (clearing draft via setDraft on success).packages/kit/convex/subscriptions/mutation.ts-31-38 (1)
31-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock reassignment of already-bound purchase tokens
The current branch allows replacing
sub.userIdwith a different user, which can let callers rebind ownership if they know a valid token. That’s a subscription-integrity/security risk.Proposed guard
if (sub.userId === args.userId) { return { ok: true, bound: true }; } + + if (sub.userId && sub.userId !== args.userId) { + return { ok: false, bound: false }; + } await ctx.db.patch(sub._id, { userId: args.userId, updatedAt: Date.now(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/mutation.ts` around lines 31 - 38, Add a guard to prevent reassigning a token that's already bound to another user: in the binding flow (the function handling the token/sub lookup where you check sub.userId and call ctx.db.patch), before calling ctx.db.patch check if sub.userId exists and is different from args.userId; if so, return a failure response (e.g., { ok: false, bound: true } or similar error) and do not call ctx.db.patch so ownership cannot be replaced by a caller who knows a token.libraries/react-native-iap/src/hooks/useWebhookEvents.ts-52-59 (1)
52-59:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
isConnectedcurrently means “has received an event,” not “stream opened.”The hook only flips
isConnectedinsideonEvent, so a healthy but idle SSE connection staysfalseforever.lastErroralso never clears after recovery, so the exposed state can remain stale even once events resume. This needs an explicit open/recovered signal fromconnectWebhookStream, or the public contract/docs need to be narrowed.Also applies to: 105-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/react-native-iap/src/hooks/useWebhookEvents.ts` around lines 52 - 59, The hook's isConnected and lastError semantics are wrong: isConnected is only set inside onEvent and never set when the SSE stream actually opens, and lastError is not cleared on recovery. Update connectWebhookStream (and any open/recover handlers it registers) to set isConnected = true when the SSE/stream opens (not just onEvent) and to clear lastError (set to null) when the stream successfully opens or recovers; keep isConnected false on close/error. Also ensure onEvent no longer toggles connection state alone (it may remain but should not be the only setter). Update UseWebhookEventsResult behavior accordingly so callers see an explicit open/recovered signal from connectWebhookStream rather than relying solely on onEvent.packages/kit/server/api/v1/products.ts-133-155 (1)
133-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep DELETE failures on the same JSON contract as the other routes.
This handler lets
removeProductexceptions escape as a generic 500, while the POST/sync routes return structured{errors:[...]}payloads. Wrapping the mutation intry/catchhere will keep SDK/MCP callers on stable error codes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/products.ts` around lines 133 - 155, The DELETE handler in products.delete currently lets exceptions from client.mutation(api.products.mutation.removeProduct) bubble up; wrap the mutation call in a try/catch so failures return the same structured JSON error contract used by POST/sync (i.e., c.json({ errors: [{ code: "...", message: "..." }] }, <status>)); catch the error from removeProduct, map it into a single errors array with an appropriate code (e.g., "INTERNAL_ERROR" or preserve known error codes if available) and message (include error.message), and return that JSON response instead of letting a raw 500 escape.packages/mcp-server/src/kit-client.ts-27-38 (1)
27-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a timeout to the shared kit HTTP wrapper.
Every MCP tool goes through
call(), so a stalled upstream connection can block the stdio server indefinitely. Please wrapfetchwith anAbortControllerand surface timeout failures asKitHttpError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp-server/src/kit-client.ts` around lines 27 - 38, The call function currently uses fetch without a timeout; wrap the fetch in an AbortController so requests time out and surface timeout failures as KitHttpError. Create an AbortController inside call(), merge its signal with any existing init.signal, start a setTimeout to call controller.abort() after a configured timeout (e.g., default or provided via init.headers/options), clear the timeout when fetch resolves, and await fetch with the controller.signal. If fetch throws with name === 'AbortError' or the timeout fired, throw a new KitHttpError indicating a timeout; for other fetch errors rethrow or wrap as appropriate. Reference: function call<T>, KitHttpError, and the existing init.headers/init.signal merging logic.libraries/kmp-iap/library/src/iosMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.ios.kt-61-88 (1)
61-88:⚠️ Potential issue | 🟠 MajorHandle transport errors in
didCompleteWithErrorand invalidate the session after each attempt.At lines 128–134, the
URLSession(session:task:didCompleteWithError:)delegate method ignores theerrorparameter entirely. This means transport failures (connection timeouts, DNS failures, etc.) are treated as successful completions, sorunOnce()always returnstrueand reconnects immediately without the intended 2-second backoff.Additionally, each
NSURLSessioncreated at line 78 is never invalidated, leaving delegate and session resources hanging across retries. Sessions must be invalidated withfinishTasksAndInvalidate()orinvalidateAndCancel()to break the delegate reference and prevent memory leaks.Current code (lines 61–88)
private suspend fun runOnce( channel: Channel<WebhookEvent>, lastEventId: String?, updateLastEventId: (String) -> Unit, ): Boolean = try { val url = NSURL(string = webhookStreamUrl(baseUrl, apiKey)) val request = NSMutableURLRequest.requestWithURL(url).apply { setHTTPMethod("GET") setValue("text/event-stream", forHTTPHeaderField = "Accept") setValue("no-cache", forHTTPHeaderField = "Cache-Control") if (lastEventId != null) { setValue(lastEventId, forHTTPHeaderField = "Last-Event-ID") } } val config = NSURLSessionConfiguration.defaultSessionConfiguration() val frameBuffer = StringBuilder() val delegate = SseDelegate(channel, frameBuffer, updateLastEventId) val session = NSURLSession.sessionWithConfiguration(config, delegate, null) val task = session.dataTaskWithRequest(request) activeTask = task task.resume() delegate.awaitFinished() true } catch (error: Throwable) { false } finally { activeTask = null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/kmp-iap/library/src/iosMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.ios.kt` around lines 61 - 88, runOnce currently ignores transport errors and never invalidates the NSURLSession, so make two fixes: (1) in SseDelegate's URLSession(session:task:didCompleteWithError:) implementation, check the error parameter and treat non-null errors as a failure (signal the awaiting coroutine/continuation so delegate.awaitFinished() completes with an error result that causes runOnce to return false) instead of silently succeeding; (2) ensure the NSURLSession created in runOnce (the session variable) is invalidated after the attempt by calling finishTasksAndInvalidate() or invalidateAndCancel() in the finally path (or immediately after delegate.awaitFinished()), so the delegate/session references are broken and resources are released; wire these changes so activeTask is still cleared and runOnce returns false on transport errors.packages/kit/server/api/v1/webhooks.ts-428-437 (1)
428-437:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
Last-Event-IDlookup can drop backlog after 500 stored events.This only scans
webhookEventsSince(apiKey, 0, 500). If the reconnectinglastEventIdfalls outside that slice, the fallbackDate.now()skips every event between the last delivered id and now. This needs a directid -> receivedAtlookup query, or a real cursor token, instead of a capped scan of history.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/webhooks.ts` around lines 428 - 437, The current lookup scans api.webhooks.query.webhookEventsSince with a hard limit (client.query(api.webhooks.query.webhookEventsSince, { apiKey, sinceMs: 0, limit: 500 })) and if lastEventId isn’t found falls back to Date.now(), which can drop events; replace this with a direct id lookup (e.g. add/use api.webhooks.query.webhookEventById or similar) to fetch the single event by id (using lastEventId) and return its receivedAt when found, and only then fall back to a safe alternative (not Date.now()) — update the code around client.query, the match logic, and remove the capped scan so the lookup reads the event by id (referencing lastEventId, client.query, and api.webhooks.query.webhookEventById/webhookEventsSince as needed).libraries/kmp-iap/library/src/androidMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.android.kt-50-53 (1)
50-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't reconnect forever on permanent HTTP failures.
Any non-2xx response is swallowed and retried after 2 seconds. For 401/403/404 that traps collectors in an infinite reconnect loop and hides bad API keys or route mistakes instead of failing fast. Only back off on transient transport failures; surface permanent 4xx responses as terminal errors.
Suggested direction
- if (connection.responseCode !in 200..299) { - throw IllegalStateException( - "SSE connect returned ${connection.responseCode}", - ) - } + when (val code = connection.responseCode) { + in 200..299 -> Unit + in 400..499 -> throw IllegalStateException( + "Permanent SSE failure: $code", + ) + else -> throw java.io.IOException("Transient SSE failure: $code") + } ... - } catch (error: Throwable) { + } catch (error: java.io.IOException) { if (closed) break // fall through to the back-off + reconnect. + } catch (error: IllegalStateException) { + throw error } finally {Also applies to: 71-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/kmp-iap/library/src/androidMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.android.kt` around lines 50 - 53, The code in WebhookTransport.android.kt currently treats any non-2xx HTTP response as a retryable transport error and will indefinitely reconnect; change the logic that checks connection.responseCode (in the WebhookTransport class, in the method that establishes the SSE connection where you currently throw IllegalStateException for non-2xx) to treat 4xx responses as terminal: if responseCode in 200..299 continue, if responseCode in 400..499 (explicitly including 401/403/404) throw a fatal/terminal exception (propagate or rethrow without scheduling a retry) so the collector fails fast, and only treat 5xx or network/transient failures as retryable (preserving the existing backoff/retry behavior). Apply the same change to both places noted (the blocks around the current checks at the two comment locations).packages/kit/server/api/v1/webhooks.ts-220-236 (1)
220-236:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when Google push auth isn't configured.
If
GOOGLE_PUBSUB_PUSH_AUDIENCEis unset, this route accepts any Google-shaped POST as long as the URL apiKey is known. That downgrades RTDN from authenticated push to an unauthenticated webhook. Require the audience in non-test environments, or hide the bypass behind an explicit insecure-dev flag.Suggested fix
const authHeader = c.req.header("authorization"); const audience = process.env.GOOGLE_PUBSUB_PUSH_AUDIENCE; - if (audience) { - const ok = await verifyPubSubOidcToken(authHeader, audience); - if (!ok) { - return c.json( - { - errors: [ - { - code: "UNAUTHORIZED", - message: "Pub/Sub OIDC verification failed", - }, - ], - }, - 401, - ); - } + if (!audience) { + return c.json( + { + errors: [ + { + code: "SERVER_MISCONFIGURED", + message: "GOOGLE_PUBSUB_PUSH_AUDIENCE is not configured", + }, + ], + }, + 500, + ); + } + + const ok = await verifyPubSubOidcToken(authHeader, audience); + if (!ok) { + return c.json( + { + errors: [ + { + code: "UNAUTHORIZED", + message: "Pub/Sub OIDC verification failed", + }, + ], + }, + 401, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/webhooks.ts` around lines 220 - 236, The current handler allows Google-shaped POSTs when GOOGLE_PUBSUB_PUSH_AUDIENCE is unset; change the logic in the webhook handler where `audience` is read and `verifyPubSubOidcToken(authHeader, audience)` is called so that in non-test environments the request is rejected if `GOOGLE_PUBSUB_PUSH_AUDIENCE` is not configured (or alternatively only allow the bypass when an explicit insecure dev flag is set). Concretely, update the block referencing `audience`, `GOOGLE_PUBSUB_PUSH_AUDIENCE`, and `verifyPubSubOidcToken` to: 1) check NODE_ENV !== "test" (or read a new INSECURE_DEV_ALLOW_PUBSUB_BYPASS flag) and if audience is missing return the same 401 JSON error, and 2) keep the existing OIDC verification path unchanged when `audience` exists so `authHeader` still uses `verifyPubSubOidcToken`. Ensure the new behavior is clearly gated by the test env or the explicit insecure flag.packages/kit/server/api/v1/webhooks.ts-463-470 (1)
463-470:⚠️ Potential issue | 🟠 MajorValidate Pub/Sub tokens against the configured push auth service account.
The
service-{PROJECT_NUMBER}@gcp-sa-pubsub.iam.gserviceaccount.com. The current suffix check rejects valid subscriptions configured with custom push auth service accounts. Comparepayload.emailto an expected configured service-account email instead. (docs.cloud.google.com)Suggested fix
const payload = ticket.getPayload(); if (!payload) { return false; } const email = payload.email; - // Pub/Sub push requests are signed by a Google service account - // dedicated to the publishing project. Reject any caller that is - // not from the gcp-sa-pubsub principal namespace. - if (!email || !email.endsWith("@gcp-sa-pubsub.iam.gserviceaccount.com")) { + const expectedEmail = process.env.GOOGLE_PUBSUB_PUSH_SERVICE_ACCOUNT; + if (!email || !expectedEmail || email !== expectedEmail) { return false; } return payload.email_verified === true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/webhooks.ts` around lines 463 - 470, The code currently validates Pub/Sub push tokens by checking payload.email endsWith the gcp-sa-pubsub suffix which rejects subscriptions using a user-managed push auth service account; update the check in the token validation (where payload/email is used) to compare payload.email exactly to the configured push-auth service account email (e.g., an env/config value like PUSH_AUTH_SERVICE_ACCOUNT or serverConfig.pubsubPushServiceAccount) instead of using endsWith, and return false if payload.email !== expectedServiceAccountEmail; keep the subsequent check that payload.email_verified === true. Ensure you reference and load the configured expected service-account email in the same module/function that performs this validation.packages/kit/convex/subscriptions/internal.ts-61-67 (1)
61-67:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGuard against stale events overwriting newer subscription state.
This mutation only ignores exact replays of the current
lastEventId. Any older event with a different id will still patch the row, so a lateExpired/Canceledcan roll back a subscription that was already moved forward by a newer webhook. Persist and compare a monotonic store timestamp/version (for example the webhookoccurredAt) before applying the transition.Also applies to: 97-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/internal.ts` around lines 61 - 67, The current check only skips exact replays by comparing existing.lastEventId to args.eventId, so older events with different ids can overwrite newer state; update mutation logic in the handler that reads/patches subscriptions (the block referencing existing, existing.lastEventId and args.eventId and the analogous block at lines 97-113) to persist and compare a monotonic event marker (e.g., webhook occurredAt or a numeric version) on the subscription document before applying transitions: read existing.occurredAt (or add occurredAt if missing), compare existing.occurredAt >= args.occurredAt and if so treat the incoming event as stale and return without applying the transition (keeping existing._id, active state via isActive(existing) and transition: null), otherwise apply the patch and update lastEventId and occurredAt to the incoming values; ensure both code paths (the initial guard and the later patch block referenced) use the same monotonic comparison to prevent late Expired/Canceled events from rolling state backward.packages/mcp-server/src/index.ts-316-341 (1)
316-341:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
openiap_simulate_webhooknever hits the authenticated Google success path.This POST sends only a JSON body, but the new Google receiver in this PR is explicitly authenticated via Pub/Sub OIDC. On a correctly configured deployment this tool can only exercise the unauthenticated failure branch, so the current description over-promises what it validates.
packages/kit/convex/subscriptions/horizon.ts-87-99 (1)
87-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpire grace-period subscriptions too, not just
Activeones.A failed Horizon entitlement check only records
SubscriptionExpiredwhen the row is exactlyActive. Rows inInGracePeriodstay stuck there forever even after Meta says the entitlement is gone, so the reconciler never converges them back to the authoritative inactive state.🧭 Suggested condition change
- } else if (!granted && probe.state === "Active") { + } else if ( + !granted && + (probe.state === "Active" || probe.state === "InGracePeriod") + ) {Also applies to: 179-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/horizon.ts` around lines 87 - 99, The reconciler only records "SubscriptionExpired" when probe.state === "Active", which leaves probes in "InGracePeriod" stuck; update the condition around the ctx.runMutation call to treat non-active-but-not-yet-expired states as expired (e.g., change the branch from !granted && probe.state === "Active" to !granted && (probe.state === "Active" || probe.state === "InGracePeriod") or simply !granted && probe.state !== "Expired") so that recordHorizonStatus is invoked with eventType "SubscriptionExpired" for grace-period rows as well; apply the same change to the other analogous block in this file that also calls internal.subscriptions.horizonInternal.recordHorizonStatus.libraries/godot-iap/addons/godot-iap/webhook_client.gd-172-192 (1)
172-192:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly advance
Last-Event-IDafter a valid event is emitted.
_last_event_idis updated before JSON parsing and required-field validation. If the stream delivers a malformed/truncated frame, this client treats it as acknowledged and reconnects after it, which can permanently drop that event.📌 Suggested fix
- if not event_id.is_empty(): - _last_event_id = event_id if data_lines.is_empty(): return if event_name == "heartbeat" or event_name == "ready": return @@ if not decoded.has("id") or not decoded.has("type") or not decoded.has("purchaseToken"): emit_signal("stream_error", "MALFORMED_EVENT", "WebhookEvent missing required fields") return + if not event_id.is_empty(): + _last_event_id = event_id emit_signal("event_received", decoded)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/godot-iap/addons/godot-iap/webhook_client.gd` around lines 172 - 192, The code currently sets _last_event_id from event_id before parsing and validating the SSE payload; change the logic so _last_event_id is only updated after a successful event emission: parse JSON with JSON.new().parse(data_str), verify err == OK, ensure decoded is a TYPE_DICTIONARY and has "id", "type", and "purchaseToken", then emit_signal("event_received", decoded) and only after that assign _last_event_id = event_id (or _last_event_id = decoded["id"] if you prefer the canonical id from payload). Do not update _last_event_id for heartbeats/ready, empty data_lines/data_str, parse errors, or malformed events (i.e., keep existing emit_signal("stream_error", ...) behavior without advancing _last_event_id).packages/kit/convex/subscriptions/internal.ts-186-190 (1)
186-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject rebinding a token that already belongs to another user.
Once a subscription is bound, this path lets any later caller replace
userIdwith a different value. On the public/bind-user/:apiKeyflow that can hijack entitlements or corrupt account linkage if a token is replayed. At minimum, only allow binding whensub.userIdis empty or already matches.🔒 Suggested guard
if (!sub) return null; if (sub.userId === args.userId) return sub._id; + if (sub.userId && sub.userId !== args.userId) { + throw new Error("Subscription is already bound to a different user"); + } await ctx.db.patch(sub._id, { userId: args.userId, updatedAt: Date.now(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/internal.ts` around lines 186 - 190, The current binding logic in internal.ts allows changing sub.userId unconditionally; update the guard around the patch so you only bind when the subscription is unclaimed or already owned: check sub.userId is null/undefined/empty or equals args.userId before calling ctx.db.patch(sub._id, ...); if sub.userId exists and differs from args.userId return or throw an error to reject the rebind (referencing variables sub, args.userId and the ctx.db.patch call).packages/mcp-server/src/index.ts-401-414 (1)
401-414:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
openiap_manage_productignores the requested action and can clobber product metadata.
disable,enable, andremoveall do the same thing here: upsert a placeholderSubscriptionwith an empty title, then echo the requested action back. That means the tool does not actually manage product state, and depending on server-side merge behavior it can also overwritetitle/typewith bogus values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp-server/src/index.ts` around lines 401 - 414, The openiap_manage_product flow is ignoring args.action and always calls client.upsertProduct with a bogus empty title and fixed type, which can clobber metadata; change the logic in the handler that uses withClient and client.upsertProduct to branch on args.action: for "remove" call the appropriate delete/remove method on the client (or mark removed) instead of upserting, and for "enable"/"disable" fetch the existing product (e.g., via a get/read method on the client using args.productId), modify only the state/flags needed, then upsert or update preserving existing title/type (do not pass an empty title or hardcoded type), and finally return the updated state with action echoed. Ensure you reference withClient, client.upsertProduct, args.action and args.productId when making these changes.libraries/godot-iap/addons/godot-iap/webhook_client.gd-77-87 (1)
77-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCustom HTTPS ports are parsed incorrectly.
For
https://host:8443/...,hoststill contains:8443, but the code keepsport = 443because colon parsing only runs in thehttp://branch. That makes non-default HTTPS kit deployments unreachable.🌐 Suggested fix
- var port := 443 - var use_ssl := true - if trimmed.begins_with("http://"): - port = 80 - use_ssl = false - var colon := host.find(":") - if colon >= 0: - port = int(host.substr(colon + 1)) - host = host.substr(0, colon) + var use_ssl := not trimmed.begins_with("http://") + var port := 443 if use_ssl else 80 + var colon := host.find(":") + if colon >= 0: + port = int(host.substr(colon + 1)) + host = host.substr(0, colon)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/godot-iap/addons/godot-iap/webhook_client.gd` around lines 77 - 87, The code only parses a host:port suffix inside the http branch, so HTTPS URLs like "https://host:8443" keep the default port 443 and retain ":8443" in host; update the parsing so the colon extraction runs regardless of scheme: after determining use_ssl and defaulting port (port := 443, use_ssl := true, then set port=80 and use_ssl=false if trimmed.begins_with("http://")), run the same colon detection logic (find ":" on host, int(host.substr(...)), host = host.substr(0, colon)) before calling _client.connect_to_host(host, port, TLSOptions.client() if use_ssl else null) so custom ports are honored for both HTTP and HTTPS.packages/kit/convex/products/play.ts-163-165 (1)
163-165:⚠️ Potential issue | 🟠 MajorRound-trip sync breaks consumable product types due to unmapped Play Store distinction.
Google Play's
InAppProduct.purchaseTypeonly definesmanagedUser(one-time) andsubscription—it does not distinguish consumable from non-consumable at the product level. The current implementation loses this distinction:
- Pull (line 207): All
managedUserproducts map toNonConsumable, making theConsumablereturn on line 208 unreachable.- Push (lines 163–164): Both
ConsumableandNonConsumablesend identical"managedUser"payload.Result: A local
Consumableproduct pushed to Play Store and pulled back becomesNonConsumable, breaking round-trip fidelity. Consumption behavior is handled at runtime via the Play Billing API, not in product metadata, so either accept losing this distinction locally or prevent syncingConsumablerows until a reversible strategy is defined.Also applies to: 204–208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/products/play.ts` around lines 163 - 165, The Play Store mapping loses the Consumable vs NonConsumable distinction: when pushing, both Consumable and NonConsumable are set to "managedUser", and when pulling, "managedUser" is always mapped to NonConsumable—so Consumable round-trips to NonConsumable. Update the push logic (the routine that builds the Play payload where purchaseType is set) to reject or skip local rows with type === "Consumable" (throw or return an error) instead of emitting a "managedUser" payload for them, and update the pull mapping routine (the function that converts Play InAppProduct to local product where "managedUser" is mapped) to leave a comment and preserve existing behavior for "managedUser" as NonConsumable; include a clear validation message referencing the local product type (e.g., in pushProductToPlay or buildPlayPayload) so Consumable products are not synced until a reversible strategy is implemented.libraries/kmp-iap/library/src/commonMain/kotlin/io/github/hyochan/kmpiap/openiap/Types.kt-4026-4045 (1)
4026-4045:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFail fast on missing required webhook enums instead of inventing defaults.
fromJsoncurrently turns absent/unknown webhook fields intoProduction/Ios/AppleAppStoreServerNotificationsV2/SubscriptionStarted. For a product/event model this critical, that masks protocol drift as a real business event and can corrupt downstream entitlement state. These required fields should throw when missing or unrecognized, with the generator updated accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/kmp-iap/library/src/commonMain/kotlin/io/github/hyochan/kmpiap/openiap/Types.kt` around lines 4026 - 4045, In WebhookEvent.companion.fromJson stop silently defaulting unknown/missing enum fields and instead fail fast: for each enum field (environment -> WebhookEventEnvironment, platform -> IapPlatform, source -> WebhookEventSource, type -> WebhookEventType) call the corresponding .fromJson on the raw string and if it returns null or the raw value is missing throw an IllegalArgumentException (or similar) with a clear message (e.g. "missing/unknown webhook field 'environment': <rawValue>") so callers can detect protocol drift; update the construction in fromJson to require these non-null enum results rather than using Production/Ios/AppleAppStoreServerNotificationsV2/SubscriptionStarted defaults.libraries/kmp-iap/library/src/commonMain/kotlin/io/github/hyochan/kmpiap/openiap/Types.kt-3959-4023 (1)
3959-4023:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd
sourceNotificationIdto the generatedWebhookEventcontract.The normalized webhook shape exercised in this PR already includes
sourceNotificationId, but this KMP model drops it entirely. That leaves webhook subscribers/backfill consumers without the store notification identifier used for dedupe/reconciliation, even though it is part of the server-side event contract. Because this file is generated, please fix this in the GraphQL/schema/codegen source rather than patchingTypes.ktby hand.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/kmp-iap/library/src/commonMain/kotlin/io/github/hyochan/kmpiap/openiap/Types.kt` around lines 3959 - 4023, The generated WebhookEvent Kotlin data class is missing the sourceNotificationId field (used for store notification dedupe/reconciliation); update the GraphQL/schema/codegen source so the WebhookEvent type includes sourceNotificationId (nullable String?) and regenerate types rather than editing Types.kt directly; locate the WebhookEvent definition in your GraphQL schema or codegen mapping and add sourceNotificationId: String (nullable) so the generated WebhookEvent data class includes val sourceNotificationId: String? = null alongside the other fields.packages/kit/convex/subscriptions/query.ts-168-182 (1)
168-182:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe unfiltered path caps both
itemsandtotalat 500 rows.
take(500)runs beforetotalis computed, so projects with more than 500 subscriptions will undercount totals and silently hide older records regardless of the requestedlimit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/query.ts` around lines 168 - 182, The current branch uses .take(500) on ctx.db.query("subscriptions").withIndex("by_project_and_updated", ...) which truncates rows before computing total and slices items, causing undercounting for projects with >500 subscriptions; change the logic so total is computed from the full filtered result (including any args.productId filter) before applying a cap, e.g. remove or defer .take(500) when computing total and only limit the items returned with .take(limit) or slice after computing total; update the block that builds rows (the query on "subscriptions", the args.productId filter, the final return that maps shapeRow and uses limit) so total = rows.length of the un-capped set and items are the capped subset mapped via shapeRow.packages/kit/convex/subscriptions/query.ts-152-180 (1)
152-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply
userIdeven whenstateis present.If callers pass both filters, the
statebranch wins anduserIdis never applied, so the dashboard returns every user in that state instead of the requested user.Suggested fix
if (args.state) { rows = await ctx.db .query("subscriptions") .withIndex("by_project_and_state", (q) => q.eq("projectId", project._id).eq("state", args.state!), ) .order("desc") .collect(); } else if (args.userId) { rows = await ctx.db .query("subscriptions") .withIndex("by_project_and_user", (q) => q.eq("projectId", project._id).eq("userId", args.userId), ) .order("desc") .collect(); } else { rows = await ctx.db .query("subscriptions") .withIndex("by_project_and_updated", (q) => q.eq("projectId", project._id), ) .order("desc") .take(500); } + if (args.userId) { + rows = rows.filter((row) => row.userId === args.userId); + } + if (args.productId) { rows = rows.filter((row) => row.productId === args.productId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/subscriptions/query.ts` around lines 152 - 180, The current branch logic picks the state-indexed query when args.state is present and never applies args.userId, so add an additional filter step after any fetch to enforce userId when provided: after the rows are loaded from ctx.db.query("subscriptions") (e.g., the block using withIndex("by_project_and_state") and collect()), check if args.userId and then do rows = rows.filter(r => r.userId === args.userId); keep the existing optimized branch for the sole-userId case (withIndex("by_project_and_user")) but always apply the in-memory userId filter when state is used.packages/kit/convex/webhooks/shared.ts-601-644 (1)
601-644:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
subscriptionStateunset for Google one-time notifications.
oneTimeProductNotificationcurrently flows throughderiveGoogleSubscriptionState, soONE_TIME_PRODUCT_PURCHASEDnormalizes tosubscriptionState: "Active". That violates this module’s own contract (subscriptionStateis subscription-only) and will make one-time purchases look like subscriptions downstream.Suggested fix
const environment: WebhookEventEnvironment = payload.testNotification ? "Sandbox" : "Production"; + + const subscriptionState = payload.oneTimeProductNotification + ? undefined + : deriveGoogleSubscriptionState(type, subscriptionInfo ?? null); return { type, source: "GooglePlayRealTimeDeveloperNotifications", platform: "Android", environment, purchaseToken, productId, - subscriptionState: deriveGoogleSubscriptionState( - type, - subscriptionInfo ?? null, - ), + subscriptionState, expiresAt: subscriptionInfo?.expiryTimeMillis, renewsAt: subscriptionInfo?.autoRenewingPlanRenewsTimeMillis, cancellationReason: mapGoogleCancellationReason( type, subscriptionInfo ?? null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/webhooks/shared.ts` around lines 601 - 644, The code sets subscriptionState for one-time purchases because deriveGoogleSubscriptionState is always called; to fix, only compute and return subscriptionState for subscription RTDNs (not for payload.oneTimeProductNotification). Modify the return so subscriptionState is undefined/null when payload.oneTimeProductNotification is present (or when productId/type indicates a one-time purchase) and only call deriveGoogleSubscriptionState(subscriptionInfo) when handling subscription notifications (e.g., when payload.subscriptionNotification or subscriptionInfo exists); reference deriveGoogleSubscriptionState, payload.oneTimeProductNotification, subscriptionState, and mapGoogleOneTimeNotificationType to locate where to gate the call and the returned field.packages/kit/convex/products/sync.ts-37-57 (1)
37-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against cross-platform overwrites when
productIdmatches.Both mutations query by
(projectId, productId)and then patch the found row. Since sync is called from both iOS and Android pipelines, a sharedproductIdcan silently mutate the wrong platform row (markPushedcurrently ignoresargs.platformentirely).Minimal safety fix (prevents silent corruption)
handler: async (ctx, args) => { const existing: Doc<"products"> | null = await ctx.db @@ .unique(); const now = Date.now(); + if (existing && existing.platform !== args.platform) { + throw new Error( + `Platform mismatch for productId=${args.productId}: existing=${existing.platform}, incoming=${args.platform}`, + ); + } if (existing) { @@ handler: async (ctx, args) => { const existing = await ctx.db @@ .unique(); - if (!existing) return null; + if (!existing || existing.platform !== args.platform) return null; + const now = Date.now(); await ctx.db.patch(existing._id, { storeRef: args.storeRef, state: "Ready", - syncedAt: Date.now(), - updatedAt: Date.now(), + syncedAt: now, + updatedAt: now, });Also applies to: 88-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/products/sync.ts` around lines 37 - 57, The current upsert logic queries products via withIndex("by_project_and_product") and patches the found existing row regardless of args.platform, allowing cross-platform overwrites; update the logic so you either include platform in the DB query (add .eq("platform", args.platform) to the withIndex call or create/use an index like "by_project_product_platform") or, if you keep the existing query, check that existing.platform === args.platform before calling ctx.db.patch(existing._id, ...); if there is a platform mismatch, treat it as a missing row (create a new product row) or return an error instead of patching. Apply the same guard/fix where similar code appears (the block around lines 88-101) and update markPushed to include platform in its query/index usage as well.libraries/react-native-iap/src/webhook-client.ts-207-233 (1)
207-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate enum fields before returning
{ kind: "ok" }.
parseWebhookEventDatacurrently accepts any stringtypeand then casts the object toWebhookEventPayload. Unknowntype/platform/environmentvalues can slip through as valid events and break downstream logic.Suggested hardening
+const WEBHOOK_EVENT_TYPES: ReadonlySet<WebhookEventType> = new Set([ + "SubscriptionStarted", + "SubscriptionRenewed", + "SubscriptionExpired", + "SubscriptionInGracePeriod", + "SubscriptionInBillingRetry", + "SubscriptionRecovered", + "SubscriptionCanceled", + "SubscriptionUncanceled", + "SubscriptionRevoked", + "SubscriptionPriceChange", + "SubscriptionProductChanged", + "SubscriptionPaused", + "SubscriptionResumed", + "PurchaseRefunded", + "PurchaseConsumptionRequest", + "TestNotification", +]); + export function parseWebhookEventData(raw: string): ParsedEventResult { @@ - const event = parsed as WebhookEventPayload; + const event = parsed as Record<string, unknown>; if ( + !WEBHOOK_EVENT_TYPES.has(event.type as WebhookEventType) || + (event.platform !== "IOS" && event.platform !== "Android") || + (event.environment !== "Production" && + event.environment !== "Sandbox" && + event.environment !== "Xcode") || typeof event.id !== "string" || typeof event.purchaseToken !== "string" || typeof event.occurredAt !== "number" || typeof event.receivedAt !== "number" ) { @@ - return { kind: "ok", event }; + return { kind: "ok", event: event as WebhookEventPayload }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/react-native-iap/src/webhook-client.ts` around lines 207 - 233, The parseWebhookEventData path currently only checks basic field types then casts to WebhookEventPayload, allowing arbitrary strings for enum-like fields; update the validation inside parseWebhookEventData to explicitly verify that event.type, event.platform and (if present) event.environment are one of the known allowed values (the project's WebhookEventPayload/type enum set and platform/environment sets) and return a { kind: "error", message: ... } if any of those values are unknown, only returning { kind: "ok", event } when all enum checks pass.
🟡 Minor comments (8)
packages/docs/src/pages/docs/index.tsx-1120-1121 (1)
1120-1121:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd sidebar links for the new docs pages.
/docs/webhooksand/docs/kit-backendare registered routes, but they aren’t discoverable from the sidebar menu in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/src/pages/docs/index.tsx` around lines 1120 - 1121, The new routes Route(path="webhooks", element={<Webhooks />}) and Route(path="kit-backend", element={<KitBackend />}) are not exposed in the sidebar; add corresponding sidebar entries (e.g., add items for "Webhooks" and "Kit Backend") to the docs sidebar links array or component used by the page (look for the sidebarLinks/docsSidebar/Sidebar component near the top of this file) so those routes become discoverable and point to "/docs/webhooks" and "/docs/kit-backend".packages/gql/scripts/sync-to-platforms.mjs-141-163 (1)
141-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast when runtime sync sources are missing.
At Line 144 and Line 154, missing
webhook-client.tsorkit-api.tsis silently ignored. That can leave stale files in RN/Expo and mask broken releases. Make these checks hard-fail (or at least explicit warnings + non-zero exit) for these newly required runtime modules.Suggested change
-if (existsSync(webhookClientSource)) { - for (const target of [rnWebhookClientTarget, expoWebhookClientTarget]) { - mkdirSync(dirname(target), { recursive: true }); - copyFileSync(webhookClientSource, target); - } - console.log('✅ webhook-client → react-native-iap + expo-iap'); - console.log(` ${rnWebhookClientTarget}`); - console.log(` ${expoWebhookClientTarget}\n`); -} +if (!existsSync(webhookClientSource)) { + throw new Error(`Missing required sync source: ${webhookClientSource}`); +} +for (const target of [rnWebhookClientTarget, expoWebhookClientTarget]) { + mkdirSync(dirname(target), { recursive: true }); + copyFileSync(webhookClientSource, target); +} +console.log('✅ webhook-client → react-native-iap + expo-iap'); +console.log(` ${rnWebhookClientTarget}`); +console.log(` ${expoWebhookClientTarget}\n`); -if (existsSync(kitApiSource)) { - for (const target of [rnKitApiTarget, expoKitApiTarget]) { - mkdirSync(dirname(target), { recursive: true }); - copyFileSync(kitApiSource, target); - } - console.log('✅ kit-api → react-native-iap + expo-iap'); - console.log(` ${rnKitApiTarget}`); - console.log(` ${expoKitApiTarget}\n`); -} +if (!existsSync(kitApiSource)) { + throw new Error(`Missing required sync source: ${kitApiSource}`); +} +for (const target of [rnKitApiTarget, expoKitApiTarget]) { + mkdirSync(dirname(target), { recursive: true }); + copyFileSync(kitApiSource, target); +} +console.log('✅ kit-api → react-native-iap + expo-iap'); +console.log(` ${rnKitApiTarget}`); +console.log(` ${expoKitApiTarget}\n`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gql/scripts/sync-to-platforms.mjs` around lines 141 - 163, The current sync silently skips missing runtime sources (webhookClientSource, kitApiSource) which can leave stale copies; change the logic so that if webhookClientSource or kitApiSource does not exist you log an explicit error (including the missing path like webhookClientSource / kitApiSource) and exit non‑zero (e.g., process.exit(1) or throw) instead of silently continuing; keep the existing copy logic for the exists cases (targets: rnWebhookClientTarget, expoWebhookClientTarget, rnKitApiTarget, expoKitApiTarget) but add the fail-fast check before the for-loops.libraries/react-native-iap/src/__tests__/hooks/useWebhookEvents.test.ts-159-186 (1)
159-186:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest intent and assertions are slightly out of sync.
The case says it “keeps the listener alive” but never verifies a post-error event is still processed. Add one
fire(JSON.stringify(...))afteronerrorand assertonEvent/eventsupdates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/react-native-iap/src/__tests__/hooks/useWebhookEvents.test.ts` around lines 159 - 186, The test simulates an onerror but never verifies the listener remains active; after calling stream.onerror in the test for HookProbe, fire one more event via the fake stream (use the stream.fire or makeFakeStream's event emitter) with JSON payload and then assert that HookProbe's onEvent or its exported events array was updated (e.g., check (HookProbe as any).events or onEvent mock called with the parsed event), ensuring the listener processed the post-error event while still asserting lastError and onError as currently done.packages/kit/src/pages/auth/organization/project/subscriptions.tsx-110-120 (1)
110-120:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose selected filter state to assistive tech
The filter chips behave like toggles but don’t expose pressed state. Add
aria-pressed(and explicittype="button") so screen-reader users can identify the active filter.Proposed change
<button + type="button" key={option.id} onClick={() => setFilter(option.id)} + aria-pressed={filter === option.id} className={`px-3 py-1 rounded-full text-xs font-medium transition-colors ${ filter === option.id ? "bg-primary text-primary-foreground"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/pages/auth/organization/project/subscriptions.tsx` around lines 110 - 120, The filter chip buttons (rendered in the map using option.id/option.label) should explicitly set type="button" and expose their toggled state via aria-pressed so assistive tech can detect the active filter; update the button in the map that calls setFilter(option.id) to include type="button" and aria-pressed={filter === option.id} while keeping the existing className and onClick behavior.libraries/expo-iap/src/__tests__/useWebhookEvents.test.ts-108-131 (1)
108-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnmount renderer in the transport-error test
This test never unmounts the hook instance, so effect cleanup is not guaranteed and the stream close path is left unverified for this scenario.
Proposed change
it('reports transport errors via onError without unmounting', () => { const {stream} = makeFakeStream(); const factory = jest.fn(() => stream); const onError = jest.fn(); + let renderer: ReturnType<typeof ReactTestRenderer.create> | null = null; ReactTestRenderer.act(() => { - ReactTestRenderer.create( + renderer = ReactTestRenderer.create( React.createElement(HookProbe, { apiKey: 'k', baseUrl: 'http://localhost', eventSourceFactory: factory as any, onError, }), ); }); ReactTestRenderer.act(() => { stream.onerror?.(new Error('disconnect')); }); expect(onError).toHaveBeenCalledWith( expect.objectContaining({code: 'TRANSPORT_ERROR'}), ); + + ReactTestRenderer.act(() => { + renderer?.unmount(); + }); + expect(stream.close).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/expo-iap/src/__tests__/useWebhookEvents.test.ts` around lines 108 - 131, The test never unmounts the HookProbe instance so the effect cleanup (stream close) isn't exercised; change the ReactTestRenderer.create call to capture its return value (e.g., const renderer = ReactTestRenderer.create(...)) when creating HookProbe in the test for transport errors, then after invoking stream.onerror inside ReactTestRenderer.act call renderer.unmount() (also inside act) and add an assertion that the fake stream's close method was called (e.g., expect(stream.close).toHaveBeenCalled()) to verify the cleanup path in HookProbe.packages/kit/server/api/v1/subscriptions.ts-43-55 (1)
43-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReuse the same
userIdvalidation across all subscription routes.
/statusrejects IDs over 256 chars, but/entitlementsand/bind-useraccept them unchanged. That means the same logical key can pass one endpoint and fail deeper in another. Pull the bound check into a shared helper and apply it here too.Also applies to: 94-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/subscriptions.ts` around lines 43 - 55, The /entitlements/:apiKey handler currently validates presence of userId but not length, causing inconsistent behavior with /status; extract the userId checks into a shared helper (e.g., validateUserId or ensureValidUserId) that enforces non-empty and max-length (256) rules, use it in subscriptions.get("/entitlements/:apiKey") and in the other subscription routes (including the /bind-user and /status handlers referenced around lines 94-110) and return the same error shape when validation fails so all subscription endpoints enforce the same userId constraints.packages/kit/server/api/v1/products.ts-15-19 (1)
15-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject unknown
platformvalues instead of widening the query.Lines 15-19 currently turn
?platform=foointoplatform: undefined, so the handler returns the full catalog rather than a 400. That hides caller bugs and can expose both platforms when the client thought it was filtered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/server/api/v1/products.ts` around lines 15 - 19, The current code reads platformParam from c.req.query and silently maps any unknown value to platform = undefined, which unintentionally returns the full catalog; instead validate platformParam and reject unknown values: if platformParam is empty set platform = undefined, but if platformParam is present and not exactly "IOS" or "Android" return a 400 error (with a clear message) from the handler; update the logic around platformParam/platform in products.ts to perform this explicit validation and early return rather than falling back to undefined.libraries/flutter_inapp_purchase/lib/types.dart-5452-5458 (1)
5452-5458:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTransport doc is inconsistent with the current webhook stream implementation.
The comment says “served over WebSocket”, but this webhook stream path is implemented as SSE in this PR context. Please update the schema/source docs so generated SDK docs don’t point integrators to the wrong transport.
Based on learnings: Never edit
libraries/flutter_inapp_purchase/lib/types.dartby hand; update schema/source and regenerate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries/flutter_inapp_purchase/lib/types.dart` around lines 5452 - 5458, Update the transport line in the webhook stream doc comment that currently reads "Transport: kit serves this over WebSocket" to correctly state the transport is Server-Sent Events (SSE) — e.g., change to "Transport: kit serves this over Server-Sent Events (SSE)". Do not edit libraries/flutter_inapp_purchase/lib/types.dart directly; instead modify the upstream schema/source that generates the doc for the webhook stream (the docblock starting "Streams normalized webhook events tied to the authenticated client's purchases.") and regenerate the generated files so SDK docs reflect the correct transport.
…ers, dedupe SubscriptionState Three CI checks were red on PR #124. Local verification + fixes for all three: 1. **Kit Typecheck + Build (Docker frozen-lockfile)**: bun.lock at the workspace root was out of sync with `packages/kit/package.json` after the recent `google-auth-library` add. Re-ran `bun install` so the root lock matches; verified locally with the same `bun install --frozen-lockfile --filter @hyodotdev/openiap-kit` the Docker step runs. 2. **kmp-iap Compile Check**: my `Subscription.webhookEvent` / `Query.webhookEventsSince` GraphQL fields meant the codegen made them required interface methods on the device-side IAP class, so `InAppPurchaseAndroid` failed to compile (missing `suspend fun webhookEventsSince` / `webhookEvent`). Plus my hand- written `WebhookEvent` data class collided with the generated `Types.kt` one. - Removed both fields from `webhook.graphql`. The webhook stream is a kit-server feature served over SSE (`/v1/webhooks/stream/{apiKey}`), not a GraphQL transport — the spec note in `webhook.graphql` documents the call-site contract instead. - Rewrote `WebhookClient.kt` to use the generated `WebhookEvent` data class + every enum from the generated `Types.kt`, with the parser falling back through the generated `fromJson` factories (KMP codegen emits PascalCase / SCREAMING_SNAKE / kebab-case aliases). 3. **Flutter Analyze (`ambiguous_export`)**: `SubscriptionState` was defined in both `lib/enums.dart` (hand-written legacy) and `lib/types.dart` (auto-generated from `webhook.graphql`). - Removed the hand-written enum from `enums.dart` (verified zero in-tree usages); the generated one is now the single source. - Rewrote `lib/webhook_client.dart` to use the generated `WebhookEvent.fromJson` with a fallback that rewrites enum fields by their `.name` to the codegen wire format (kebab-case). Drops the duplicated `WebhookEventTypeName` enum I had hand-defined. - Updated `test/webhook_client_test.dart` accordingly: unknown event types now correctly return null (PR #123 review's fail- fast expectation) instead of mapping to a synthetic `Unknown`. Cascading cleanup: - Updated `packages/docs/src/pages/docs/webhooks.tsx` Kotlin / Dart examples from `WebhookEventTypeName.subscriptionRenewed` to the generated `WebhookEventType.SubscriptionRenewed`. - Re-ran codegen + sync; generated `Types.swift` / `Types.kt` / `types.dart` / `types.gd` / `types.ts` no longer carry the webhook Query / Subscription typings. Local verification (matches CI): - kit lint clean (0 errors); 281/281 vitest; smoke green. - `bun install --frozen-lockfile --filter @hyodotdev/openiap-kit` clean. - KMP `./gradlew :library:compileDebugKotlinAndroid` BUILD SUCCESSFUL. - KMP `./gradlew :library:testDebugUnitTest` BUILD SUCCESSFUL. - Flutter `flutter analyze` no issues, `flutter test test/webhook_client_test.dart` 3/3 pass. - react-native-iap 276/276 jest, expo-iap 46/46 jest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two pieces of CI guardrail so the same kind of green-locally-red-in-CI loop doesn't repeat: 1. **bun version pin**: PR #124's repeated lockfile-frozen failures in the kit Docker step were because local bun was 1.3.0 while the Docker image pins 1.3.13 — bun lockfiles aren't stable across versions, so the locally-passing `--frozen-lockfile` would still fail in Docker. - Bumped `package.json` `packageManager` to `bun@1.3.13`. - Regenerated root `bun.lock` with bun 1.3.13. - Added a pre-commit guard that compares `bun --version` against the pinned version and refuses to commit on mismatch with a one-liner pointing at `bun upgrade`. 2. **pre-commit path-aware extensions**: - `libraries/flutter_inapp_purchase/{lib,test}/` edits now trigger `flutter analyze` (catches the `ambiguous_export` class of failure that just took out CI). Skipped with a warning if `flutter` isn't on PATH. - `libraries/kmp-iap/library/src` or `packages/gql/src` edits trigger `./gradlew :library:compileDebugKotlinAndroid` (catches the redeclaration / missing-interface-member errors). Skipped with a warning if `gradlew` isn't executable. Net effect: the next contributor (or the next me) can't push a kit / flutter / kmp / gql change that fails CI for any of the categories of issue we've already burned a CI run on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #124 still failed after the previous lockfile pin because: 1. **Lockfile drift across bun versions**: GitHub Actions workflows (`ci.yml`, `deploy-kit.yml`, `dependabot-bun-lockfile.yml`) pinned `bun-version: 1.3.0`, but `packages/kit/Dockerfile` pins `oven/bun:1.3.13-slim`. So even after I regenerated the lockfile with 1.3.13 (matching Docker), the `Test GQL Types` job ran with 1.3.0 which sees drift. - Bumped all three workflow files to `bun-version: 1.3.13` so every CI step + Docker uses the same bun. 2. **62 lint errors after the bun bump**: bun 1.3.13 resolved a newer `@typescript-eslint/eslint-plugin` that ships `no-unnecessary-type-assertion` enabled. All 62 errors were pre-existing (unrelated to this PR's changes) but newly surfaced after the dependency upgrade. Ran `eslint --fix` which auto-resolved every one cleanly. 3. **Prettier reformat ripple**: a few of the auto-fixed files needed prettier reflow after the type-assertion drops. Re-ran prettier on the affected files; pre-commit gate stays green. Local verification: - `packages/kit/`: lint 0 errors, 281/281 vitest, smoke green, prettier clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Dockerfile's deps stage copies each workspace package.json so bun can plan the install graph against the root lockfile. PR #124 added `packages/mcp-server` as a new workspace but didn't update the Dockerfile, so bun saw the lockfile reference an unknown workspace member and rejected `--frozen-lockfile` with "lockfile had changes, but lockfile is frozen". Adding the COPY line for mcp-server. Local verification reproducing exactly what CI runs (root bun + workspace filter): bun install --frozen-lockfile --filter @hyodotdev/openiap-kit passes cleanly with no changes after the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…solves bun 1.3.13 + workspaces installs some deps (vite, @vitejs/plugin-react) under `packages/kit/node_modules/` instead of fully hoisting them to the workspace root. The Dockerfile's builder stage was only copying `/app/node_modules` from deps, so `bun run build:all` failed with `vite: command not found` even though the install step succeeded. Adding a second COPY for `packages/kit/node_modules` after the source copy (so it isn't clobbered by `COPY packages/kit ./packages/kit`). Tested locally with the same `bun install --frozen-lockfile --filter @hyodotdev/openiap-kit` flow — kit/node_modules contains `.bin/vite`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a unified server-side webhook normalization and streaming architecture, providing real-time subscription lifecycle events for iOS, Android, and Meta Horizon via Server-Sent Events (SSE). The changes span the entire stack, including new Convex actions for webhook ingestion, incremental subscription stats maintenance, and updated client SDKs for Expo, React Native, Flutter, Godot, and KMP. Feedback highlights a bug in MRR calculation during product upgrades where the previous billing period is not correctly resolved, a missing coroutine cancellation check in the Android KMP transport that causes shutdown delays, and an incorrect identifier comparison in the Google RTDN enrichment logic.
- packages/kit/convex/webhooks/google.ts: drop the `latestSuccessfulOrderId === notificationToken` match. The two ids are not interchangeable — `latestSuccessfulOrderId` is a GPA Order ID, the notification carries a Play purchaseToken. The longest-`expiryTime` heuristic (now the primary path) was already what selected the line item in practice. - libraries/kmp-iap/library/src/androidMain/.../WebhookTransport.android.kt: check `currentCoroutineContext().isActive` in the generic Throwable catch so cancellation-induced SocketException tears the loop down immediately instead of falling through to delay(2_000) and keeping the transport alive an extra 2s. - packages/kit/convex/subscriptions/internal.ts: compute the BEFORE stats contribution against the OLD product's billing period. Upgrade / downgrade events flip `productId`, so reusing the new product's period to subtract the BEFORE delta normalized the wrong monthly amount and corrupted the incremental MRR counter. New `beforeBillingPeriod` does a second fetchBillingPeriod when the productId actually changed; otherwise reuses the AFTER value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request implements a unified server-side lifecycle webhook system and a hosted product catalog sync for the OpenIAP kit. It introduces normalized WebhookEvent ingestion for Apple and Google stores, a robust subscription state machine, and incremental metrics aggregation within the Convex backend. The changes also include SSE-based webhook clients and hooks across all SDKs, expanded documentation, and enhanced pre-commit validation. Feedback suggests refining the error message for iOS subscription group requirements to clarify kit's auto-creation behavior and using ConvexError for invalid service account credentials to ensure appropriate HTTP status codes are returned to Google Pub/Sub.
- webhooks/google.ts: wrap the service-account JSON.parse in a
try/catch and throw ConvexError("INVALID_SERVICE_ACCOUNT_JSON") on
failure. A SyntaxError from a malformed upload would otherwise
reach the route layer as an un-mapped exception and 500 the
Pub/Sub push, which Google retries forever on a permanent config
error. ConvexError → mapWebhookError default branch → 400 so
Pub/Sub gives up and the operator sees the structured code.
- products/mutation.ts: extend the subscriptionGroupName error
message to clarify that kit's push-sync creates the group in App
Store Connect on first push and reuses an existing group with the
same name — operator does not need to create the group in ASC
manually first.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive server-side lifecycle webhook system and a unified product catalog push-sync mechanism across all supported platforms (iOS, Android, Meta Horizon). Key additions include a hosted SSE stream for real-time normalized WebhookEvent delivery, incremental subscription stats maintenance with paginated recompute for scalability, and automated product synchronization between the kit backend and store consoles. The framework SDKs (Expo, React Native, Flutter, Godot, KMP) have been updated with new hooks and clients to consume these events. Feedback highlights a potential memory leak in the Google Play client cache and a robustness issue in the webhookEventsSince pagination logic when handling high-frequency event bursts within a single millisecond.
- webhookEvents schema + webhookEventsSince query: add a composite
`(projectId, receivedAt, _creationTime)` index and switch the SSE
backfill query to a two-phase walk on it. Phase 1 (only when
`afterCreationTime` is set) exhausts the boundary-millisecond tail
past the cursor via the index's `gt("_creationTime", ...)`; Phase
2 walks the post-boundary range via `gt("receivedAt", ...)`.
Removes the in-memory boundary filter that previously dropped
pages silently when a single-millisecond burst exceeded the
fetchLimit cap of 5_000.
- google.ts playClientCache: bound the per-project Play client cache
to PLAY_CLIENT_CACHE_MAX_ENTRIES=100 with insertion-order LRU
eviction. Convex action containers are reused across projects, so
the prior unbounded Map would have leaked memory in the long-
running Node process on multi-tenant deployments. Hot projects
stay cached via re-set-on-hit; cold ones get evicted via
trimPlayClientCacheLru after each insert.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a unified webhook normalization and streaming system, enabling client SDKs to react to subscription lifecycle events via Server-Sent Events (SSE). It includes backend receivers for Apple and Google notifications, incremental metrics maintenance, and platform-specific transport implementations. Feedback identifies opportunities to improve the robustness of these systems, such as allowing Android push-sync to recover from partial failures, broadening HTTP success checks in the Godot client, and ensuring proper error mapping in the Google RTDN receiver to prevent unnecessary retries.
- products/sync.ts: drop the `storeRef === undefined` filter from
`listDraftAndroidProducts`. The iOS path filters on `state ===
"Draft"` only; the prior Android-extra guard was added to skip
Pull-imported rows, but it also blocked partial-sync resumption
for Android Drafts whose create succeeded but later steps
failed. play.ts's push loop now branches on `row.storeRef` at the
top and PATCHes existing storeRefs (added in earlier round), so
both pull-then-push and partial-sync cases are handled correctly
without the extra filter.
- libraries/godot-iap/addons/godot-iap/webhook_client.gd: accept
any 2xx (200-299) instead of exactly 200. Brings parity with the
Flutter / KMP / expo SSE clients fixed in earlier rounds — kit
returns 200 today but the SSE spec and intermediate proxies allow
201/202/204 success codes too.
- webhooks/google.ts: throw `ConvexError({code:"UNSUPPORTED_EVENT",
...})` for `UnknownEventType` instead of a plain Error. The route
layer's `mapWebhookError` (webhooks.ts:788) translates that
ConvexError code to a 200 ACK so Pub/Sub stops retrying; the
prior plain-Error path 500-ed and triggered Google's exponential
retry on payloads kit will never accept. Matches the Apple
path's ConvexError shape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request implements a unified webhook normalization and streaming system across all platforms, including a hosted backend for Apple ASN v2 and Google RTDN, a polling reconciler for Meta Horizon, and SSE client implementations in all SDKs. It also introduces significant improvements to the product sync logic for App Store Connect and Google Play, adhering to the repository's platform-specific naming conventions. The review feedback identifies several instances where truthy checks on priceAmountMicros could lead to zero-price tiers being incorrectly skipped during the push-sync process, recommending explicit undefined checks instead.
All four sites checked `row.priceAmountMicros &&` (truthy), which silently skips the price-setting step when a kit row has `priceAmountMicros === 0`. While most paid IAPs are non-zero in practice, a zero-tier (intro / promo / freemium ladder) configured in the dashboard would have been pushed without its price. Switched to `row.priceAmountMicros !== undefined` everywhere: - products/play.ts:497 (inappproducts.patch path) - products/play.ts:598 (inappproducts.insert create path) - products/asc.ts:1385 (subscription price-schedule push) - products/asc.ts:1508 (one-time IAP price-schedule push) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
packages/kit/convex/products/play.ts (1)
452-467:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPlay subscription sync is still missing required request fields, and create can report
Readytoo early.
monetization.subscriptions.patchrequiresregionsVersion, andmonetization.subscriptions.createrequires bothproductIdandregionsVersion. The Play docs also say newly added base plans remain draft untilbasePlans.activate, so marking the row pushed immediately after create can still leave a non-purchasable subscription reported asReady. In the current shape this path will either 400 on the API call or declare success before the SKU is actually sellable. (developers.google.com)Also applies to: 539-577, 617-628
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/convex/products/play.ts` around lines 452 - 467, The patch/create calls to androidpublisher.monetization.subscriptions (methods androidpublisher.monetization.subscriptions.patch and .create) are missing required fields and can mark SKUs as pushed prematurely; update the request bodies to include regionsVersion (and include productId in create), and after creating a subscription do not mark the DB row pushed/Ready until you confirm a sellable base plan by calling basePlans.activate and verifying activation (or polling the subscription/base plan state) — modify the code paths where subscriptions are created (monetization.subscriptions.create) and patched (monetization.subscriptions.patch) to add regionsVersion, ensure create includes productId, and move the "mark pushed" logic to run only after basePlans.activate succeeds (or after an explicit check that the subscription is active/purchasable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libraries/expo-iap/example/app/webhook-stream.tsx`:
- Around line 197-201: The FlatList keyExtractor currently builds keys using the
array index which causes keys to change when new events are prepended; update
the key generation in keyExtractor (used in the FlatList component) to return a
stable unique identifier using item.id (e.g., return item.id as a string) so
rows keep stable keys and avoid remounting when events are prepended.
- Around line 56-73: The UI remains stuck at "connecting" until the first event
arrives; after calling connectWebhookStream(...) you should mark the stream as
connected immediately. After assigning listenerRef.current =
connectWebhookStream({...}), call setStatus('connected') and clear status
message via setStatusMessage(null) (keep the existing onEvent behavior intact),
so the banner reflects a healthy active connection as soon as the listener is
created.
In `@libraries/godot-iap/addons/godot-iap/webhook_client.gd`:
- Around line 70-77: The loop currently emits a "TRANSPORT_ERROR" whenever
_open_and_drain() returns false even during intentional shutdown; modify
_run_loop so the emit_signal("stream_error", ...) is only called when the loop
is still meant to be running (e.g., guard with if not ok and _running then
emit_signal(...)) or introduce/use an explicit shutdown flag (set by
close_stream()/ _exit_tree()) and only emit when not shutting down; update
references in _run_loop (and set the flag in close_stream/_exit_tree if you add
one) and keep the reconnect_delay_seconds sleep/stop behavior unchanged.
- Around line 106-154: The loop that checks _client.get_status() (in the
connect/request sequence inside _open_and_drain/_run_loop) treats any
non-CONNECTED/REQUESTING/BODY status as a simple false return, which lets the
outer reconnection logic retry forever; update the connection and request status
checks to explicitly detect HTTPClient.STATUS_CANT_RESOLVE,
HTTPClient.STATUS_CANT_CONNECT, and HTTPClient.STATUS_TLS_HANDSHAKE_ERROR (and
any other terminal HTTPClient statuses) and, when seen,
emit_signal("stream_error", "HTTP_CLIENT_FATAL", "<brief message>") (or reuse
the "stream_error" pattern), set _running = false to stop retries, and return
false so the caller surfaces the terminal failure instead of infinite
reconnects. Ensure these checks appear where _client.get_status() is evaluated
after the connect loop and after the request loop (around the existing
STATUS_CONNECTING/STATUS_RESOLVING, STATUS_REQUESTING, and STATUS_BODY checks)
and include the response_code branch behavior for non-2xx as already
implemented.
In
`@libraries/kmp-iap/library/src/androidMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.android.kt`:
- Around line 97-99: The current check in WebhookTransport.android.kt that sets
closed = true whenever code in 400..499 is too broad and treats transient client
errors as terminal; update the condition around the variable closed (and the
HTTP status variable code) so only non-recoverable 4xx statuses mark the
transport closed (e.g., explicitly list terminal codes such as 400, 401, 403,
404, 410 or a configured set) and do NOT treat transient ones like 408 or 429 as
terminal so reconnect/retry logic can still run.
In `@packages/kit/convex/subscriptions/internal.ts`:
- Around line 103-119: The billing period resolution currently ignores
subscription platform and may pick the wrong SKU; update calls to
fetchBillingPeriod to pass the subscription's platform (use next.platform and
existing.platform as appropriate) when computing billingPeriod and
beforeBillingPeriod (i.e., replace fetchBillingPeriod(ctx, args.projectId,
productId) with fetchBillingPeriod(ctx, args.projectId, platform, productId)),
and update any recompute cache keys to include platform as part of the key
(platform + productId) so products are looked up by (projectId, platform,
productId) consistently.
In `@packages/kit/convex/webhooks/google.ts`:
- Around line 341-360: The code that parses uploaded service-account JSON
creates and throws a ConvexError on malformed JSON but a surrounding catch path
later swallows all errors and returns null; update the outer catch(s) that
handle parsing/validation (the blocks surrounding the JSON.parse and the similar
logic around lines handling service account parsing) to detect and re-throw
structured errors by doing if (err instanceof ConvexError) throw err; otherwise
continue with the existing fallback handling so permanent config errors surface
as 4xx instead of being silently converted to null. Ensure you apply this same
change to both the parse block around credentials and the second parsing block
referenced (the block around 454-495).
---
Duplicate comments:
In `@packages/kit/convex/products/play.ts`:
- Around line 452-467: The patch/create calls to
androidpublisher.monetization.subscriptions (methods
androidpublisher.monetization.subscriptions.patch and .create) are missing
required fields and can mark SKUs as pushed prematurely; update the request
bodies to include regionsVersion (and include productId in create), and after
creating a subscription do not mark the DB row pushed/Ready until you confirm a
sellable base plan by calling basePlans.activate and verifying activation (or
polling the subscription/base plan state) — modify the code paths where
subscriptions are created (monetization.subscriptions.create) and patched
(monetization.subscriptions.patch) to add regionsVersion, ensure create includes
productId, and move the "mark pushed" logic to run only after basePlans.activate
succeeds (or after an explicit check that the subscription is
active/purchasable).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ab1c6be-be5b-4c4a-89ab-f87c418e2476
📒 Files selected for processing (13)
libraries/expo-iap/example/app/webhook-stream.tsxlibraries/flutter_inapp_purchase/example/lib/src/screens/subscription_flow_screen.dartlibraries/godot-iap/addons/godot-iap/webhook_client.gdlibraries/kmp-iap/library/src/androidMain/kotlin/io/github/hyochan/kmpiap/openiap/WebhookTransport.android.ktpackages/kit/convex/products/mutation.tspackages/kit/convex/products/play.tspackages/kit/convex/products/sync.tspackages/kit/convex/schema.tspackages/kit/convex/subscriptions/horizonInternal.tspackages/kit/convex/subscriptions/internal.tspackages/kit/convex/subscriptions/stats.tspackages/kit/convex/webhooks/google.tspackages/kit/convex/webhooks/query.ts
✅ Files skipped from review due to trivial changes (1)
- packages/kit/convex/subscriptions/horizonInternal.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libraries/flutter_inapp_purchase/example/lib/src/screens/subscription_flow_screen.dart
- packages/kit/convex/products/mutation.ts
- packages/kit/convex/products/sync.ts
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive cross-platform webhook system and the 'kit' hosted backend. Key changes include normalized event mapping for Apple and Google, SSE-based streaming clients for all SDKs, a Meta Horizon entitlement reconciler, and updated documentation. It also adds toolchain version guards, security-focused .gitignore updates, and strict guidelines for addressing PR feedback. Feedback was provided to improve the readability of the base64 encoding logic in the Expo example by extracting it into a dedicated helper function.
CodeRabbit review 4215920557 — 7 inline + 1 duplicate.
Backend correctness:
- webhooks/google.ts: re-throw structured ConvexErrors (e.g.
INVALID_SERVICE_ACCOUNT_JSON) before the outer "transient API
failure" catch swallows them. Permanent config errors now surface
as 4xx so Pub/Sub stops retrying instead of silently degrading
to "no enrichment".
- subscriptions/internal.ts: thread `platform` through
`fetchBillingPeriod` and look up exactly (projectId, platform,
productId). The prior iOS-preferred walk made an Android sub
inherit the iOS billing period when the same SKU shipped on both
stores with different periods, skewing MRR.
- subscriptions/stats.ts: re-key the recompute period map by
`${platform}:${productId}` for the same reason.
KMP / Godot transport hardening:
- WebhookTransport.android.kt: stop treating every 4xx as terminal.
408 / 425 / 429 are transient and should reconnect on back-off;
only the permanent codes (401/403/404/410/412/422) flip the
closed flag.
- godot-iap/webhook_client.gd: detect HTTPClient terminal statuses
(CANT_RESOLVE / CANT_CONNECT / TLS_HANDSHAKE_ERROR), emit a
`HTTP_CLIENT_FATAL` signal, and stop retries — a misconfigured
hostname or broken TLS cert no longer triggers an infinite 2s
reconnect loop. Also skip the `TRANSPORT_ERROR` signal during
intentional shutdown so `close_stream()` / `_exit_tree()` don't
look like a failure.
Demo screen polish:
- expo-iap webhook-stream.tsx: set status to `connected` as soon as
`connectWebhookStream` returns instead of waiting for the first
event (a healthy idle stream was stuck in `connecting`); use
`item.id` directly as keyExtractor so prepended events don't
remount existing rows.
Play API correctness (duplicate finding):
- packages/kit/convex/products/play.ts: add the required
`regionsVersion.version` ("2022/01") param to both
monetization.subscriptions.patch and monetization.subscriptions
.create — the v3 API 400s without it. After create, also call
monetization.subscriptions.basePlans.activate so the just-created
plan is actually purchasable; Play's v3 API leaves new base plans
in DRAFT regardless of the `state` field on the create payload,
so the prior code marked the row Ready while the upstream SKU
was still non-purchasable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #124 Gemini review (3177680890): the inline `btoa(unescape(encodeURIComponent(...)))` is opaque at the call site. Extract into a `base64EncodeUtf8` helper with a docstring explaining why the wrapper is needed (Buffer is a Node global; Hermes/JSC don't ship it). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CR review 4215920557 duplicate finding ( |
Handle typed SSE events in JS clients, move kit live-tail delivery to a cursor-drained wake-up flow, and select subscription status from active/latest rows instead of index order. Also fix MCP webhook snippets and clear existing kit lint warnings before commit.
|
@hyochan nice work! Seems that "Sync with App Store Connect" function does not work even if the right AppStore Connect API Team Key is added. Always same error but Apple shows that the key was used. |
|
@LukasB-DEV the fix from #127 is now deployed to kit.openiap.dev — would you mind retrying "Sync with App Store Connect" when you have a moment? 🙏 The sync now runs as a background job (returns immediately, polls progress reactively in the dashboard) so iOS Safari won't abort it on long catalogs. There's also a Cancel button if anything looks stuck, and a dry-run / Reset-local-cache for safer recovery. If you hit anything weird, Discord is the fastest way to ping me: https://discord.com/invite/5AQd8BbxWT Thanks again for the original report! |
Summary
Turns kit into a full hosted backend so apps using openiap don't need their own server. Wires up:
WebhookEventshape and persisted with idempotency for replay/dedup.@hyodotdev/openiap-mcp-server, 10 tools).Surface map
POST /v1/webhooks/{apiKey}— unified ingest; auto-detects Apple ASN v2 vs Google Pub/Sub by payload shape. Legacy/apple/{apiKey}+/google/{apiKey}aliases retained.GET /v1/webhooks/stream/{apiKey}— SSE stream powered by ConvexonUpdatereactive subscribe (no polling).GET /v1/subscriptions/{status,entitlements,list,metrics,bind-user}/{apiKey}.GET/POST/DELETE /v1/products/{apiKey}+POST /v1/products/{apiKey}/sync/{ios|android}.getSetupStatusquery →412 IOS_NOT_CONFIGURED/ANDROID_NOT_CONFIGUREDerrors when notifications arrive for an unconfigured platform.Auth model
GOOGLE_PUBSUB_PUSH_AUDIENCE, gated to the*@gcp-sa-pubsub.iam.gserviceaccount.comprincipal. apiKey in the path resolves the project.Idempotency
(projectId, source, sourceNotificationId)wheresourceNotificationIdisnotificationUUIDfor ASN v2 / Pub/SubmessageIdfor RTDN.projectIdscoping prevents cross-project messageId collisions.deduped: trueand a 200 ACK so Apple / Pub/Sub stop retrying.pruneWebhookEventscron.Test plan
bun run --filter @hyodotdev/openiap-kit typecheck— 0 errorsbun run --filter @hyodotdev/openiap-kit test— 331/331 passingbun run --filter @hyodotdev/openiap-kit smoke:server— server compiles + boots, all probes greenbun run audit:docs— no new failures./gradlew :library:compileDebugKotlinAndroid+testDebugUnitTest— BUILD SUCCESSFULflutter analyzeno issues, webhook tests 3/3convex/webhooks/conformance.test.ts) — 6 multi-step lifecycle scenarios cover the full receiver → state machine → entitlement pipeline against canned ASN v2 + RTDN payloads🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores