Repository navigation
Conversation
|
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: 7251.68 → 7253.18 (+1.5) 🆕 New findings (21)
…and 1 more. ✅ Resolved (18)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![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 new store handoff drops fresh badge-campaign intent on phones and the requested /card destination on desktop QR installs.
Findings
-
MAJOR · src/app/shhhhh/ShhhhhLandingPage.tsx:220 · Queue URL campaigns before the mobile handoff
With pwa-sunset on, a fresh signed-out phone visitor following/shhhhh?badge_campaign=<active-skip-id>returns here before the URL-derived campaign is queued on line 223.openStore()builds the deferred payload fromgetPendingBadgeCampaigns(), so the install carries/cardbut not this campaign; first launch cannot restore or claim the intended Skip Pass and the card flow rejects the visitor. QueuebadgeCampaignsbefore interception (or pass them explicitly into the deferred payload) and assert that the serialized mobile handoff contains them. -
MAJOR · src/app/shhhhh/ShhhhhLandingPage.tsx:220 · Preserve the destination through the desktop QR
On desktop with pwa-sunset on, this passes{ dest: '/card' }, but the WEB branch only opensScanToDownloadModal;DownloadQRencodes plain/app, whose smart redirect opens a bare store URL. After scanning and installing, the app therefore has no deferred/carddestination and opens its default route instead of the door's target. Thread the handoff through the QR/smart-link path (with the same sanitization asopenStore) and add an assertion for the encoded QR destination. -
MAJOR · src/app/shhhhh/ShhhhhLandingPage.tsx:220 · [claude-opus] Store hand-off drops the /shhhhh campaign, so the documented Skip Pass is lost
product/card.md:87 and :145 state the Skip Pass (WAITLIST_SKIP, skips the card waitlist queue) is obtained via/invite?campaign=skipor/shhhhh?campaign=skip. This change putsif (!user && interceptGuestCta({ dest: '/card' })) returnat ShhhhhLandingPage.tsx:220, above thequeuePendingBadgeCampaigns(badgeCampaigns, 30)call at :223. For a signed-out visitor on a phone during the migration window the flow is now: read campaign tags → intercept →openStore()→buildDeferredPayload(dest, invite). That payload picks campaigns up only from the cookie (getPendingBadgeCampaigns()in src/utils/deferred-link.ts) — which was never written, because the queue call is unreachable behind the early return. Passingdest: '/card'also overrides the default dest (current path + search), which would otherwise have carried?campaign=skipback into the app. So both recovery routes are gone: the visitor installs the app, lands on /card with no campaign identity, hits the Berghain gate and is put on the waitlist instead of skipping it. Code disagrees with product truth, and the code is the wrong side.
The author already handles this correctly on the sibling surface: InvitesPage.tsx:255 queues the campaigns and only then calls interceptGuestCta(), with the comment "cookie/queue bookkeeping above still runs so a keep-web signup or post-install link re-tap recovers the invite context." The door skips that bookkeeping.
This is also the untested shared-state mutation here (the pending-badge-campaign cookie feeding a backend claim): the new case at ShhhhhLandingPage.test.tsx:117 ("hands off before the campaign signup route when the link carries campaign tags") asserts only that mockPush was not called, so it locks in the lossy behaviour rather than catching it.
Fix: move the intercept below const queuedBadgeCampaigns = queuePendingBadgeCampaigns(badgeCampaigns, 30) (or queue first when badgeCampaigns.length > 0, mirroring InvitesPage), and extend the campaign test to assert the queue was written before interceptGuestCta fired — i.e. that the deferred payload carries the campaign.
Checked clean
- Verified the detached worktree HEAD, supplied base SHA, merge base, trusted author, base ref, and live PR metadata.
- Traced signed-out, signed-in, campaign-tagged, flag-off, and native-app branches through the Shhhhh CTA.
- Traced mobile deferred payload creation/restoration and the desktop DownloadQR -> /app -> bare-store redirect path.
- Exact-head required CI is green, including unit, typecheck, eslint, format, native-export, and ci-success; advisory ds-shots was still in progress.
- The focused local Jest command could not run because this detached checkout has no Jest binary; it made no worktree changes.
- Reviewed the optional surface plumbing for existing hook callers and found no auth, secret, privacy, money, or privilege boundary change.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
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: 0f2ad5702c4c · Context: repo · Took 8m
| // desktop opens the scan-to-download QR, phones deep-link — carrying | ||
| // /card so the app lands them back on the door's destination. Flag off | ||
| // (and in the native app) this is a no-op and the routes below run. | ||
| if (!user && interceptGuestCta({ dest: '/card' })) return |
There was a problem hiding this comment.
MAJOR: Queue URL campaigns before the mobile handoff
With pwa-sunset on, a fresh signed-out phone visitor following /shhhhh?badge_campaign=<active-skip-id> returns here before the URL-derived campaign is queued on line 223. openStore() builds the deferred payload from getPendingBadgeCampaigns(), so the install carries /card but not this campaign; first launch cannot restore or claim the intended Skip Pass and the card flow rejects the visitor. Queue badgeCampaigns before interception (or pass them explicitly into the deferred payload) and assert that the serialized mobile handoff contains them.
| // desktop opens the scan-to-download QR, phones deep-link — carrying | ||
| // /card so the app lands them back on the door's destination. Flag off | ||
| // (and in the native app) this is a no-op and the routes below run. | ||
| if (!user && interceptGuestCta({ dest: '/card' })) return |
There was a problem hiding this comment.
MAJOR: Preserve the destination through the desktop QR
On desktop with pwa-sunset on, this passes { dest: '/card' }, but the WEB branch only opens ScanToDownloadModal; DownloadQR encodes plain /app, whose smart redirect opens a bare store URL. After scanning and installing, the app therefore has no deferred /card destination and opens its default route instead of the door's target. Thread the handoff through the QR/smart-link path (with the same sanitization as openStore) and add an assertion for the encoded QR destination.
🖼 Visual diff — 3 screens moved3 of 66 shots changed · 63 identical · baseline
new screens (1)
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. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The mobile campaign ordering is fixed, but the desktop QR still drops its deferred payload at /app, so the destination and campaign do not survive install.
Findings
-
MAJOR · src/components/Migration/DownloadQR.tsx:27 · Forward the QR payload past /app
This line makes a desktop scan open/app?pnutdl=1&dest=%2Fcard..., butsrc/app/app/page.tsxnever reads the query: its phone effect redirects to the bareSTORE_URL[targetStore], while the already-installed native branch replaces the route with/home. An Android or iOS scan therefore still loses/cardand any queued badge campaign before install/open; the modal's raw store badges lose them as well. Make/appforward the supplied payload into the platform handoff (and consume it when already native), then cover the QR-to-smart-store path with an integration test. -
MAJOR · src/components/Migration/DownloadQR.tsx:27 · [claude-opus] Desktop QR payload is dropped by /app, so the Skip Pass is still lost
The round-1 fix carries the campaign and dest through the desktop path by building a deferred payload and encoding it in the QR:DownloadQRnow renders${origin}/app?${payload}(src/components/Migration/DownloadQR.tsx:27), fed frombuildDeferredPayload(handoff.dest, handoff.invite)in useGuestStoreHandoff.tsx:59.
The receiving end never reads it. src/app/app/page.tsx is the only handler for that route, it does not call useSearchParams or touch window.location.search anywhere, and it bounces the scanning phone with window.location.replace(STORE_URL[targetStore]) (page.tsx:84) — a bare store URL. The store buttons below it are the same bare href={STORE_URL[s]} (page.tsx:113). Neither branch appends &referrer=<payload> for Play nor calls copyIOSHandoff for the iOS clipboard, which are the only two channels restoreDeferredContext reads on first launch (src/utils/deferred-link.ts:253-282). There is no middleware or redirects.json rule for /app either. So pnutdl=1&campaign=…&dest=/card is discarded at the redirect.
What that costs, against product truth: product/card.md:87 and :145 declare the Skip Pass (WAITLIST_SKIP) obtainable via /invite?campaign=skip or /shhhhh?campaign=skip. Before this PR a signed-out desktop visitor on /shhhhh?campaign=skip was pushed to the web signup with the campaign queued in the cookie, and the claim ran on registration. With the pwa-sunset flag on, that visitor now gets the QR, installs on a phone with no payload, and registers as an organic install — no queued campaign, no Skip Pass, and no dest=/card either (which is prior finding P2, also still unfixed by the same root cause). The code is what is wrong here, not the product doc. The phone path is fine: openStore builds and writes the payload itself (src/utils/migration.utils.ts:96-113).
Fix: have /app read its own querystring and forward it — playStoreUrlWithReferrer(payload) for the Android redirect and both store anchors, copyIOSHandoff(payload) before the iOS bounce (the anchor click is a user gesture, so the clipboard write is allowed there; the automatic window.location.replace on iOS is not, and may need to fall back to showing the buttons when a payload is present). A test asserting the redirect target carries referrer= would have caught this; the current hook test only asserts the payload reaches the modal prop, which is one hop short of the consumer.
Checked clean
- Verified the pinned head, base, author, and merge base; reviewed the landing CTA, guest handoff hook, QR modal, smart /app redirect, deferred-link build/restore, and affected consumers.
- P1 and P3 are fixed for mobile handoffs: URL campaigns are queued before interception, and buildDeferredPayload reads that cookie queue synchronously.
- Exact-head required CI is green, including unit, native-export, typecheck, eslint, format, ds-lint, be-baseline, analyze, human-authors, and ci-success; advisory ds-shots was still running.
- Focused local Jest execution was unavailable because the detached worktree has no installed Jest binary.
- Reviewed flag-off and native fallthrough, signed-in waitlist routing, modal state, analytics surface propagation, and unchanged hook consumers.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
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: 268fa758c9a6 · Context: repo · Took 13m
| return ( | ||
| <div className="flex w-full flex-col items-center gap-3 py-2"> | ||
| <QRCodeWrapper url={`${origin}/app`} /> | ||
| <QRCodeWrapper url={payload ? `${origin}/app?${payload}` : `${origin}/app`} /> |
There was a problem hiding this comment.
MAJOR: Forward the QR payload past /app
This line makes a desktop scan open /app?pnutdl=1&dest=%2Fcard..., but src/app/app/page.tsx never reads the query: its phone effect redirects to the bare STORE_URL[targetStore], while the already-installed native branch replaces the route with /home. An Android or iOS scan therefore still loses /card and any queued badge campaign before install/open; the modal's raw store badges lose them as well. Make /app forward the supplied payload into the platform handoff (and consume it when already native), then cover the QR-to-smart-store path with an integration test.
|
Superseded by #3018 (all four folded into one PR per Konrad); branch kept until that merges. |
This branch was successfully deployed
1 participant
Summary
The
/shhhhhdoor sends a signed-out visitor into/setupto create an account on the web. Once thepwa-sunsetflag is on, that signup is the thing we are closing, so the door now hands the visitor to the app store instead: desktop opens the scan-to-download QR modal, phones deep-link their store, both carrying/cardas the post-install destination so the app lands them on the door's real target.Flag off,
/shhhhhbehaves exactly as it does ondev.Part of the landing funnel work for TASK-21788 (PR 4 of 4).
What changed
src/app/shhhhh/ShhhhhLandingPage.tsx:handleCTAreturns early whileisFetchingUser, then capturesDOOR_TRY, then queues any campaign tag, thenif (!user && interceptGuestCta({ dest: '/card' })) return. Every door affordance (hero button, "or join the waitlist" link, §7 button, sticky mobile bar) callshandleCTA, so one branch covers all four, and both/setuproutes below it (/setup?redirect_uri=/cardand the campaign/setup?step=signup). The hook is asked for the guest impression withtrackImpressionWhenGuest: !isFetchingUser && !user && !isJoined. The sticky mobile bar unmounts whilestoreHandoffModalis open, so it cannot paint over the QR sheet's own close / store buttons on a narrow desktop-mode phone.src/hooks/useGuestStoreHandoff.tsx: optionalsurfaceargument, defaultMIGRATION_SURFACES.GUEST_FLOW, labellingopenStore,ScanToDownloadModaland the guest-impression event. The desktop branch now also builds the deferred payload at click time (buildDeferredPayload(handoff.dest, handoff.invite)) and hands it to the modal, so the phone that scans lands on the caller'sdest.src/components/Migration/ScanToDownloadModal.tsx/DownloadQR.tsx: optionalpayloadprop threaded to the QR URL (${origin}/app?${payload}; bare${origin}/appwhen absent). See divergence 5 — PR 1 owns this prop and supersedes it on merge.src/constants/migration.consts.ts:LANDING_DOOR: 'landing_door'.src/app/shhhhh/ShhhhhLandingPage.test.tsx(10 tests): surface + settled impression flag, no impression and no action during the pre-auth flash, campaign tag queued before the intercept, sticky bar hidden while the modal is open, hand-off from the door and from the waitlist link, campaign-tagged link, flag-off keeps/setup?redirect_uri=%2Fcard, signed-in visitor still joins the waitlist and is never intercepted.src/hooks/__tests__/useGuestStoreHandoff.test.tsx(9 tests): phone bounce carries the caller surface, default surface, desktop modal, handoff reaches the desktop QR payload, bare QR without a handoff, no interception inside Capacitor, no impression inside Capacitor, no interception with the flag off, impression event surface.No new i18n strings, no new copy.
How verified
pnpm test -- src/app/shhhhh/ShhhhhLandingPage.test.tsx src/hooks/__tests__/useGuestStoreHandoff.test.tsx→ 2 suites, 19 tests, all pass.pnpm test -- src/components/Migration/__tests__/MigrationDownloadModal.test.tsx src/components/Invites/InvitesPage.test.tsx src/components/Profile/components/__tests__/PublicProfile.test.tsx(the otheruseGuestStoreHandoff/ migration consumers) → 3 suites, 53 tests, all pass.pnpm lint --quieton the touched files → clean.prettier --check→ clean.pnpm typecheck→ 6 pre-existingTS2307 Cannot find moduleerrors, none in a file this PR touches:src/utils/web-vitals-shim.ts(x3),src/utils/__tests__/web-vitals-shim.test.ts,src/utils/app-review.ts(x2),src/utils/native-settings.ts. The worktree'snode_modulesis a symlink to the shared checkout andweb-vitals,@capacitor/app-launcher,@capgo/capacitor-in-app-reviewandcapacitor-native-settingsare physically absent from it (verified byls). Environment gap, not a type error in source; the count is identical before and after this PR. CI runs a real install and should be clean.Divergences
openStoreitself. The brief split the two branches betweenuseGuestStoreHandoff(desktop) and a directopenStore(store, LANDING_DOOR, {dest:'/card'})(phone). The hook already owns both branches and hardcodedGUEST_FLOWin three places, so a direct call would duplicate the device split and leave the desktop modal reportingguest_flowwhile the phone reportedlanding_door. Instead the hook takes an optionalsurface, defaulting to the old value, and the door passesLANDING_DOORonce.LANDING_DOORis added here as well as in PR 1. PR 4 cannot compile without it and both branch off the sameorigin/dev. Same key, same value as the brief specifies. For the merger: if PR 1 lands first, drop this one line on rebase — the key and the value are identical, there is nothing to reconcile./setupleft on the door when the flag is on, and/setup?step=signupis the same closed web signup.queuePendingBadgeCampaignsis hoisted above the intercept (behaviour-preserving — the campaign branch already queued before its own!usercheck), so the tag is in the cookie queue thatbuildDeferredPayloadreads and it rides the Android install referrer, the iOS clipboard hand-off and the desktop QR payload;applyDeferredPayloadre-queues it inside the app. Correction to the first round of this PR, which claimed the tag could not survive — it can, via the deferred-link mechanism built for exactly this. It is still lost for a desktop visitor who ignores the QR and installs by hand from a store listing. Flag off, the campaign path is untouched.pnpm typecheckis not clean — 6 pre-existing errors from the incomplete sharednode_modules, itemized under "How verified".payloadonDownloadQR/ScanToDownloadModallands here, ahead of PR 1. PR 1 owns this prop, but without it PR 4's desktop branch dropped{dest:'/card'}on the floor and the QR encoded a bare/app, so a desktop visitor who scanned landed on/homeinstead of the door's destination — the stated purpose of the PR. Same prop name and semantics as PR 1's spec. PR-1 reconciliation: keep PR 1's version ofDownloadQR(it also appends&s=<surface>and addssize) and drop this one on rebase. The two other handoff-passing callers (SendLinkActionList,SendWithPeanutCta, both{invite}) get the same upgrade — their desktop QR now carries the invite code and the current page asdest, matching whatopenStorealready does for them on phones.InvitesPageandPublicProfilepass no handoff and stay byte-identical.MIGRATION_GUEST_CTA_SHOWNfor the door is gated on settled auth. The brief says nothing about the impression event; every other migrated guest surface suppliestrackImpressionWhenGuest, and without itlanding_doorwould have hand-off clicks and no denominator. Gated withisFetchingUser(as inSendWithPeanutCtaandPublicProfile) so the pre-auth flash never counts — andhandleCTAreturns early in that window, so a returning signed-in user who taps before auth resolves is not bounced to the store by a full-page navigation with no way back in-tab.Full text:
run/DIVERGENCES-shhhhh.md.Flag-off impact
None on
/shhhhh: the only new behaviour is behindinterceptGuestCta, which returnsfalsewheneveruseMigrationFlag()is false or the app is running under Capacitor, and the routes below it are unchanged. TheisFetchingUserearly return applies in both flag states and is a strict improvement (a tap during the auth round-trip previously misrouted a signed-in user through/setup).LANDING_DOORis a new constant that nothing reads while the flag is off.Outside
/shhhhh, also flag-gated: thepayloadprop is only ever non-empty on a path thatinterceptGuestCtaalready gated on the flag, soSendLinkActionListandSendWithPeanutCtasee the richer QR only with the flag on.