feat(review): native app-store review sheet + drop the custom prompt (TASK-22193, TASK-21044) - #2995
Conversation
… launch the real store ids already landed on dev (ca45419); this pins the ios store + write-review urls to the canonical /us/ listing form (apps.apple.com/us/app/id6786373552) so they match the published listing exactly. play url confirmed unchanged. launch-blocker per TASK-21044 §2. branch doubles as the handoff branch for slava's review-nudge changes.
App Store Review Guidelines 5.6.1 disallows custom review prompts outright: "Use the provided API to prompt users to review your app ... we will disallow custom review prompts." ReviewPromptModal is one — our own copy, our own CTA, deep-linking out to the store's write-review page. Two further problems on the same modal: routing "Could be better" to the support drawer filters unhappy users away from the store (5.6.3 Discovery Fraud; Google's In-App Review guidelines name the "Do you like the app?" pre-question explicitly), and it fired on the first home view after app open, which is the moment the HIG tells you not to ask. Removing it rather than relocating it: the native review sheet (SKStoreReviewController / Play In-App Review) needs a Capacitor plugin, so it cannot ride an OTA bundle. This deletion is JS-only and unblocks flipping pwa-sunset to 100% without carrying a 5.6.1 violation into App Review; the happy-moment replacement lands with the next binary. REVIEW_URL, the reviewPromptShownAt preference and the APP_REVIEW modal type lose their only consumer and go with it. migration.review.* is removed from the three source catalogs and the marketing subsets regenerated.
…193) Replaces the deleted custom prompt with the OS review sheet (@capgo/capacitor-in-app-review: SKStoreReviewController on iOS, Play In-App Review on Android), which is the only ask guideline 5.6.1 permits. This is the binary half of the review-nudge work — it adds native plugins and so cannot ship over OTA; the JS-only removal is TASK-22193's other PR. requestAppReview(userId, trigger) is the single entry point. It carries the whole policy so call sites stay one line: - native only, skipped in demo mode - an engagement floor (>=2 qualifying moments) before the first ask, so a day-0 depositor is never asked — HIG's "demonstrated engagement" - a 120-day floor between requests and at most 2 per 365 days, one under Apple's silent 3/365 ceiling so a strong moment always has budget. The OS reports nothing back, so this stamp is the only control we have. - a 7-day friction quiet period: any money-flow failure (observed for free in posthog before_send) suppresses the ask. This is the honest version of what the old "Could be better" branch reached for — suppress the question, never filter the answer toward or away from the store. Wired on the tail of four happy moments, each gated on the success being real and fired ~2.5s after the celebration settles (never mid-flow, never layered on our own modal): reward/perk claim (a friend joined + money landed — the strongest moment), shared payment success, claim-link money-received, and QR pay. review_requested is captured with the trigger and budget state; there is deliberately no shown/rated counterpart because neither OS exposes one — measure against App Store Connect / Play Console. Also adds a user-initiated "Leave a review" row in Profile -> About. A prompt built from a write-review deep link would breach 5.6.1, but Apple documents that exact link for a review the user starts themselves, and it is the release valve for anyone the OS quota silently skipped. Opens via @capacitor/app-launcher, not the in-app browser: SFSafariViewController does not follow the universal link, so ?action=write-review would otherwise land on the web listing with no composer.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 similar comment
Code-analysis diffPainscore total: 7233.57 → 7237.64 (+4.07) 🆕 New findings (78)
…and 58 more. ✅ Resolved (81)
…and 61 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 6 screens moved9 of 66 shots changed · 57 identical · baseline
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The native review flow is narrowly scoped and its primary budget tests pass, but the exact-head unit gate is red and two edge paths bypass the intended native-only and post-failure guards.
Findings
-
MINOR · package.json:55 · Register both plugins in the native fingerprint list
The exact-head unit job failsnative-fingerprint › lists every plugin Capacitor generated in NATIVE_DEPENDENCIES: the generated manifests now contain both new packages, whilescripts/native-fingerprint.mjsomits them fromNATIVE_DEPENDENCIES. Add@capacitor/app-launcherand@capgo/capacitor-in-app-reviewto that list so the repository's native-surface contract passes and remains complete when generated manifests are stale. -
MINOR · src/utils/app-review.ts:69 · Require a live bridge before spending a review request
isCapacitor()is intentionally true forNEXT_PUBLIC_CAPACITOR_BUILD=truebrowser previews even when no native bridge exists. In that reachable environment the dependency's web adapter resolvesrequestReview()without displaying anything, so this code appendsrequestedAtand emitsreview_requestedfor a nonexistent prompt; the About row is exposed there for the same reason. Gate native review APIs and native-only UI withisNativeBridge()and cover the env-only/no-bridge case. -
MINOR · src/utils/app-review-friction.ts:33 · Treat collateral spend failures as review friction
QR pay can fail inresolveSpendStrategyoruseSignSpendBundle, where the only analytics terminal isCARD_WITHDRAW_FAILED; this set ignores that event. If the user hits such a collateral/signing failure and then succeeds on retry, no quiet-period stamp exists and the second qualifying moment can open the review sheet 2.5 seconds later despite the immediately preceding money-flow failure. IncludeCARD_WITHDRAW_FAILEDand pin this retry scenario in the friction test. -
MINOR · src/hooks/useAppReviewNudge.ts:25 · [claude-opus] New gating hook useAppReviewNudge ships with no test
CONTRIBUTING.md (mono, "Testing & verifiability" → Frontend): "Every custom hook that fetches data, gates a flow, or holds persistent state needs a test." useAppReviewNudge gates the review flow behind a 2.5s timer and drives the only write to the persistentreviewNudgebudget in localStorage, and there is no src/hooks/tests/useAppReviewNudge.test.tsx — while ~30 sibling hooks in that directory do have one. app-review.test.ts covers requestAppReview's budget arithmetic but mocks nothing about the hook, so the hook's own contract is entirely unexercised.
Exact untested cases, both of which are behaviours the hook's own docstring promises:
- Unmount (or
enabledflipping back to false, e.g. a post-factum error on qr-pay) before SETTLE_MS elapses must cancel the timer and never call requestAppReview — "a user who navigated away is no longer at a stopping point". Nothing asserts the cleanup. - "Fires at most once per mount."
fired.currentis only set inside the timeout callback, so any dep change during the 2.5s window (userId resolving late,enabledtoggling) tears down and restarts the timer rather than being a no-op; a re-render storm can push the ask arbitrarily late or, after the first fire, the guard is what stops a second banked moment. A render-hook test pinning "one requestAppReview call across N re-renders" would lock this down.
Fix: add src/hooks/tests/useAppReviewNudge.test.tsx with jest fake timers and @testing-library/react's renderHook — mock @/utils/app-review, assert (a) no call when unmounted at 2.4s, (b) exactly one call at 2.5s with (userId, trigger), (c) no call when enabled is false or userId is undefined, (d) one call only across repeated re-renders.
Checked clean
- Verified the detached worktree, trusted author, base ref/base SHA, and exact head SHA.
- Traced all four success-surface call sites; request creation and unconfirmed claim states are excluded.
- Checked the engagement floor, 120-day cooldown, two-per-year budget, friction expiry, and plugin-rejection behavior; the new focused tests pass in exact-head CI.
- Inspected the pinned review plugin's web, Android, and iOS implementations and the AppLauncher web fallback; no caller-controlled URL or sensitive-data path was introduced.
- Checked removal of the custom prompt, modal orchestration, translations, store URLs, and surface-gallery references.
- Exact-head format, typecheck, eslint, native-export, and analyze checks passed; unit failed only on the native fingerprint registry assertion, and ds-shots was still running when reviewed.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 1d3803a2f64d · Context: repo, dependency · Took 11m
…es, register the plugins - requestAppReview and the About row use isNativeBridge(): a capacitor-flavoured web preview resolves the plugin's web adapter without showing anything and would spend a request from the budget - CARD_WITHDRAW_FAILED is friction: a failed collateral spend that succeeds on retry must not open the sheet 2.5s later - both new plugins in NATIVE_DEPENDENCIES, which the fingerprint test enforces
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The exact head fixes P1-P3. P4 remains as a minor regression-test gap around the delayed review hook; required exact-head CI is green.
Findings
-
MINOR · src/hooks/useAppReviewNudge.ts:30 · Cover the delayed review hook itself
The utility tests call requestAppReview directly, so they never exercise the hook that owns the 2.5-second delay, cancellation when the success surface disappears, and once-per-mount guard. A regression that drops the cleanup could open the OS sheet after the user has left the success screen, while a guard regression could schedule it more than once, without failing the current suite. Add a renderHook test with fake timers that covers the delay, unmount/disable cancellation, and a single call across rerenders. -
MINOR · src/hooks/useAppReviewNudge.ts:25 · [claude-opus] New gating hook useAppReviewNudge ships with no test
useAppReviewNudgeis the only production caller ofrequestAppReview, and it decides whether the persistedreviewNudgestate in localStorage gets mutated at all — yet it has no test, whileapp-review.tsandapp-review-friction.tsboth got thorough ones (src/utils/__tests__/app-review.test.ts,src/utils/__tests__/app-review-friction.test.ts).
Exact untested cases:
- Unmount inside the 2.5s settle window must cancel the ask — a user who taps away from
PaymentSuccessView/Success.view.tsxwithinSETTLE_MSshould bank no moment. Nothing asserts theclearTimeoutcleanup, so a future edit that movesfired.current = trueout of the timer callback, or drops the cleanup, would silently start banking moments (and eventually spending one of the two requests/year) for users who never stayed on the screen. - Fire-at-most-once-per-mount when the effect re-runs.
HomeCarouselCTApassesenabled = claimedPerkIds.size > 0 && !selectedPerk, which toggles as the perk modal opens and closes; onlyfired.currentstops each toggle from banking another moment, and that guard is unasserted.
Fix: a small @testing-library/react renderHook test with fake timers mocking @/utils/app-review — assert (a) no call when unmounted before 2500ms, (b) exactly one call after toggling enabled false→true→false→true across a full settle, (c) no call for userId === undefined.
Checked clean
- Verified the detached worktree head, trusted author, exact base ref and SHA, merge base, and PR metadata match the requested target.
- Reviewed the complete base-to-head diff across native dependency wiring, review budgeting, friction capture, success-surface gates, store links, custom-prompt removal, localization, and dev-surface cleanup.
- Rechecked P1: both new Capacitor plugins are listed in NATIVE_DEPENDENCIES, and the native fingerprint manifest includes the changed plugin-version input.
- Rechecked P2: requestAppReview returns before reading or writing reviewNudge state unless isNativeBridge() is true; the exact case is covered by the utility test.
- Rechecked P3: CARD_WITHDRAW_FAILED is now a friction event, is emitted by collateral-spend failure paths, and the retry-then-success case is covered.
- Traced all four nudge call sites: each is gated on a completed money or reward outcome, with request creation excluded and claim/QR failures cancelling the pending timer.
- The deleted custom modal still appears in two design-system audit inventories, but the PR explicitly scopes that known cleanup to TASK-22226 rather than leaving it untracked.
- Required exact-head CI is green, including aggregate CI, unit, typecheck, native export, lint, format, analysis, provenance, and preview deployment; advisory ds-shots was still running when checked. A focused local Jest run was unavailable because dependencies are not installed in the detached worktree.
- Correctness, security, adversarial, and slop passes found no additional actionable defect and no security, privacy, credential, authorization, or money-control surface requiring a dedicated security review.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: c36ec0423a93 · Context: repo, github_ci · Took 11m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The app-review nudge is correctly gated, budgeted, and covered at this head; all supplied prior findings are fixed and exact-head required CI is green.
Checked clean
- Confirmed the detached worktree, trusted author, dev base, base SHA, and head SHA match the supplied review target.
- Reviewed the app-review budget, native-bridge and demo gates, friction suppression, plugin failure handling, manual store fallback, and all four success-surface call sites.
- Verified both native plugins appear in package metadata, generated Android/iOS integration files, and the explicit native fingerprint dependency list.
- Reviewed the delayed hook suite covering timer threshold, unmount and disabled cancellation, once-per-mount behavior, missing users, and disabled success state.
- Exact-head aggregate CI, unit, native export, typecheck, lint, format, analysis, design-system lint, provenance, and deploy preview checks passed; the non-gating ds-shots job was still in progress.
- A local targeted Jest rerun could not start because the detached worktree has no installed node_modules; exact-head unit CI passed the committed suites.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 5d1482c9e9db · Context: repo · Took 9m
2 participants
Summary
Re-author of #2945 (+ its stacked #2952, already merged into that branch) onto current
dev. The old branch forked before the claim-settle, web-vitals and About-view changes landed and could no longer merge; nothing in it was superseded —ReviewPromptModalis still live ondevtoday.1. Canonical store URLs (TASK-21044). Pins the iOS store + write-review URLs to the official
/us/listing form (https://apps.apple.com/us/app/id6786373552); Google Play unchanged (id=me.peanut.wallet).2. Removes the custom app-review prompt (TASK-22193).
ReviewPromptModal("Loving Peanut so far?" → store deep link / "Could be better" → support) is a custom review prompt, disallowed verbatim by App Store guideline 5.6.1. It also routed unhappy users away from the store (5.6.3; Google's In-App Review guidelines forbid the pre-question by name) and fired on the first home view after open. TheAPP_REVIEWmodal type,reviewPromptShownAtpref,migration.review.*copy (three locales + regenerated marketing subsets) and the surface-gallery entry go with it.3. The compliant replacement: the OS review sheet on happy moments.
useAppReviewNudgemounts on four confirmed-success surfaces (payment success, claim success, QR payment, perk claimed) and callsrequestAppReview, which:before_send, so no call sites);Profile → About gains a user-initiated Rate Peanut row (native only) that hands the write-review URL to the OS via AppLauncher — a tap, never a prompt, and the release valve when the OS quietly swallows the sheet.
Conflict resolution vs the old branch
Success.view.tsx: the nudge now gates onisClaimed && !claimFailure, matching the haptic and the processing-state gate that fix(native): OTA beta switch off PostHog, Peanut-only receive, claim settles on CLAIMED #2956 introduced.About.view.tsx: keepsfetchUser(team-badge tap) next to the new native/store state.instrumentation-client.ts: both new imports kept.OTA vs binary
The JS is OTA-safe: both plugins are dynamically imported and every call is guarded, so a binary predating them just never shows the sheet and falls back to the in-app browser on the Rate row. The sheet itself only fires from a binary that includes
@capgo/capacitor-in-app-reviewand@capacitor/app-launcher(Package.swift / gradle wiring included here).Tasks
ReviewPromptModal.QA
pnpm typecheckclean; related suites green (app-review, app-review-friction, About, Success.view, HomeModals, migration — 6 suites / 74 tests); prettier + eslint clean on touched files.pwa-sunsetflips.