fix(kilo-pass): emit kilo_pass_purchase_completed server-side, once per sale - #4892
Merged
Conversation
… result Slice 1 of kilo-pass-event-audit: server becomes the sole emitter of kilo_pass_purchase_completed. Adds trackKiloPassPurchaseCompleted (discriminated app_store/stripe params, snake_case wire properties, never throws) and runAfterResponse (Next.js after() for provider webhooks). CompleteStoreKiloPassPurchaseResult is now a discriminated union carrying purchaseKind (initial/renewal/upgrade) only when alreadyProcessed is false.
Slice 3 of kilo-pass-event-audit: the server (completeAppStorePurchase / Apple server notifications / Stripe invoice.paid) is now the sole emitter. The client-only event never fired on recovery/restore completions (RC1) and was lossy across app suspend (RC3). started/failed client events are unchanged. The negative test imports the real event constant so knip keeps the export used; posthog.ts itself is owned by a parallel section.
…e.paid Slice 2b of kilo-pass-event-audit: the Stripe web channel never emitted the event at all (RC2). Emits at most once per successfully handled invoice, anchored on the absence of a prior KiloPassInvoicePaidHandled/ Success audit row, checked in-transaction after the subscription upsert (whose row lock serializes same-subscription redeliveries) and before this run's own Success append. Emission is post-commit via runAfterResponse (Next.js after()) as the first post-commit statement. purchase_kind maps from invoice.billing_reason only; $0 invoices emit; blocked duplicate-card purchases never emit.
…l sites Slice 2a of kilo-pass-event-audit: recovery/restore/ASSN completions never emitted the event (RC1). completeAppStorePurchase tracks fire-and-forget (user-facing mutation; claw_trial_started precedent); the Apple server-notifications completion tracks post-commit via runAfterResponse (provider webhook durability). Both gate on alreadyProcessed === false, which the kilo_pass_store_purchases unique constraint on (payment_provider, provider_transaction_id) enforces across replays, races, and retries.
An implementer ran Prettier on its owned files, adding ~3000 lines of reformat churn; oxfmt's expanded-object heuristic made the churn unrecoverable by formatting. Records the revert-and-redo recovery and the handoff/diff-size prevention pins.
…assignment The let-outside/assign-inside pattern leaves completionResult narrowed to null in straight-line flow; a widening 'as ... | null' on the post-commit copy resets control-flow analysis. Also replace the union-illegal purchaseKind destructure in the idempotency test with an explicit-field expectation.
… SUT oxlint consistent-type-imports forbids the import() type annotation used to type the dynamic-import handle.
Sync-point root pnpm format rewrite.
50%-worker cold starts exceed the 60s per-worker setup hook and poison the whole run; two concurrent runs collide on JEST_WORKER_ID and DROP each other's databases. Documents JEST_MAX_WORKERS=1 with no concurrent DB runs as the local shape.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 3 Issues Found (all carried forward, author-declined) | Recommendation: Address before merge Executive SummaryThe incremental commit Overview
Issue Details (click to expand)WARNING
SUGGESTION
Resolved since the previous review
Incremental scope reviewed (1 file)
Diff reviewed: Assumptions: no tests, typechecks, or builds were run (read-only review); the two carried-forward production findings were re-verified by reading the current code at HEAD ( Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit c7c1967)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit c7c1967)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe at-most-once gating is sound on both channels, but the tRPC App Store call site emits without Overview
Issue Details (click to expand)WARNING
SUGGESTION
Verified clean (checked, no finding)
Files Reviewed (14 files)
Assumptions: no tests, typechecks, or builds were run (read-only review); type-level conclusions are based on repo-internal precedent, not an executed Fix these issues in Kilo Cloud Previous reviewStatus: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe emission logic itself (at-most-once gating on both channels) holds up under review; the blocking issue is a TypeScript error in the App Store notifications test that jest cannot catch but Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Verification notes and assumptions
Files Reviewed (14 files)
Reviewed by claude-opus-5 · Input: 32 · Output: 6.2K · Cached: 709.4K Review guidance: REVIEW.md from base branch |
…de kind Kilobot CRITICAL: the import-type alias did not propagate contextual typing (TS7006 downstream); a type-only namespace import + typeof member satisfies both tsgo and oxlint consistent-type-imports. Kilobot WARNING: the DID_CHANGE_RENEWAL_PREF UPGRADE case asserted purchaseKind against the recorded call's own value (vacuous); the fixture is a same-period tier19 -> tier49 upgrade, so assert 'upgrade' concretely.
Contributor
Author
|
(bot) @kilocode-bot please review |
Kilobot: without proof the completion callback ran, the negative assertion passes vacuously. Assert the onCompleted spy fired (the callback where the client capture used to live) before asserting no completed event was captured.
Contributor
Code Review SummaryReviewed at: Overview
Previously flagged items — re-verified
"Once per sale" guarantees — verified
WARNING
Two distinct problems in one place:
I acknowledge the author declined (1) as an accepted residual matching the SUGGESTION
Optional follow-up (not this PR): Stripe at-most-once currently rests on lock-order reasoning rather than a constraint. A partial unique index on Also checked, no finding
Verification performed (and what I could not run)
|
Contributor
|
The review is already posted. Summary: 1 new WARNING (tRPC emit call at No further action needed unless you want me to spawn a follow-up Cloud Agent session to fix the tRPC try/catch issue. |
jeanduplessis
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What:
kilo_pass_purchase_completedis now emitted server-side, at most once per completed sale, at exactly three call sites — tRPCcompleteAppStorePurchase, Apple App Store Server Notifications completion, and StripehandleKiloPassInvoicePaid— and the mobile client stops emitting it. Newpurchase_kind/channel(and per-channel) properties make the series filterable.Why: the event undercounted actual Kilo Pass sales. The data team flagged that the PostHog count sits below real sales; the investigation found four root causes (below). The count stepping up to the true number is the fix, not an anomaly.
Investigation narrative (root causes, with evidence)
Sales are recorded on the server across two channels and five code paths; the event was emitted only on one lossy client-side path.
onPurchaseCompleted(apps/mobile/src/lib/kilo-pass/use-store-kilo-pass-purchase.ts:503), reachable only whenonPurchaseSuccessmatchedactivePurchaseRequestRef.current?.sku === purchase.productId. That ref is in-memory: if the app was backgrounded/killed/restarted between Apple's payment sheet succeeding and JS handling the success, the purchase completed later via theavailablePurchasesrecovery effect (:577) orrecoverPurchases— both passnotifyCompletion: false, so the backend recorded the sale and no event fired.KiloPassSubscribeCard.tsx) and the Stripe webhook flow (stripe-handlers-invoice-paid.ts) contained no PostHog emission at all. Both channels feedkilo_pass_subscriptions, so "passes sold" excluding Stripe was guaranteed to undercount.captureEvent(apps/mobile/src/lib/analytics/posthog.ts) enqueues in memory; an app suspend right after capture lost the queued event (and capture is disabled in dev builds).notifyCompletion: false), and TestFlight/sandbox purchases DID emit with no property to filter them out. The event as defined could never equal any backend-derived "passes sold" number.How (design)
Server is the sole emitter, once per completed sale. The single choke point for App Store sales is
completeStoreKiloPassPurchase(inserts onekilo_pass_store_purchasesrow per provider transaction id, unique constraint on(payment_provider, provider_transaction_id), returnsalreadyProcessed). Emission gates onalreadyProcessed === false, so recovery replays, ASSN/app races, and tRPC retries collapse to one event. For Stripe, the anchor is the absence of a priorkilo_pass_audit_logrow withaction = KiloPassInvoicePaidHandled,result = Successfor the invoice id — checked in-transaction after thekilo_pass_subscriptionsupsert (whose row lock serializes same-subscription webhook concurrency) and before this run's own Success-row append.baseCreditsResult.wasIssuedwas considered and rejected: issuance is unique per(subscription, issue_month), not per invoice, so second paid invoices in one month (upgrades, prorations) would silently never emit.Delivery guarantee: at most once, best-effort after commit. The hard requirement is no double counting; emission happens only after the recording transaction commits (fire-and-forget on the user-facing tRPC mutation, Next.js
after()on both provider webhooks so the serverless function stays alive). A crash between commit and capture loses that one event — accepted residual. Concurrent same-invoice Stripe redelivery is covered by the upsert row-lock reasoning above, not a DB uniqueness constraint — accepted residual; sequential redelivery is the tested bar. A full at-least-once outbox is out of scope (possible follow-up).Semantics (deliberate change, documented for the data team):
purchase_kind(initial|renewal|upgrade|unknown). Rationale: under-delivery of events is unrecoverable downstream; over-delivery with a filter property is self-serve fixable. A renewal is a completed purchase (money moved); the DB records one row per charge.purchase_kindis abilling_reason/store-flow proxy, not a tier diff — proration/cadence-change invoices can readupgrade; a resubscribe on an existing provider subscription readsrenewal; Stripe null/unmappedbilling_reasonreadsunknown(never guessed from subscription-row presence, becausecustomer.subscription.createdcan create the row before the firstinvoice.paidarrives — guessing would mislabel true first purchases).$0invoices (100% promo/trial first months) DO emit — a pass was granted and the subscription started;amount_paid_usd: 0makes them filterable.claw_trial_startedconvention): both channels sendchannel,tier,cadence,purchase_kind,user_id; App Store addsprovider_transaction_id,product_id,environment; Stripe addsstripe_invoice_id,amount_paid_usd,currency,livemode.distinctIdis the user email, matching the web/mobileidentify(email)convention so funnels stay joined.channelproperty — filter them out withchannel IS SET. PostHog cannot dedupe a client event against a server event for the same purchase, which is why the client emission is removed rather than kept.Data-team guidance
kilo_pass_purchase_completedtochannel IS SET. Events withoutchannelare the old lossy client series (transition window).channel IS SET) counts transactions (initial + renewal + upgrade), not conversions.kilo_pass_purchase_started→ completed): filter completed tochannel = 'app_store' AND purchase_kind IN ('initial', 'upgrade')to approximate the old user-initiated funnel. Resubscribes readrenewaland Stripe null/legacybilling_reasonreadsunknown; both are excluded from that funnel view — intentional.unknownshould be ~zero in production.environment. $0 promos: filter viaamount_paid_usd(andcurrencyfor correctness; Kilo Pass bills USD today).kilo_pass_store_purchases/ Stripe invoices — a one-off PostHog bulk backfill is possible as a follow-up if wanted (not in this PR).Verification
No manual testing: E2E is environment-limited — real App Store purchases cannot run in the simulator/E2E stack and PostHog capture is disabled in dev builds, so no E2E slot was taken. Verification is automated:
purchaseKindclassification unit tests (initial / renewal / upgrade) + idempotent-replay shape.renewal; SUBSCRIBED → 1initial; DID_CHANGE_RENEWAL_PREF+UPGRADE → 1), Stripe (first invoice → 1; sequential redelivery → 0; second invoice same month → 1; $0 invoice → 1; blocked duplicate-card → 0;subscription_createwith pre-existing row →initial; nullbilling_reason→unknown).JEST_MAX_WORKERS=1and hours — see the committed learning; failures there are pre-existing per-test 5s timeouts under docker postgres load, in suites this diff does not touch).Visual Changes
N/A
Reviewer Notes
purchaseKindexists only whenalreadyProcessed === false— no??fallbacks), and the snake_case wire map.CompleteStorePurchaseOutputSchemais unchanged:purchaseKindis internal, Zod strips it from the tRPC response; the mobile contract is untouched.jest.mockthe tracking module with a factory that keepsrunAfterResponseworking inline — a bare automock would no-op it and the emit callback would never run..kilo_workflow/learnings/entries ride along (workflow metadata, not product code).KILO_PASS_PURCHASE_COMPLETED_EVENTremains exported inapps/mobile/src/lib/analytics/posthog.ts(owned by a parallel section); the negative test imports it, keeping knip green.