feat: add mobile onboarding and StoreKit billing - #255
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d764e838e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (auth.status === 'signedIn' && !workspaceReady && !isExplore) { | ||
| return <Redirect href="/explore" /> |
There was a problem hiding this comment.
On a cold start or deep link to Photos/Map/Studio, useMobileOnboarding() is still idle/loading, so isMobileWorkspaceReady() returns false even for an already configured workspace. This branch immediately redirects signed-in users to Explore, and once readiness later becomes ready there is no navigation back to the original/default tab, so valid signed-in sessions lose their intended route.
Useful? React with 👍 / 👎.
| } | ||
| const config = this.photoStorage.mapProviderToStorageConfig(resolvedProvider) | ||
| const storage = StorageFactory.createProvider(config) | ||
| await this.withTimeout(storage.getFile(`.afilmory-connection-test/${randomUUID()}`)) |
There was a problem hiding this comment.
For S3-compatible/B2/GitHub-style providers, getFile() returns null for a missing object and in several failure cases rather than throwing; because the probe intentionally reads a random path and ignores the result, invalid credentials, bucket names, or permissions can still be reported as { connected: true }. That lets the handoff page save a broken storage provider and complete onboarding even though subsequent photo storage operations will fail.
Useful? React with 👍 / 👎.
| if (products.appStoreProductId?.trim()) { | ||
| const productId = products.appStoreProductId.trim() | ||
| await this.upsertProduct(offerId, 'app_store', 'sandbox', productId) | ||
| await this.upsertProduct(offerId, 'app_store', 'production', productId) |
There was a problem hiding this comment.
When an App Store product id is changed or cleared in system settings, this sync only upserts the new id and never deletes or deactivates the old billing_offer_product row; listAppStoreOffers() and transaction reconciliation both read from that table, so removed products can remain visible/purchasable and still grant the old offer. The sync should also prune provider mappings that are no longer present in the current settings.
Useful? React with 👍 / 👎.
The billing module had accumulated 25 flat files mixing four unrelated concerns, two competing sources of truth for tenant plans, and the Creem reconciliation pipeline living inside auth.provider.ts. Boundaries: - Move mobile onboarding and storage handoff out of billing into a new platform/mobile module; neither touches billing tables. Routes unchanged. - Extract the Creem webhook pipeline from auth.provider.ts (569 -> 372 lines) into billing/providers/creem; auth now only spreads the plugins the billing module hands it. - Split billing into catalog/entitlement/providers/plan/usage. BillingError lives at the billing root so the entitlement core does not depend on the provider adapters. - Rename BillingReconciliationService to CreemBillingService; it only ever handled Creem. Single source of truth: - Drop the legacy creemSubscriptions read path from StoragePlanService; tenants.storagePlanId is written solely by the entitlement projection, so a resolved plan already implies a live grant. - Delete the projection-bypassing writers that no longer had callers: BillingPlanService.updateTenantPlan, StoragePlanService.updateTenantPlan / updateCurrentTenantPlan / assignPlanToTenant, and the tenant repository's updatePlan / updateStoragePlan. Deduplication: - Unify the twice-written provider-event idempotency into BillingProviderEventService. - Extract sha256Hex into @afilmory/be-utils (was inlined in three services), centralise the storage setting keys, move isByoStorageActive to the storage domain, and replace the repeated auth/activeTenantId controller boilerplate with requireSessionIdentity / requireActiveTenantIdentity. Read paths: - synchronizeConfiguredProducts no longer runs on every offer lookup; a listener drives it from module init and the system.setting.updated event. Errors: - Replace the bare Error throws with a typed BillingError whose code is persisted to billing_provider_event.error_code, so the stored code no longer depends on message truncation. BizException remains for caller-facing errors. photo-asset.service.ts and data-sync.service.ts only needed an import path update, but touching them put them through lint-staged for the first time: they carry the resulting autofix plus a Buffer import and hoisted regex literals for the rules autofix could not resolve. Behaviour note: a provider-event receipt failure now throws instead of being silently skipped on the Creem path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c40cad09e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| products.offer_id, | ||
| 'creem'::"billing_provider", | ||
| COALESCE(NULLIF(subscriptions.creem_subscription_id, ''), 'record:' || subscriptions.id), | ||
| 'legacy', |
There was a problem hiding this comment.
For tenants that already have active Creem subscriptions when this migration runs, storing the new billing row with environment = 'legacy' means later Creem webhooks are reconciled under getCreemEnvironment() (production or test) and create/update a separate billing_subscription row. If that later live row expires or is revoked, the active entitlements tied to the migrated legacy row are never inactivated, so existing customers can keep paid application/storage grants after cancellation. Migrate these rows into the environment used by reconciliation or explicitly retire the legacy row when processing the first live event.
Useful? React with 👍 / 👎.
| sourceId: `superadmin:${params.tenantId}`, | ||
| tenantId: params.tenantId, | ||
| value: dto.storagePlanId, |
There was a problem hiding this comment.
When a super-admin clears storage for a tenant that had storage_plan_id before migration 0020, this only targets sourceId = superadmin:<tenantId>. The migration seeded those existing manual storage grants under migration:tenant:<tenantId>, and setManualGrant(... value: null) inserts no replacement, so projection still selects the old migrated grant and the API returns success while tenants.storagePlanId remains unchanged. Clearing should also deactivate the migrated/manual grant for this tenant/kind, or use a shared source id.
Useful? React with 👍 / 👎.
…scope P1 — account deletion tombstoned the wrong billing subjects, in both directions. billing_subject.tenant_id cascades from tenants, so the tombstone write ran after the tenant delete had already removed the row: - A deleted workspace never got its tombstone, and the tombstoned branch in reconcileVerifiedTransaction was unreachable. Apple's later renewals resolved to no subject, raised APP_STORE_BILLING_SUBJECT_NOT_FOUND, returned 5xx, and Apple retried them indefinitely while billing_provider_event filled with failures. - A transferred workspace did get tombstoned, even though it outlives its former owner. The successor then hit "This billing subject is no longer available" on every purchase, with no path to clear the flag. Transferred workspaces now release their subject instead: the owner is cleared and the App Store account token is rotated, which also detaches the departing owner's Apple identity from the workspace. Deleted workspaces keep relying on the cascade, and an unattributable notification is now accepted and dropped rather than retried, since retrying it could never succeed. P2 — the storage handoff capability cookie derived Secure from req.url, which is plain http behind a TLS-terminating proxy, so the cookie carrying a capability that can write tenant storage credentials shipped without Secure. Four other call sites already read x-forwarded-proto; this adds isSecureRequest so there is one place to get it right. P2 — StoreKitBillingFinishGate was decorative: its only call site passed isVerified: true, serverAcknowledged: true as literals, so the guard was always satisfied, while StoreKitBillingPolicyTests asserted the false cases the caller never produced. The real ordering lives in JS and was correct but untested. Removed the gate and its tautological test, moved the acknowledgement flow into storeKitAcknowledgement.ts behind an injectable port, and covered the invariant with tests that fail if finish is ever reached without a matching server acknowledgement. Removing two .swift files requires a pod install before the next native build. Note: the P1 fix depends on tenant cascade behaviour and has no automated coverage; this repo has no DB-backed integration setup and a pure-function test here would only restate the branch condition.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0f41c7b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (const [storagePlanId, products] of Object.entries(storageProducts)) { | ||
| const definition = storageCatalog[storagePlanId] | ||
| if (!definition || !products) { |
There was a problem hiding this comment.
When a storage plan is marked inactive but its product mapping is left configured, this branch still calls ensureOffer(), which writes the offer back with isActive: true; listAppStoreOffers() and Creem reconciliation then treat that disabled storage plan as purchasable/grantable. The sync should skip or deactivate offers for definition.isActive === false so disabling a plan in the catalog actually removes it from billing flows.
Useful? React with 👍 / 👎.
| kind: 'application_plan', | ||
| sourceId: `superadmin:${params.tenantId}`, | ||
| tenantId: params.tenantId, | ||
| value: dto.planId as BillingPlanId, |
There was a problem hiding this comment.
When a super-admin selects the existing Free option, this stores an active manual application_plan entitlement with value free; manual entitlements outrank subscription entitlements in selectEffectiveEntitlement(), so any later App Store or Creem subscription for this tenant can still project planId as free until the row is manually removed. Treating Free as null/removing the manual grant preserves the default free fallback without blocking paid subscriptions.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
Mobile previously depended on users entering an existing tenant and had no supported storage configuration path. A dismissed onboarding sheet could also leave an incomplete account without a clear way to resume. Managed storage purchases must remain inside Apple's in-app purchase system, while connecting storage the user already owns needs a separate Web configuration surface without external payment messaging.
User and developer impact
workspace_requiredand ownerstorage_requiredsteps are native non-dismissible form sheetsValidation
pnpm -C apps/mobile type-checkpnpm -C apps/mobile bundlepnpm -C be/apps/core buildpnpm -C be/apps/dashboard buildDetailed evidence:
docs/superpowers/reports/2026-08-05-mobile-onboarding-storekit-local-verification.mdExternal follow-up
Apple Sandbox/TestFlight purchase, renewal, refund, revocation, and App Store Server Notifications V2 validation still require App Store Connect products and credentials.