fix: review follow-ups — ds-lint font-(--x) weight, SupportDrawer hardware back, landing Log in link, claim receipt cancelledAt - #2990
Conversation
Tailwind 4 compiles a bare font-(--value) as font-weight; a family needs the family-name: type hint. The matcher excluded the untyped form as a family, so text-body-s font-(--my-weight) stacked an off-ramp weight while countWeightStacks returned 0 and --check stayed green (TASK-22101)
#2920 put DS drawers and modals on the LIFO back stack; SupportDrawer is a hand-rolled overlay and was left out, so back with the sheet open navigated the page underneath. It now registers while open and consumes the press (TASK-22118)
peanut.me only offered SIGN UP: a user with an account and an expired session had no visible way back in from the marketing site. #2920 made /setup?step=login land on the passkey Log In step; this links to it from the hero and the sticky mobile CTA, in all four landing locales (TASK-22117)
The receipt derived cancelledDate from events[0], which only holds a date when a claim attempt was recorded — a sender's cancel/reclaim leaves none, so the receipt showed no cancellation date. GET /send-links now carries cancelledAt (peanut-api-ts fix/kyc-qr-sendlink-followups); prefer it, keep the events fallback (TASK-22068)
|
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: 7167.05 → 7167.8 (+0.75) 🆕 New findings (25)
…and 5 more. ✅ Resolved (25)
…and 5 more. |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff —
|
![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
Clean review: the four follow-ups correctly tighten the typography ratchet, add SupportDrawer hardware-back handling, expose the returning-user login route, and prefer the send link's cancellation timestamp. No actionable defect remains, and all exact-head CI checks completed green.
Findings
- MAJOR · src/components/Claim/Claim.tsx:170 · [claude-opus] GET /send-links does not return
cancelledAton the pinned API branch, so the receipt fix stays inert
Claim.tsx now readsclaimLinkData.cancelledAtfrom the GET /send-links/:pubKey payload, and services.types.ts adds it to theSendLinktype. On the peanut-api-ts policy checkout, that route's projection does not select it:SEND_LINK_SELECTin src/routes/send-links/index.ts:40-104 lists pubKey…claimFailureCode, sender and intents but nocancelledAt, andsanitizeSendLink(index.ts:153,178) returns{...rest}from exactly that select, so the field never reaches the wire. The column exists (prisma/schema.prisma:240) and is stamped on cancel (src/routes/claim.ts:330), and /users/history does surface it (src/transaction-intent/history.ts:781), but the claim page does not use that endpoint.
Nothing breaks at runtime — the ?? claimLinkData.events?.[0]?.timestamp fallback preserves today's behaviour — but the bug this PR fixes (cancelled receipt with no date, because a sender cancel leaves no SEND_LINK_CLAIM intent) is not actually fixed until the API side ships. The test comment at src/components/Claim/tests/claim-states.test.tsx:422 asserts the opposite ("GET /send-links now carries the row's own cancelledAt"), which is what makes this worth flagging rather than assuming.
This normally ships as a pair and the peanut-api-ts half is likely an open PR not present in the pinned policy branch, so this is major rather than blocking. Fix: confirm the companion PR adds cancelledAt: true to SEND_LINK_SELECT (and to the cached row shape covered by src/routes/send-links/cache-response-shape.test.ts, which is derived from the same select), and link it here; otherwise soften the test comment to say the field is expected but not yet served.
Checked clean
- Confirmed the detached HEAD, trusted author, dev base ref/SHA, exact PR head, and merge base match the supplied review target.
- Verified the font-weight matcher counts Tailwind's untyped font-(--x) weight shorthand while excluding the typed family-name form; targeted matcher cases and git diff --check passed.
- Traced SupportDrawer registration through the shared LIFO hardware-back stack: it registers only while open, closes through the modal context, consumes the press, and unregisters when closed.
- Traced both landing-page login links through /setup?step=login and resolveSetupEntryStep, and checked the new string is present in every supported locale and the narrowed LandingStrings contract.
- Traced cancelledAt from the send-link wire type into the cancelled receipt, including the legacy events fallback and the current API projection boundary; the new field is optional and does not regress older responses.
- Correctness, security, adversarial, and slop passes found no actionable authorization, data-exposure, failure-path, regression, dead-code, duplication, or architecture-drift issue in the changed behavior.
- All exact-head GitHub checks completed successfully, including unit, typecheck, eslint, format, ds-lint, native export, analysis, screenshots, preview deployment, provenance, report, and ci-success.
- Local Jest was not repeated because the detached worktree has no node_modules; exact-head CI supplied the test evidence instead.
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: 5f0fdcd73b4a · Context: repo, sibling-api · Took 10m
Mirror of the main-targeted fix (#2991): the scheme://host prefix kept `user:pass@` intact, so a crafted https://SECRET@peanut.me/qr/… link would carry that userinfo into telemetry next to the redacted path. Only the host survives now. Also names peanut-api-ts#1525 as the API half of the cancelledAt receipt fix in the claim-states test.
|
Re the claude-opus finding on Claim.tsx:170 — the API half is peanut-api-ts#1525 (adds Also in f4d2b02: mirrored the userinfo redaction fix from #2991 (CodeRabbit finding there) so dev and main carry identical |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The support-back, login-link, typography-ratchet, and telemetry-redaction changes hold, but the cancellation-date fix remains inert against the pinned API response contract.
Findings
- MAJOR · src/components/Claim/Claim.tsx:170 · Return cancelledAt from the send-link API
This value still cannot arrive at runtime: the pinned API's SEND_LINK_SELECT does not select cancelledAt, so sanitizeSendLink cannot include it in either the DB or cache response. For a sender cancellation with no claim intent, events is empty as well, leaving cancelledDate undefined; the new component test passes only because it injects a response shape the route cannot emit. Add cancelledAt to the API projection/response and cover both GET paths, or land that API half before treating this receipt fix as complete.
Checked clean
- Exact head and dev base matched the supplied SHAs; the detached worktree was clean.
- SupportDrawer registers only while open and correctly consumes the LIFO hardware-back press.
- The landing Log in links route to the existing passkey-capable auth landing, with locale key parity covered.
- The Tailwind weight matcher now counts untyped font custom properties while preserving the typed family-name escape.
- Deep-link telemetry removes authority userinfo while preserving the scheme and host; focused tests cover credential and bearer-secret shapes.
- Exact-head aggregate CI, unit, typecheck, lint, format, native export, analysis, and provenance checks passed. The non-gating ds-shots job exited during its Next build without a changed-code diagnostic; preview deployment was still in progress.
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: f4d2b02e6c3f · Context: repo, api · Took 9m
1 participant
Sweep of the small follow-ups filed off #2905 / #2920 / the sendlink-crash investigation. One commit per task.
What
font-(--x)bypassed the typography ratchet. Tailwind 4 compiles a barefont-(--value)asfont-weight; a family needs thefamily-name:hint.WEIGHT_STACK_REexcluded the untyped form as a family, sotext-body-s font-(--my-weight)stacked an off-ramp weight whilecountWeightStacksreturned 0. It now counts the untyped form and excludes onlyfont-(family-name:…); regression cases updated. Nofont-(--insrc/, so the baseline is unchanged (ds-lint --checkok).useBackHandlerwhile open and consumes the press. Tests dispatchdispatchBackPress()open (closes, consumed) and closed (falls through)./setup?step=loginland on the passkey Log In step — this links to it from the hero (under the primary CTA) and from the sticky mobile CTA, with alandingLogInstring in en / es-419 / es-ar / pt-br (messages.testkey-parity green). DStext-body-stoken, no new ratchet debt.Claim.tsxderivedcancelledDatefromevents[0], which only holds a date when a claim attempt was recorded; a sender's cancel/reclaim leaves none. peanut-api-ts#1525 puts the row's owncancelledAtonGET /send-links/:pubKey; the receipt now prefers it and keeps the events fallback. Test asserts the receipt receivescancelledDatefromcancelledAton the cache-hit shape.Verification
tsc --noEmitclean;ds-lint-rules,SupportDrawer,i18n/messages,Claim/claim-statessuites green;node scripts/ds-lint-counts.mjs --checkok; prettier clean.