feat: open signup and preserve referral attribution - #3104
innolope-dev wants to merge 22 commits into
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: Repository: peanutprotocol/peanut-ui/.coderabbit.yaml Review profile: CHILL Plan: Advanced 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: 8787.29 → 8802.82 (+15.53) 🆕 New findings (139)
…and 119 more. ✅ Resolved (136)
…and 116 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 32 screens moved59 of 150 shots changed · 91 identical · baseline
gone 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. |
![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 — changes requested
Exact-head CI is green, but the attribution implementation has one signup-blocking header defect plus five privacy, handoff, navigation, and native-durability defects.
Findings
-
BLOCKING · src/hooks/useZeroDev.ts:141 · Keep attribution out of passkey fetch headers
The live task requires source evidence to attach through Peanut's authenticated API after registration, but this raw JSON is spread by @zerodev/webauthn-key into both register fetches. A landing URL such as?utm_campaign=東京passescleanValue; Fetch rejects that non-ByteString header before/register/options, so a valid campaign link prevents account creation. Move delivery post-registration as specified (or at minimum encode and cap an ASCII envelope) and make attribution failure fail open. -
MAJOR · src/utils/deferred-link.ts:120 · Keep the Play install referrer under its transport limit
On a normal tagged/es-419/send-money-to-argentinalanding,firstTouch,firstContentTouch, andlastTouchrepeat the tags; this line puts the whole JSON beforedest, and the final encoded referrer is 1,392 characters. Google Play documents a 512-character encoded referrer ceiling, so Play can truncate or drop the context and the trailing destination. Use the task's short opaque handoff token and assert the final encoded-referrer bound. -
MAJOR · instrumentation-client.ts:40 · Capture attribution on client-side navigation
captureSignupAttribution()has no production caller after this one-time module initialization. If a visitor opens/and client-navigates to a content page or a later tagged CTA before signup,firstContentTouchandlastTouchnever update; the unit test's second manual call is not wired to the app. Drive capture from App Router navigation or entry events and add a wiring-level regression test. -
MAJOR · src/utils/signup-attribution.ts:145 · Honor disabled and unavailable analytics state
Capture always creates a journey UUID and touches and labels themenabledwithout checking the app's collection state. A browser already opted out therefore still sends account-linkable source evidence during registration. The live task requires disabled or unavailable state to retain no identifiers or touches and requires opt-out/account-switch cleanup; model that state explicitly and recheck it before delivery. -
MAJOR · src/utils/signup-attribution.ts:36 · Reject identifier-shaped campaign values
cleanValuestrips controls and enforces length only. A wallet-address- or UUID-shaped UTM value is accepted, copied into store or clipboard handoffs, and attached to account registration—the exact identifier leak the prior privacy review excluded. Apply the approved ASCII grammar and reject identifier-shaped tags during both capture and restore. -
MAJOR · src/utils/signup-attribution.ts:172 · Persist native pending attribution in device storage
Native restore persists pending evidence only throughdocument.cookie. On a native shell where WebView cookie state is absent after restart—a state existing auth code already handles—the install journey disappears before registration with no pending delivery or retry. The current task requires Capacitor Preferences plus acknowledged, retryable post-auth attachment; clear it only after acknowledgment, expiry, opt-out, or account switch.
Checked clean
- Verified the detached worktree head, exact base SHA, merge base, trusted author, and dev base ref.
- Reviewed the full diff and surrounding passkey, logout, deferred-link, native capability, cookie, PostHog, and setup-completion paths.
- Checked the live TASK-22460 brief and current attribution architecture: post-registration authenticated attachment, app-owned privacy state, short opaque handoffs, and Preferences-backed native pending storage are required.
- Checked the live Lexicon: Registered means account created; signup attribution itself is not defined there.
- Inspected the installed ZeroDev WebAuthn source: custom headers are spread directly into both registration fetches.
- Reproduced non-Latin-1 Fetch header rejection and a 1,392-character encoded Play referrer from a normal tagged localized content landing.
- All exact-head CI completed successfully, including unit, typecheck, ESLint, format, native export, design-system checks, CodeQL, and preview deployment.
- Focused local Jest could not run because the detached worktree has no node_modules; the exact-head unit job passed in CI.
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: 60f839fcce80 · Context: repo, product, notion, provider · Took 23m
…p-attribution # Conflicts: # src/app/ClientProviders.tsx # src/components/Setup/Views/SignTestTransaction.tsx # src/context/authContext.tsx
innolope-dev
changed the title
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Three findings: direct native signup never creates attribution, oversized handoffs can still break the download modal, and stale setup captures leave screenshot CI red.
Findings
-
MAJOR · src/hooks/useZeroDev.ts:140 · Create attribution for direct native signups
On a fresh iOS or Android install without a deferred handoff, both instrumentation paths deliberately skip capture. Registration reaches this marker, but attachSignupAttribution then finds neither a cookie nor Preferences value and returns false, so the backend record and signup_completed event have no journey or native platform and the pending marker keeps retrying. Create and persist a native context when registration starts or completes (subject to analytics availability), then mark it pending only when that context exists. -
MINOR · src/app/(setup)/setup/page.tsx:285 · Remove the retired setup-session captures
All four capture_after locale jobs fail at this head because fixture-setup-pending and p50-setup-session still expect /setup but now reach /setup/finish after the existing-session interstitial was removed. Remove or update those stale page/catalogue entries and their journey assertions so the screenshot workflow reflects the intended redirect and returns green. -
MAJOR · src/components/Setup/setup-entry.ts:31 · [claude-opus] Product truth still says Peanut is invite-only with a Jail waitlist
This PR retires the waitlist/Jail gate (JoinWaitlistPage.tsxandSetup/Views/JoinWaitlist.tsxdeleted,resolveSetupEntryStepno longer gates on an invite, allhasAppAccessUI gates dropped). The product source of truth still states the opposite, and so does live customer-facing copy: -
/home/chip/mono/product/app.md:28access_model: invite-only-skip-waitlist,:29waitlist_size: ~5000,:55"Peanut is invite-only. ~5,000 people on the waitlist.",:59-68"How the Waitlist Works" / "Ways to Skip the Waitlist" (Peanut Jail, prisoner number, promo codes),:77onboarding step 2 "Waitlist bypass". -
/home/chip/mono/product/quick-ref.mdand/home/chip/mono/content/help/waitlist-invite-codes/en.md:20("Peanut is invite-only. When you sign up without an invite code, you land on the Peanut Jail screen.") — a published help article. -
/home/chip/mono/content/press/en.md:7-8boilerplate: "Currently invite-only." -
/home/chip/mono/product/send-links.md:71-79documents that a Link claim "accepts that invite before it claims the link" and that a sender without a username makes "the claim fails with a missing-inviter error". This PR deletes exactly that code path insrc/components/Claim/Link/useInitialClaimFlow.ts(theinvitesApi.acceptInviteblock);errors.missingInviternow has no caller in the repo.
The code is the new intent; the docs and generated content are what is wrong. Fix: update product/app.md (access_model, waitlist_size, § Access & Invites, § Onboarding Flow) and product/send-links.md § "A Link claim clears the invite gate" through the product-fact/update-content path, then regenerate content/help/waitlist-invite-codes and the press boilerplate, so support and SEO copy don't keep telling users they need an invite after this merges.
- MAJOR · src/services/signup-attribution.ts:31 · [claude-opus] POST /users/me/signup-attribution does not exist in peanut-api-ts
attachSignupAttributionposts{ attribution: <json string> }to/users/me/signup-attribution. The pinned peanut-api-ts policy branch has no such route:signup-attribution/signupAttributionappears nowhere under itssrc/orprisma/, and the route is also absent from this repo'ssrc/types/api.openapi.json. OnlyPOST /invites/accept,GET /users/me, etc. exist.
If the UI half merges alone, every signup posts to a nonexistent path, apiFetch returns not-ok, line 36 throws, and the caller in src/hooks/useZeroDev.ts plus the retry in src/context/authContext.tsx:153 swallow it into a Sentry warning. Because clearSignupAttribution() only runs after a 2xx, the cookie and the signup-attribution-pending marker survive, so the failing POST and its Sentry event repeat on every authenticated app start until the 90-day cookie expires — and no attribution is ever recorded, which is the feature's whole point.
This normally ships as a pair and the server half is most likely an open peanut-api-ts PR that this checkout cannot see, so treat it as a sequencing requirement rather than a defect in this diff: the API route (path, { attribution: string } body shape, and the idempotent finalization the comment at lines 11-16 promises) must be deployed before or with this. The PR does not claim the two sides already agree.
- MAJOR · src/components/Invites/InvitesPage.tsx:142 · [claude-opus] Open signup leaves hasAppAccess=false, so a new user's own referral link cannot attribute
peanut-api-ts still keys inviter eligibility onhasAppAccess, which defaults to false:prisma/schema.prisma:514(@default(false)), andsrc/routes/invite.ts:147resolves a personal invite onlyif (inviter?.hasAppAccess)(same check at:128and:136). When resolution fails,POST /invites/acceptreturns 400 'Invalid Invite' (src/routes/invite.ts:246-248).
The only things that flip the flag are accepting an invite (invite.ts:271-274), a badge capability grant (src/acknowledgments/capabilities.ts:76), and the waitlist-unlock cron, which waits 20 minutes to 48 hours per user (src/jobs/waitlist-unlock.ts:19-20,27-41). This PR removes the last in-product surface that made a no-invite signup gain access promptly, while simultaneously un-gating the referral surfaces for every account (src/components/Invites/InvitesPage.tsx:141-142 drops user.user.hasAppAccess, PublicProfile.tsx:208, and en.json:3186 now promises "Peanut is open to everyone. Your personal link still tracks referral rewards.").
Net effect after merge: a user who signs up without an invite is shown their referral link immediately, but for up to 48 hours anyone using it gets a 400 from /invites/accept; the invitee's settlePendingInviteAttribution marks it retryable, extends the cookie 30 days and fires invite_accept_failed to Sentry/PostHog on every app start (src/services/pending-invite-attribution.ts:75-84). If the API half retires the waitlist-unlock cron without also defaulting hasAppAccess to true (or dropping the inviter gate) and backfilling existing rows, those referrals never attribute at all — contradicting product/rewards.md's referral promise. src/jobs/lifecycle-email.ts:750-754 also filters on hasAppAccess: true, so the same cohort silently drops out of lifecycle email during the window.
The server-side counterpart is presumably an open peanut-api-ts PR not in this checkout; name it explicitly in the PR description and land it first, or state which path grants access to a no-invite signup.
- MINOR · src/services/signup-attribution.ts:40 · [claude-opus] No test for the signup-attribution attach, the one new server-mutating module
CONTRIBUTING.md:519: "if code moves money or mutates shared state, it needs a test before merge."src/services/signup-attribution.tsis the only new module that mutates server state (POST to/users/me/signup-attribution, writing source evidence onto the user record) and it has no test file; the added tests coversrc/utils/signup-attribution.ts,src/utils/deferred-link.tsand the sibling invite settlement only.
The untested case is the invariant the code's own comment states: on a non-2xx the device copy must be kept. Nothing asserts that a 500/404 leaves the signupAttribution cookie and the signup-attribution-pending marker intact for the authContext retry, nor that clearSignupAttribution() runs only after an acknowledged write, nor that the attachInFlight guard collapses the concurrent useZeroDev (registration) and authContext (user-change effect) calls into one POST. Moving clearSignupAttribution() above the response.ok check would destroy every user's attribution in production and no test would fail. The sibling src/services/__tests__/pending-invite-attribution.test.ts already tests exactly these three properties for invites — mirroring it here is a short file.
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MAJOR · src/utils/deferred-link.ts:120 · Fall back safely when attribution exceeds the referrer cap — task
Checked clean
- Exact detached HEAD, supplied base SHA, base ref, and merge base all match
- Exact-head unit, typecheck, eslint, format, native-export, ds-shots, and aggregate CI checks are green
- Failed screenshot jobs were inspected: every after-capture locale fails only fixture-setup-pending and p50-setup-session; every before-capture locale succeeded
- Registration, authenticated retry, invite settlement, logout cleanup, client navigation capture, deferred restore, and analytics privacy validation were traced end to end
- Canonical Lexicon, relevant history, churn, and the policy-branch API sibling were checked; the companion open API PR is not present in the sibling checkout
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: 4 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: e490f7c1df7e · Context: repo, product, ci, history, sibling · Took 22m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One major attribution defect remains: external document referrers are recorded as last touch instead of first touch.
Findings
-
MAJOR · instrumentation-client.ts:41 · Include the initial referrer in the first touch
On a normal production web landing, this eager capture creates the journey with the defaultincludeDocumentReferrer: falsebefore the React navigation effect runs. WhenSignupAttributionNavigationCapturelater passestruefor that same document,captureSignupAttributionpreserves the existingfirstTouchand writes the external referrer only tolastTouch. For example, a user arriving fromhttps://example.com/articleat/blog/creator-guideis therefore stored with nofirstTouch.referrerHost, corrupting first-touch source attribution. PassincludeDocumentReferrer: truein this initial capture (while keeping later SPA entries referrer-free), or make the navigation effect the sole owner of initial journey creation. -
MINOR · src/components/Invites/InvitesPage.test.tsx:141 · [claude-opus] Widened auto-claim gate has no no-app-access test
MISSING TEST. This PR widens the gate on the mutation that creates the immutable referral edge and grants app access, and no test exercises the newly admitted case.
Before: const isInviteAutoClaim = !redirectUri && user.user.hasAppAccess && hasValidInvite (and the matching spinner branch at line 126). After: the hasAppAccess term is gone, so a logged-in visitor without app access now auto-claims — it calls acceptInvite, which writes the invites row and flips hasAppAccess in peanut-api-ts. That edge is what drives referral revenue share.
Every case in src/components/Invites/InvitesPage.test.tsx runs with hasAppAccess: true (set in beforeEach at line 141; it is the only occurrence in the file besides the type at line 20). So all existing assertions pass identically against the old and the new predicate, and the behaviour change this PR is actually making is unasserted.
The untested case, precisely: authenticated visitor with hasAppAccess: false, no redirectUri, validation returning onboardingResolved: true with a username — assert acceptInvite/auto-claim fires and the CTA is suppressed. Add it to InvitesPage.test.tsx.
Checked clean
- Confirmed the detached worktree, trusted author, base, and exact head SHA.
- Rechecked P1-P25 across passkey headers, referrer sizing, analytics-state validation, native persistence and cleanup, signup completion, retired copy, and SPA referrer replay; their code defects are fixed at this head.
- Verified invite settlement keeps the invite through unsuccessful or unresolved acceptance and clears it only after a terminal campaign handoff or confirmed referral edge.
- Verified open-signup gate removal still routes authenticated incomplete accounts through the existing /setup/finish account-setup guard.
- Core exact-head CI is green for unit tests, typecheck, ESLint, formatting, native export, and screen tests; visual capture jobs were still running when checked.
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: 0f7ccfe385cf · Context: repo, product, ci · Took 14m
|
Addressed the review’s missing-test note in bfef98d: |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Found one minor attribution-persistence edge: the JSON bound does not account for percent-encoded cookie size.
Findings
-
MINOR · src/utils/signup-attribution.ts:226 · Bound attribution by encoded cookie size
canonicalSignupAttributionaccepts candidates by raw JSON length, butsaveToCookiepercent-encodes that JSON and browsers commonly cap a cookie near 4096 bytes. A valid context with allowed 128-character space-containing UTM fields and a slash-heavy 512-character/blog/...path is about 2.99 KB as JSON after the current touch pruning but about 5.29 KB as thesignupAttributioncookie; that write can be rejected or evicted, so setup later creates a new organic journey and loses the campaign. Bound the actual encoded cookie name/value size (or move the context out of cookies) and add a maximum encoded-size regression. -
MAJOR · src/components/Invites/InvitesPage.tsx:142 · [claude-opus] Direct signups cannot refer anyone — the API still gates inviter eligibility on hasAppAccess
Removing the hasAppAccess condition here makes any authenticated visitor auto-claim an invite, but the other side of the contract has not moved. In peanut-api-ts, resolveInviteAttribution rejects an inviter without app access at src/routes/invite.ts:128, :136 and :147 ('Only users with hasAppAccess=true can send invites', :18). prisma/schema.prisma:514 defaults hasAppAccess to false, and the only writers are invite acceptance (invite.ts:272), a skip:app-jail badge (acknowledgments/capabilities.ts:76) and jobs/waitlist-unlock.ts:68 — none of which fire for someone who just signs up directly now that signup is open.
Failure: Alice signs up directly (hasAppAccess=false). She shares her code. Bob signs up with it. POST /invites/accept resolves attribution to null and, with no campaign tag, returns 400 'Invalid Invite' (invite.ts:246). settlePendingInviteAttribution reads that as status 'retryable', calls extendInviteForRetry(30) and keeps the cookie, so the authContext effect retries the same 400 on every app start indefinitely. Alice is never credited and Bob's referral rewards never exist.
Second, sharper case: the fallback loop at invite.ts:143-148 does not stop at a no-access inviter, it continues to the next candidate. So code maria23, where maria23 is a direct signup, skips her and credits the different real user maria — exactly the misattribution PublicProfile.tsx:112 warns about, now reachable for ordinary users rather than only typos.
As with the attribution endpoint, the API half is presumably an unseen open PR; the PR description does not claim it is already merged, so major. The API needs either to grant hasAppAccess on registration or to drop it as the inviter-eligibility predicate, and the fallback loop should fail closed rather than walk past an ineligible inviter.
- MINOR · src/context/authContext.tsx:302 · [claude-opus] Logout-time clearing of signup attribution is untested
authContext.tsx:302 addsawait clearSignupAttribution()to the logout path, mutating durable shared device state (localStorage plus native Preferences) whose purpose is to stop the next account on the device inheriting the previous account's referral journey — i.e. it guards a reward-attribution leak between users.
The case with no test: log in as user A with a stored signup attribution context, log out, and assert the stored context is gone before the next registration can read it. src/context/tests/authContext-logout.test.tsx was touched by this PR but only to add a pending-invite-attribution mock; its four tests cover the latch, the tab state and server-side revocation, and none assert on the attribution clear. @/utils/signup-attribution is not mocked there, so a regression that dropped or un-awaited the call would pass silently. The ordering matters specifically because prior review (P17) already had to fix this call not being awaited — nothing now pins that fix.
Checked and not raised again
- MAJOR · src/components/Setup/setup-entry.ts:31 · [claude-opus] Product truth still declares Peanut invite-only with a waitlist — this review checked it and does not believe it. No task filed.
- MAJOR · src/services/signup-attribution.ts:37 · [claude-opus] POST /users/me/signup-attribution does not exist in peanut-api-ts — this review checked it and does not believe it. No task filed.
Checked clean
- Pinned detached HEAD, trusted author, base ref/SHA, merge base, PR metadata, and the full 63-file change surface.
- Exact-head unit, typecheck, lint, format, native-export, screen, and aggregate CI checks are green; design screenshots were still running.
- Open-signup routing across setup, authenticated layout, invite pages, claims, public profiles, retired waitlist surfaces, and legacy hasAppAccess=false coverage.
- Signup-attribution capture, referrer handling, validation, web/native persistence, authenticated attachment, completion ordering, opt-out cleanup, and account-switch cleanup.
- Deferred-link 512-character transport pruning, invite preservation, authenticated referral retries, and immutable-edge settlement.
- Paired API head d097ad76 implements the authenticated attribution route and direct-signup inviter eligibility; paired mono head c3d168e7 defines open signup, and both PR bodies require product then API then UI release order.
- Live Lexicon terminology, canonical product sources, correctness, privacy/security boundaries, adversarial failure scenarios, and slop.
- Reproduced the remaining edge with an allowed context measuring 2987 raw JSON characters and 5287 encoded cookie name/value characters.
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: 4 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: bfef98dc4493 · Context: repo, product · Took 22m (queued 4m)
|
Also addressed the logout-coverage note in eb787e8: |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Found one major defect: terminal invalid invite codes are retried indefinitely and can later attach the wrong inviter and reward edge.
Findings
-
MAJOR · src/services/pending-invite-attribution.ts:82 · Stop retrying terminal invite codes
Native/invite?code=typolinks go straight to setup and stash the code without validation. When/invites/acceptreturns a terminal unsuccessful result, this branch extends both invite cookies for 30 days; authenticated recovery then repeats the call and refreshes that expiry on every later start. If that username is registered later, the stale code can create an immutable referral and reward edge to the wrong person. Preserve a retryability/status discriminator fromacceptInvite, clear terminal invalid/forbidden codes, and extend only network or 5xx failures. -
MAJOR · src/components/Invites/InvitesPage.tsx:142 · [claude-opus] A direct signup's referral link fails until the unlock cron reaches them
Opening signup lets a user reach the app withhasAppAccess=false(prisma/schema.prisma:514 defaults it to false, and only an accepted invite, an app-jail badge, or the unlock cron flips it). This PR removes the last FE gate on that flag —src/app/(mobile-ui)/layout.tsxno longer routes such a user to JoinWaitlistPage — so a direct signup lands on /home with the full invite/referral UI (InviteFriendsDrawer, Invites page) and a shareable link.
peanut-api-ts still refuses them as an inviter. resolveInviteAttribution bails on !inviter?.hasAppAccess at src/routes/invite.ts:128, :136 and :147; with no attribution and no badge campaign, POST /invite replies 400 { error: 'Invalid Invite' } (invite.ts:248).
Failure scenario: Alice signs up with no invite code (now possible), immediately shares her link, Bob taps it and completes signup. Bob's settlePendingInviteAttribution call gets a 400, the FE records invite_accept_failed and retries for 30 min, and no referral edge or reward is created for Alice. The window is bounded — jobs/waitlist-unlock.ts unlocks her 20 min to 48 h after signup (computeUnlockTime, MIN_DELAY_MS/MAX_DELAY_MS) and then she works normally — so this is a silent up-to-48-hour dead zone on the referral program, not a permanent one. It also means every direct signup now passes through that cron and receives the waitlist.unlocked push/email for a waitlist they never saw.
The backend half is most likely an unseen open peanut-api-ts PR; this checkout is pinned to the policy branch. Fix: land the API change that makes an uninvited account eligible as an inviter (grant app access at registration, or drop the hasAppAccess condition from resolveInviteAttribution) and retire or re-scope the waitlist-unlock cron and its waitlist.unlocked notification before this merges.
Checked and not raised again
- MAJOR · src/components/Setup/setup-entry.ts:31 · [claude-opus] Product truth still declares Peanut invite-only with a waitlist — this review checked it and does not believe it. No task filed.
- MAJOR · src/services/signup-attribution.ts:37 · [claude-opus] POST /users/me/signup-attribution does not exist in peanut-api-ts — this review checked it and does not believe it. No task filed.
Checked clean
- Verified the detached worktree HEAD and merge base against the supplied exact SHAs.
- Reviewed signup-attribution capture, consent gating, validation, cookie/native persistence, API attachment, completion ordering, and logout cleanup.
- Reviewed Android Play and iOS deferred handoffs, including the encoded referrer cap and invite-preservation pruning order.
- Reviewed open-signup route gates, authenticated claim/invite behavior, public profiles, setup flow, and retired waitlist surfaces.
- Checked the canonical Notion Lexicon, current mono product truth, the documented coordinated release order, and the sibling API policy branch.
- Exact-head CI is green for unit, typecheck, native-export, eslint, and format; local focused tests could not run because the detached worktree intentionally has no node_modules.
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: 3 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: eb787e861937 · Context: repo, product, sibling, ci · Took 21m
|
The API-contract concern is already covered by companion API PR #1579: its current resolveInviteAttribution path accepts any non-deactivated inviter and no longer checks hasAppAccess; the schema default is true. The release order still requires API deployment before this UI ships. I also fixed the new terminal-code retry finding in 77758bd. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Three major issues remain: transient invite responses can erase referrals, and durable invite and signup-attribution retries are not account-bound.
Findings
-
MAJOR · src/services/pending-invite-attribution.ts:25 · Bind pending invite retries to the authenticated account
This service retains an invite across retryable failures but records no account identity. If signup A's accept call fails and A's session later expires without explicit logout, logging into account B runs this same recovery path with A's durable cookie; B can receive A's referral, or a terminal response for B clears A's retry. Persist the first authenticated user ID with the pending invite, require it on recovery, and key or invalidate the in-flight attempt by that binding. -
MAJOR · src/services/invites.ts:75 · Keep referrals after transient HTTP throttling
Every non-5xx HTTP response is classified as terminal. A temporary 429 rate limit (or 408/425) therefore returns retryable=false, after which the pending-attribution service clears the invite and permanently loses the referral edge. Treat transient 4xx statuses as retryable, as the badge-claim path already does, while keeping invalid and forbidden invite responses terminal; add a 429 regression case. -
MAJOR · src/context/authContext.tsx:155 · Bind signup attribution recovery to its registrant
The provider calls attachSignupAttribution for every authenticated profile, but the pending marker and context contain no user ID. If signup A's POST fails and the session expires passively, a later login as B uploads A's journey to B or receives a terminal acknowledgement and clears A's marker. Bind the marker to registeredUser.user.userId after hydration, pass the current user ID into recovery, and deduplicate per binding so another account cannot join the old request.
Checked and not raised again
- MAJOR · src/services/pending-invite-attribution.ts:82 · [moonshotai/kimi-k3] Stop retrying terminal invite codes — this review checked it and does not believe it. No task filed.
- MAJOR · src/components/Setup/setup-entry.ts:31 · [claude-opus] Product truth still declares Peanut invite-only with a waitlist — this review checked it and does not believe it. No task filed.
- MAJOR · src/services/signup-attribution.ts:37 · [claude-opus] POST /users/me/signup-attribution does not exist in peanut-api-ts — this review checked it and does not believe it. No task filed.
- MAJOR · src/components/Invites/InvitesPage.tsx:142 · [claude-opus] Open signups cannot refer anyone — the API still gates inviter eligibility on hasAppAccess — this review checked it and does not believe it. No task filed.
Checked clean
- Verified the detached worktree head and supplied base/merge base, then inspected the signup, referral, deferred-link, logout, setup-routing, claim, and open-access diffs.
- Exact-head CI passed unit, typecheck, eslint, format, native-export, preview, screen-tests, and the aggregate ci-success gate; visual capture jobs were still running.
- Rechecked the previously resolved attribution and deferred-link paths: passkey headers exclude attribution, document referrer is captured once, payload and encoded-cookie bounds are enforced, native state is recovered before signup completion, and explicit logout awaits cleanup.
- Consulted the live Lexicon and current mono product sources. Mono still describes invite-only access, but this PR explicitly gates UI release on publishing mono PR #203 and deploying the companion API first.
Security review by moonshotai/kimi-k3: 1 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: 3 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: 77758bde215d · Context: repo, product · Took 21m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings at the supplied head.
Checked and not raised again
- MAJOR · src/components/Setup/setup-entry.ts:31 · [claude-opus] Product truth still declares Peanut invite-only with a waitlist — this review checked it and does not believe it. No task filed.
- MAJOR · src/services/signup-attribution.ts:37 · [claude-opus] POST /users/me/signup-attribution does not exist in peanut-api-ts — this review checked it and does not believe it. No task filed.
- MAJOR · src/components/Invites/InvitesPage.tsx:142 · [claude-opus] A direct signup's referral link is permanently discarded until the unlock cron reaches them — this review checked it and does not believe it. No task filed.
Checked clean
- Confirmed the detached worktree head, merge base, trusted author, base ref, and pull-request metadata match the supplied review target.
- Reviewed invite stashing, account ownership, concurrent settlement, terminal versus retryable HTTP failures, and stale-invite replacement; the current head binds settlement to the authenticated user and preserves only retryable failures.
- Reviewed signup-attribution consent gating, canonicalization, native and web persistence, account-bound retry markers, authenticated upload, signup-completion ordering, and logout cleanup.
- Reviewed initial-document versus client-navigation capture, Play referrer size pruning, passkey request headers, direct native signup, open-signup routing, and send-link referral settlement.
- Checked the live Lexicon, current mono product truth, the sibling API policy checkout, and the pull request's explicit content-then-API-then-UI release order; open companion changes are prerequisites and their absence from policy checkouts is not evidence of a defect in this head.
- Exact-head unit, typecheck, lint, formatting, native export, screen-test, capture, analysis, and aggregate CI checks completed successfully; design-system snapshots and their artifact publication were still running at review completion.
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: 3 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: dddb31981a69 · Context: repo, product, sibling-api, notion, ci · Took 21m
|
English · Español · Español (Argentina) · Português (Brasil) Review preview: 0317113 → 5206beb. Capture complete in all locales. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Two major handoff defects remain: a fitting badge campaign can be discarded in favor of marketing attribution, and the Android-only referrer cap also strips iOS clipboard attribution.
Findings
-
MAJOR · src/utils/deferred-link.ts:235 · Preserve badge campaigns before optional marketing attribution
With a valid first touch containing four 56-character UTM values, the compact payload is about 490 encoded characters; adding a singlebadge_campaign=nitatakes it above 512. This branch deletes the badge campaign first, leaving the attribution token under the cap, so a new Android installation never receives the badge-claim intent (the web cookie does not transfer). Drop or compact marketing attribution before discarding badge identities, and cover a campaign-plus-attribution boundary case. -
MAJOR · src/utils/deferred-link.ts:240 · Do not apply the Play referrer cap to iOS clipboard handoffs
A valid captured first touch with four 64-character UTM values exceeds 512 characters once compactly encoded.buildDeferredPayloadremoves its attribution for every store;openStoreand StoreBadges reuse that result for the iOS clipboard, which has no Play-referrer transport limit. After install the native app cannot recover the original journey or campaign touch, contrary to the intended web-to-native attribution flow. Build a full iOS clipboard payload separately and enforce 512 only when constructing an Android Play URL.
Checked clean
- Confirmed exact detached head, supplied merge base, trusted PR metadata, and clean worktree.
- Exact-head CI unit, typecheck, ESLint, format, native export, and screen checks succeeded; one publish check was pending at review time.
- Read the live Lexicon open-signup definition and current mono product truth; mono#203 is merged.
- Read the companion API PR exact head: the authenticated attribution route exists, new-account access defaults true with migration backfill, and inviter validation no longer gates on hasAppAccess. The documented release order requires API deployment before UI.
- Traced Android referrer, iOS clipboard, badge-campaign queue, native restore, authenticated attribution attachment, and signup-completion consumers.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion: did not run — it reads only the first review of a pull request; the first reviewer checks later rounds. This review is one reviewer short.
Exact head: bf8dc55a5df9 · Context: repo, product, other-repository, ci · Took 6m (queued 21m)
| // durable person-to-person rewards edge, while attribution can still | ||
| // be captured directly by the newly installed app. | ||
| params.delete('dest') | ||
| params.delete('badge_campaign') |
There was a problem hiding this comment.
MAJOR: Preserve badge campaigns before optional marketing attribution
With a valid first touch containing four 56-character UTM values, the compact payload is about 490 encoded characters; adding a single badge_campaign=nita takes it above 512. This branch deletes the badge campaign first, leaving the attribution token under the cap, so a new Android installation never receives the badge-claim intent (the web cookie does not transfer). Drop or compact marketing attribution before discarding badge identities, and cover a campaign-plus-attribution boundary case.
| } | ||
|
|
||
| if (encodeURIComponent(params.toString()).length > MAX_PLAY_REFERRER_LENGTH) { | ||
| params.delete(ATTRIBUTION_PARAM) |
There was a problem hiding this comment.
MAJOR: Do not apply the Play referrer cap to iOS clipboard handoffs
A valid captured first touch with four 64-character UTM values exceeds 512 characters once compactly encoded. buildDeferredPayload removes its attribution for every store; openStore and StoreBadges reuse that result for the iOS clipboard, which has no Play-referrer transport limit. After install the native app cannot recover the original journey or campaign touch, contrary to the intended web-to-native attribution flow. Build a full iOS clipboard payload separately and enforce 512 only when constructing an Android Play URL.
Integrate PR #3104 signup attribution and waitlist removal, then add three benefit steps, enlarge the mobile mascot hero, and align the inviter prompt styling.
|
Superseded by #3392. The exact head of this PR (b516685) is included in the setup branch, including durable signup attribution, optional inviter handling, and waitlist removal. The two later deferred-link review findings were fixed in #3392 (bbed628), and its description preserves the API-before-UI release order. Companion API PR peanut-api-ts#1579 remains open. |
This branch was successfully deployed
1 participant
Summary
signup_completed, then clear both web and native copies after the analytics join key is capturedhasAppAccess=falseinvite routing without flashing the signup CTAhasAppAccess=falseaccountsValidation
pnpm typecheckpassed;pnpm lintpassed with 0 errors and 66 warningsgit diff --check origin/dev...HEADpassedpnpm typecheck, 47 focused tests (including step revisit and referral preservation), and the design-system lint ratchet passedThe initial test invocation used this machine's unsupported Node 26 runtime and reported QR-payment failures; those passed on Node 22, and validation was repeated on the supported runtime.
Release order
The client keeps pending attribution across transport, server, timeout, and throttling failures, but the API-first order avoids relying on retry recovery during rollout.
This order keeps every public source accurate before access changes and prevents the UI caller from preceding its server contract.