Repository navigation
feat(migration): QR payload hand-off through /app, store pair fixes, app-link coverage (TASK-21788) - #3009
feat(migration): QR payload hand-off through /app, store pair fixes, app-link coverage (TASK-21788)#30090xkkonrad wants to merge 2 commits into
Conversation
…app-link coverage (TASK-21788)
|
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 |
Code-analysis diffPainscore total: 7255.9 → 7260.19 (+4.29) 🆕 New findings (31)
…and 11 more. ✅ Resolved (30)
…and 10 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 4 screens moved5 of 66 shots changed · 61 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 deferred store handoff works across platform branches, but its QR-source attribution is discarded, and the new hero width cap prevents the store buttons from ever forming the intended desktop row.
Findings
-
MAJOR · src/app/app/page.tsx:129 · Record the QR source before the store handoff
For a QR emitted bylanding_footer,/app?...&s=landing_footerremovessfrom the forwarded payload, then manual taps hard-codesurface: smart_linkhere while Android auto-bounces record only the platform. Current context-free QRs therefore auto-bounce without any scan event, and payload QRs cannot be attributed to the surface that produced them, so the per-surface scan/conversion rate this parameter was added for is unmeasurable. Parse and validatesagainst the known migration surfaces, then capture it on a scan or handoff event before either redirect path runs. -
MINOR · src/components/Migration/StorePair.tsx:40 · Give the hero pair enough width to stay paired
Atsmand above the two hero anchors are 13rem each; withgap-3they need 26.75rem (428px), but this wrapper caps itself at 26rem (416px). Flex line breaking therefore puts them on separate rows even on the desktop landing hero, so the documented two-button row can never render. Raise the cap to at least 26.75rem or reduce the child widths/gap, and cover the desktop one-line layout.
Checked clean
- Verified the detached worktree head, trusted author, exact base ref and SHA, merge base, and PR metadata.
- Traced deferred payload construction, marker parsing, surface stripping, Play referrer generation, iOS clipboard handoff, and native payload application/routing.
- Checked bare and payload-bearing /app visits across Android, iOS, desktop, native, and flag-off branches, including redirect fallbacks and destination sanitization.
- Checked QR impression observation, current DownloadQR and StorePair callers, and the landing sticky CTA device fallback.
- Checked iOS AASA, Android intent-filter, native export mapping, and the repository's parity/drift guards; the new /app claim maps to a shipped native route.
- Reviewed relevant file history for the smart-link origin, curated App Links allowlist, and native deep-link handling.
- Exact-head aggregate CI, unit tests, typecheck, native export, lint, formatting, design-system checks, visual snapshots, provenance, and preview deployment all completed successfully.
- No earlier findings were supplied for reconciliation; correctness, security/privacy, adversarial, and slop passes found no other reachable defect in the changed behavior.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
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: 42946479c4f2 · Context: repo, ci · Took 20m
| // must stay synchronous up to the clipboard call: a web clipboard write | ||
| // only succeeds inside the user gesture that triggered it | ||
| const onStoreTap = (store: StoreKind) => { | ||
| trackStoreClick(store, MIGRATION_SURFACES.SMART_LINK, !!payload) |
There was a problem hiding this comment.
MAJOR: Record the QR source before the store handoff
For a QR emitted by landing_footer, /app?...&s=landing_footer removes s from the forwarded payload, then manual taps hard-code surface: smart_link here while Android auto-bounces record only the platform. Current context-free QRs therefore auto-bounce without any scan event, and payload QRs cannot be attributed to the surface that produced them, so the per-surface scan/conversion rate this parameter was added for is unmeasurable. Parse and validate s against the known migration surfaces, then capture it on a scan or handoff event before either redirect path runs.
| ? 'flex w-full flex-col gap-3' | ||
| : // wraps rather than overflows: two 208px hero buttons plus the gap | ||
| // need 428px, more than a 390px phone has, so they stack there | ||
| 'mx-auto flex w-full max-w-[26rem] flex-wrap items-center justify-center gap-3' |
There was a problem hiding this comment.
MINOR: Give the hero pair enough width to stay paired
At sm and above the two hero anchors are 13rem each; with gap-3 they need 26.75rem (428px), but this wrapper caps itself at 26rem (416px). Flex line breaking therefore puts them on separate rows even on the desktop landing hero, so the documented two-button row can never render. Raise the cap to at least 26.75rem or reduce the child widths/gap, and cover the desktop one-line layout.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The hero pair now has enough width, and payload-bearing handoffs preserve QR attribution, but the surface-only QRs used by every current caller still auto-redirect without recording their source.
Findings
- MAJOR · src/app/app/page.tsx:171 · Attribute surface-only QR auto-redirects
Every currentDownloadQRcaller passes neitherpayloadnorhandoff, so scanning (for example)/app?s=landing_footerleavespayloadnull. This condition then skipscountHandoff, and the immediatelocation.replacesucceeds withoutonStoreTapever running; no custom event recordsqr_surface, so the current per-surface QR-to-store funnel remains unmeasurable. Record a dedicated scan/auto-redirect event (or the equivalent store event withhandoff:false) with the validatedqrSurfacebefore every surface-tagged redirect, while keeping deferred-handoff counts payload-only.
Checked clean
- Verified the detached worktree head, exact base ref and SHA, merge base, trusted PR metadata, and changed-file scope.
- Reconciled both prior findings: P2 is fixed by the 27.5rem hero cap and geometry test; P1 remains for surface-only auto-redirects.
- Traced deferred payload building, marker and surface parsing, Play referrer generation, iOS clipboard handoff, once-per-visit counting, and native payload application and routing.
- Checked bare and payload-bearing /app visits across Android, iOS, desktop, native, and flag-off branches, including redirect fallback behavior.
- Checked QR impression observation, all current DownloadQR and StorePair callers, sticky CTA device fallback, and the hero layout arithmetic.
- Checked iOS AASA, Android intent-filter parity, native export mapping, and /app wildcard collapsing against the shipped route set.
- Consulted the canonical Notion Lexicon and linked TASK-21788 task; neither defines an additional conflicting product or analytics contract.
- Exact-head ci-success, unit, typecheck, native-export, lint, formatting, design-system lint, provenance, analysis, and preview checks passed; advisory ds-shots was still running.
- A local focused Jest run was unavailable because this detached worktree has no node_modules; exact-head unit CI passed.
- Correctness, security/privacy, adversarial, and slop passes found no other reachable defect in the changed behavior.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
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: aebf6bbcf8fe · Context: repo, product, ci · Took 15m
| // can still bounce; so can any device with nothing to hand off. | ||
| if (payload && targetStore === 'ios') return | ||
| // counted before the navigation, and only once per visit | ||
| if (payload && targetStore === 'android') countHandoff('android') |
There was a problem hiding this comment.
MAJOR: Attribute surface-only QR auto-redirects
Every current DownloadQR caller passes neither payload nor handoff, so scanning (for example) /app?s=landing_footer leaves payload null. This condition then skips countHandoff, and the immediate location.replace succeeds without onStoreTap ever running; no custom event records qr_surface, so the current per-surface QR-to-store funnel remains unmeasurable. Record a dedicated scan/auto-redirect event (or the equivalent store event with handoff:false) with the validated qrSurface before every surface-tagged redirect, while keeping deferred-handoff counts payload-only.
|
Superseded by #3018 (all four folded into one PR per Konrad); branch kept until that merges. |
This branch was successfully deployed
1 participant
Summary
PR 1 of the peanut.me landing → app-store migration (TASK-21788): the mechanism only, no page layout. It makes the download QR carry deferred context, teaches
/appto hand that context to the right store on the right platform, fixes the store-button pair so it lays out correctly on desktop and can't overflow a phone, and claims/appfor App Links on both platforms so an already-installed user who scans a QR lands in the app instead of the store page.Everything here is inert until the
pwa-sunsetPostHog flag is on, except the App Links files (which are additive) and theStoreBadges→StorePairrename.What changed
DownloadQRtakessize?: 160 | 192 | 224and one context input: eitherpayload(a querystring the caller already holds) orhandoff({dest, invite}), mutually exclusive in the props type. Givenhandoffit derives the payload itself after mount and passes the samehandoffto theStorePairbeneath it, so the code and the buttons can never carry different context. The encoded URL is${origin}/app?${payload}&s=<surface>(or?s=<surface>alone), so every scan is attributable to the QR that produced it and carries locale / invite / badge campaigns / destination through the install.sizeis the QR frame width; the module area is 36px narrower (p-4+border-2): 160 → 124px modules, 192 → 156px, 224 → 188px. Documented on the constant. PR 2 should passsize={192}for the hero andsize={224}for the app fold and the footer to hit the brief's stated module areas.MIGRATION_QR_SHOWNmoved from mount to a real impression: anIntersectionObserverat 50% visibility, once per mount, with{ surface, hasContext }. The app fold and the footer render their QR far below the fold, so counting mounts would have made the scan rate look broken. Falls back to counting the mount whereIntersectionObserveris missing (jsdom, old WebViews); in a modal the QR is already in view on open, so that is the open event. The capture waits for the derived payload, so a contextful QR never reportshasContext:false./app(src/app/app/page.tsx) readswindow.location.searchafter mount and validates it withparseDeferredPayload(marker check):playStoreUrlWithReferrer(payload)and firestrackDeferredHandoffCreated('android').copyIOSHandoff(payload)inside the click handler before the anchor navigates.applyDeferredPayload(parsed)thenrouter.replace(dest ?? '/home').DEFERRED_LINK_HANDOFF_CREATEDis counted once per visit (ref guard shared by the auto-redirect and the tap). Without it, a bounce that never took over — blocked, offline, Play missing — re-enabled the buttons after 4s and a tap counted a second hand-off, deflating the restore match rate on exactly the flaky devices.trackStoreClick(store, 'smart_link', hasPayload, qrSurface). Thes=tag is read here (validated againstMIGRATION_SURFACES) and reported asqr_surfaceon the click and hand-off events, which is what joins a smart-link click back to the hero / app fold / footer / rates QR that produced the scan. It is still stripped before the payload is re-emitted, so it never rides into the Play install referrer.StoreBadges→StorePair(rename, imports updated). Anchors usestoreAnchorHref(store, handoff)+onStoreAnchorClick(store, surface, handoff). Container widths are per-appearance:heroismax-w-[27.5rem](440px) so twosm:w-52anchors (208px each) plusgap-3(12px) = 428px sit on one line at every width ≥sm, including 1366 and 1440; the compact row keepsmax-w-[26rem]. Belowsmthe hero anchors arew-fulland stack by design. The two hrefs are memoized per handoff identity — the android one runsbuildDeferredPayload(cookie + badge-campaign +window.locationreads) and the hero pair lives insideLandingPageClient, which re-renders every scroll-driven animation frame until PR 3 lands.storeAnchorHref/onStoreAnchorClicktake an optionalStoreHandoff, matching whatopenStorealready accepted.StickyMobileCTA:deviceType === WEB(desktop-mode phone, or a UA we don't recognise) now renders the compact store pair instead of guessing iOS. Everything else is unchanged./app+/app/*added to all three appIDs inpublic/.well-known/apple-app-site-association, andandroid:path="/app"+android:pathPrefix="/app/"to the verified intent filter.appadded toNATIVE_EXPORT_ROOTS, andmapDeepLinkPathcollapses the whole/app/*claim onto/app— the wildcard has no route behind it on either the web app or the static export, so a passthrough would open the app onto a missing route (see Divergences).MIGRATION_SURFACESgainsSMART_LINK,LANDING_APP_FOLD,LANDING_FOOTER,LANDING_RATES,LANDING_COUNTRIES,LANDING_DOOR; newMIGRATION_SURFACE_PARAM = 's'andisMigrationSurface()are shared by the QR builder and/app.DownloadQRURL building, handoff-derived payload reaching both channels, sizes, impression threshold andhasContext;StorePairhrefs, handoff, tap tracking and the hero row's one-line arithmetic;/apppayload branching (android bounce, iOS no-bounce + clipboard-on-tap, bare visit, native apply-and-route, flag-off 404),qr_surfaceattribution, an unknown surface tag, and the once-per-visit hand-off count;StickyMobileCTAWEB case;/app/*collapsing in the deep-link mapper; an App Links parity guard (iOS AASA ↔ Android intent filter).No new user-facing copy — no i18n keys added.
How verified
pnpm typecheck→ clean on every touched file. 6 errors remain, allTS2307in files this PR does not touch (src/utils/web-vitals-shim.ts,src/utils/app-review.ts,src/utils/native-settings.tsand the web-vitals test):web-vitals,@capacitor/app-launcher,@capgo/capacitor-in-app-reviewandcapacitor-native-settingsare absent from this sandbox's sharednode_modules. Same 6 before and after the change.pnpm test -- src/components/Migration/__tests__/DownloadQR.test.tsx src/components/Migration/__tests__/StorePair.test.tsx src/app/app/__tests__/smart-store-link.test.tsx src/utils/__tests__/app-links.test.ts src/utils/__tests__/native-routes.test.ts src/components/LandingPage/__tests__/StickyMobileCTA.test.tsx src/components/Migration/__tests__/MigrationDownloadModal.test.tsx→ 7 suites, 246 tests, green.pnpm test -- src/utils/__tests__/deferred-link.test.ts src/utils/__tests__/migration.utils.test.ts→ 2 suites, 63 tests, green.pnpm lint --quiet <touched files>→ clean.prettier --check <touched files>→ clean.StorePair.test.tsxagainst the resolved values rather than the class string (jsdom has no layout). Tailwind v4 with no--spacingor--breakpoint-smoverride insrc/styles/globals.css, sow-52= 13rem = 208px,gap-3= 0.75rem = 12px,sm= 640px, and themax-w-[27.5rem]= 440px cap clears the 428px the row needs. The previousmax-w-[26rem](416px) wrapped the pair at every width ≥sm.Divergences
/appalso had to become a mappable native route. The brief asked only for the AASA + manifest entries. Adding them immediately failed the existing AASA drift guard, which asserts every claimed root resolves throughdeepLinkToNativePath. That is not a test technicality: an App Link the mapper drops cold-boots the app to/home, so the brief's own native branch (applyDeferredPayload→router.replace(dest)) could never run./appis already built by the native static export, so nothing new ships — only the mapper's verdict changed.appwas therefore added toNATIVE_EXPORT_ROOTSand removed fromWEB_ONLY_EXPORTEDin the export-drift test, with both comments updated. Side effect, accepted: an in-app anchor to/appwould now push in-app rather than open the browser; there are none in the codebase.handoffthreaded through the store helpers, and it is now the only context channel onDownloadQR(a caller passespayloadorhandoff, never both;handoffderives the payload internally). PR 2 opens the hoisted modal withdest=/sendon fold 4 and destination context on fold 5, and that modal's content isDownloadQR→StorePair. As first written the two props were independent:handoffalone left the QR context-free (and reportedhasContext:false) while the buttons beneath it carried the destination, andpayloadalone did the reverse, with nothing catching it. One input, one derivation. The brief's reason for the caller building the payload — it readswindow— is met by deriving it in a mount effect.sizeis the QR frame width, not the module area. 160 is exactly the frame widthQRCodeWrapperships today (max-w-[160px]); the modules inside it are 124px. Reading "160 default" as the module area would silently resize every existing QR on merge, so the reading that keeps the stated default a no-op is the frame width. The frame→module mapping is now documented on the constant and a224option added, so PR 2 can hit the brief's module areas:192for the hero (156px modules),224for the app fold and footer (188px)./app/*App Links wildcard is collapsed in the deep-link mapper. The claim itself is exactly what the brief asked for on both platforms, but/app/<anything>is not a route on the web app or the static export.mapDeepLinkPathnow returns/appfor any/app/*deep link, so an installed user openingpeanut.me/app/anythinglands on the real page instead of the SPA's missing-route → home bounce. Neither the AASA drift guard nor the app-links parity test can see this class of bug — both only compare the two hand-maintained lists to each other./appreads thes=surface tag instead of only stripping it. The brief had the QR write the tag and/appstrip it before re-emitting the payload. It still strips it (the tag must never ride into the Play install referrer), but it is now also validated againstMIGRATION_SURFACESand reported asqr_surfaceon this page's click and hand-off events. Without that read the tag was costing QR modules for nothing and the landing surfaces were indistinguishable in the smart-link funnel — the one attribution question the param exists to answer.Flag-off impact
None on the page: every behaviour change is inside a
pwa-sunsetbranch (/appstill 404s with the flag off,StickyMobileCTA's flag-off arm is untouched,StorePair/DownloadQRonly render on migration surfaces). Three changes are live regardless and are additive by design: the/appApp Links claim on both platforms,appjoiningNATIVE_EXPORT_ROOTS, and the mapper's/app/*collapse.Release gate — the two halves of the
/appclaim ship on different vehicles. The AASA is a static file served by the web deploy, so iOS picks up the/appclaim for already-installed apps within days of merge. The Android intent-filter lives in the APK and only takes effect in the next Play release.app-links.test.tsasserts the two files agree and stays green through the entire window in which the two platforms do not. So: do not flippwa-sunseton for Android traffic until an Android build containing this manifest is live in Play, and run the runtime checklist's "Android UA" item against that binary, not against the web deploy. Until then an installed Android user who scans a download QR still lands on web/app(404 with the flag off; store buttons for an app they already have once it is on).