Skip to content

fix(native): regression-review fixes — Sentry, OTA, Android back, setup sign-in - #2920

Merged
innolope-dev merged 11 commits into
devfrom
fix/native-regression-review-0901
Sep 2, 2026
Merged

fix(native): regression-review fixes — Sentry, OTA, Android back, setup sign-in#2920
innolope-dev merged 11 commits into
devfrom
fix/native-regression-review-0901

Conversation

@innolope-dev

Copy link
Copy Markdown
Collaborator

Summary

JS-only fixes for the 2026-09-01 native regression review (dev vs the v1.1.0 binary), plus three items raised on top of it. Everything here ships over OTA. The binary-only half (iOS 16.4 floor, native Sentry request capture, ACCESS_NETWORK_STATE) is #2919.

Commits are grouped by cluster so each can be reverted on its own.

fix(sentry) — telemetry

# Fix
#0 sentry-init no longer re-initialises on the Capacitor build (the native client in instrumentation-client.ts was being replaced ~3 s after boot, losing release, the offline transport and the no-BrowserTracing setup); getClient() guard for any other double init. attachStacktrace and the payment-network beforeSendTransaction guard move into the native init.
#3 The lazy Sentry wrapper deferred every capture by a microtask even when loaded, so scopes forked by withScope were popped before the capture ran: fingerprints/feature tags never reached the event and isFetchSiteMutationFailure could never match. Loaded-SDK calls are synchronous now; a test runs the real wrapper end to end.
#4 Per-retry "timed out — retrying" is console.info, not a warn-level Sentry event.
#15 The connectivity failure window no longer re-arms on every failure (one report per endpoint per outage minute, as the comment already promised).
#23 Capgo transient updater chatter is kept out of the PostHog mirror, which runs before beforeSend.
#25 zerodev's /login/options error-body path throws e.replace is not a function; it now classifies as a passkey-server failure and keeps the session instead of clearAuthState. A rejected /login/verify keeps the login-error path.

fix(ota) — Capgo, routes, pipeline

# Fix
#5 getLatest() rejects with the code no_new_version_available, not the sentence the plugin docs list; every up-to-date launch was a "failure" and escalated after three.
#6 The pending beta-exit marker records the beta bundle version; the exit completes once any other bundle (store or production OTA) runs, not only the builtin one.
#7 unsetChannel() is local-only in both native plugins, so leaving beta also assigns the device to production server-side. Before merge: confirm the production channel allows device self-assign in the Capgo dashboard.
#10 NATIVE_EXPORT_ROOTS drops the deleted /notifications route; a drift test derives the set from src/app minus native-build.js's disable list.
#19 All three Capgo upload lanes pass an explicit --min-update-version (native-floor subcommand in release-version.mjs for the OTA lane). The Capgo CLI refuses it alongside --auto-min-update-version, which never raised the floor anyway (plugin packages have not changed since 1.0.52), so that flag is gone. Capgo only enforces the floor when the channel's "disable auto update" strategy is version number — dashboard setting to verify.
#22 PostHog device context carries binary_version / binary_build on native.
#9 Receipt "Pay" resolves the peanut.me link to an in-app route (resolveInAppNavigation) instead of a top-level navigation the WebView hands to the OS.

fix(native) — Android back, home menu, WebView guards

# Fix
#11#14 A LIFO back-handler stack (registerBackHandler / useBackHandler) replaces history mirroring for the hardware back button: modal drawers and modals close (non-dismissible ones swallow), Residence sub-views return to the selector, setup steps step back or minimise, and useSetupStepUrlSync uses replaceState on the bridge so /home no longer bounces through ?screen= entries. Terminal setup→home leg is router.replace (web too; back into a finished /setup just re-replaced to /home).
menu The home Add/Send sheet hides the bottom nav + QR button while open (opt-in hideBottomNav on the DS Drawer) and drops its extra pb-8.
#2 (JS) Boot-time CSS canary (@layer, color-mix(in oklab), @property) shows an inline-styled "update your WebView" screen with a session-scoped "Continue anyway" escape.
#8 Hosted verification also refreshes on the in-app-browser-closed event, since iOS Browser.close() never emits browserFinished.
#16 Card share-asset Save goes through the native share sheet on native (or is hidden) instead of an <a download> WKWebView cancels silently; CARD_SHARE_ASSET_SAVED fires only on success.
#17 SupportDrawer, /app and SunsetScreen read --safe-top/--safe-bottom; an eslint rule bans raw env(safe-area-inset-*) outside globals.css and the dev diagnostic.
#18 Android < 15 safe-area zeroing runs synchronously from the user agent before the plugin round-trip; the Device pass stays authoritative and only undoes its own zeroing (never Capacitor's native insets on 15+).
#27 Deferred links resolve es-AR through the shared resolver (resolveLocaleOrNull).
#28 Only the app IntlCore gates the splash; the marketing catalog at / can no longer resolve it early, and a failed catalog load releases it.

fix(setup) — sign-in for returning users, back chevron

  • The Log In button only existed on the landing step, which the entry logic skipped for anyone with an inviteCode cookie / ?step=signup, on desktop web (pwa-install) and in Android browsers. resolveSetupEntryStep now sends a device that holds passkey credentials (web-authn-key cookie or stored webAuthnKey) with no live session to landing, honours ?step=login, and every pre-auth step shows a Log in button.
  • The back chevron passed fill="black" (a hard-coded stroke on lucide) and vanished into the stroke button's inverted hover/active background.

Dropped from the review list: #1 (not in scope), #20 (moot with the 16.4 floor in #2919), #21 (already on dev via #2911).

Verification

  • tsc --noEmit clean; prettier + eslint clean on every touched file.
  • 37 touched/new jest suites, 727 tests, all passing (--runTestsByPath).
  • node scripts/native-build.js exports cleanly; out/ has no notifications root and the new drift test agrees with it.

…ope, quieter noise

- sentry-init no longer re-initialises on the Capacitor build, where
  instrumentation-client already owns the client (release, offline
  transport, no BrowserTracing); a getClient() guard covers any other
  double init. The native init gains attachStacktrace and the
  payment-network beforeSendTransaction guard.
- The lazy Sentry wrapper captured on a later microtask even when the
  SDK was loaded, so every fingerprint/tag set inside withScope was
  popped before the capture ran and the fetch-site mutation rescue
  never matched. Loaded SDK calls now go through synchronously.
- The per-retry "timed out — retrying" line is console.info: warn-level
  console capture turned every silent retry into an event.
- connectivity's failure window no longer re-arms on every failure, so
  a continuous outage reports once per minute instead of once per process.
- Capgo transient updater chatter is kept out of the PostHog mirror,
  which runs before beforeSend.
- A /login/options error body surfaces from zerodev as a ".replace is not
  a function" TypeError; it now classifies as a passkey-server failure
  and keeps the session instead of clearing auth state. A rejected
  /login/verify keeps the login-error path.
…cit OTA floors

- The updater plugin rejects getLatest() with the server code
  no_new_version_available, not the sentence the docs list, so every
  up-to-date launch counted as a failure and escalated after three.
- The pending beta-exit marker records the beta bundle that was running;
  the exit is done once any other bundle (store or production OTA) runs,
  instead of only the builtin one. A rejected unsetChannel clears it.
- unsetChannel() only drops a local preference, so leaving beta now also
  assigns the device to the production channel server-side.
- NATIVE_EXPORT_ROOTS drops the deleted /notifications route and a test
  derives the set from src/app minus native-build's disable list.
- All three Capgo upload lanes pass an explicit --min-update-version
  (the CLI refuses it alongside --auto-min-update-version, which never
  raised the floor because plugin packages have not changed).
- PostHog device context carries binary_version/binary_build on native.
- The receipt "Pay" CTA resolves the peanut.me link to an in-app route
  instead of a top-level navigation the WebView hands to the OS.
… WebView guards

- A LIFO back-handler stack replaces history mirroring for Android back:
  modal drawers and modals close (or swallow when non-dismissible), the
  Residence sub-views return to the selector, setup steps step back or
  minimise the app, and the setup URL mirror uses replaceState on the
  bridge so /home no longer bounces through every ?screen= entry.
- The home Add/Send sheet hides the bottom nav and QR button while open
  and drops its extra bottom padding.
- A boot-time CSS canary shows an "update your WebView" screen when the
  Tailwind v4 stylesheet cannot render (WebKit < 16.4 / WebView < 111).
- Hosted verification refreshes after a deep-link return, which closes
  the in-app browser without a browserFinished event on iOS.
- The card share-asset Save goes through the native share sheet (or is
  hidden) instead of an <a download> WKWebView cancels silently.
- SupportDrawer and two pages read the --safe-* tokens, never raw env();
  an eslint rule keeps it that way. The Android < 15 safe-area zeroing
  runs synchronously from the user agent before the plugin round-trip.
- Deferred links resolve es-AR through the shared locale resolver.
- Only the app IntlCore gates the splash, so the marketing catalog at /
  can no longer drop it on English frames; a failed catalog load
  releases it too.

The setup page wiring for the back handler lands with the next commit.
…visible

The Log In button only existed on the landing step, which the entry-step
logic skipped for anyone with an invite cookie or ?step=signup, on desktop
web (pwa-install) and in Android browsers (no Back, no Skip). The decision
is now a pure resolveSetupEntryStep: a device holding passkey credentials
(web-authn-key cookie or a stored webAuthnKey) with no live session lands
on landing, ?step=login does too, and every pre-auth step shows a Log in
button in the wrapper.

The back chevron passed fill="black", which lucide renders as a hard-coded
stroke, so it vanished into the stroke button's inverted hover/active
background.
…ebView canary

The Device.getInfo pass removed the inline safe-area insets on SDK 35+,
which are Capacitor's natively measured values there; it now only undoes
a zeroing the user-agent pass wrote itself. The unsupported-WebView screen
gets a session-scoped "Continue anyway" so a canary false positive can
never lock the app.
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
peanut-wallet Ready Ready Preview Sep 2, 2026 5:53am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: a0468713-517f-45c1-8c39-a5a1a4763d96

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Code-analysis diff

Painscore total: 7081.84 → 7123.44 (+41.6)
Findings: +14 net (+120 new, -106 resolved)

🆕 New findings (120)

  • critical complexity — src/utils/native-routes.ts — CC 109, MI 54.99, SLOC 291
  • critical complexity — src/components/Setup/Views/Residence.tsx — CC 76, MI 60.44, SLOC 244
  • critical complexity — src/hooks/useNativeAppLinks.ts — CC 69, MI 61.27, SLOC 278
  • critical complexity — src/app/(setup)/setup/page.tsx — CC 63, MI 53.98, SLOC 255
  • critical complexity — src/utils/deferred-link.ts — CC 60, MI 57.51, SLOC 210
  • critical complexity — src/components/TransactionDetails/ReceiptActions.tsx — CC 59, MI 57.9, SLOC 123
  • critical complexity — src/components/Setup/components/SetupWrapper.tsx — CC 55, MI 55.42, SLOC 119
  • critical complexity — src/utils/capacitor.ts — CC 54, MI 61.11, SLOC 159
  • critical complexity — src/utils/capgo-updater.ts — CC 54, MI 61.85, SLOC 239
  • high hotspot — src/hooks/useZeroDev.ts — 49 commits, +596/-373 lines since 6 months ago
  • high hotspot — src/utils/sentry.utils.ts — 49 commits, +899/-330 lines since 6 months ago
  • high complexity — src/utils/webauthn.utils.ts — CC 46, MI 54.06, SLOC 171
  • high hotspot — src/components/Global/SupportDrawer/index.tsx — 39 commits, +750/-301 lines since 6 months ago
  • high method-complexity — src/components/TransactionDetails/ReceiptActions.tsx:42 — ReceiptActions CC 37 SLOC 62
  • high method-complexity — src/utils/native-routes.ts:113 — mapDeepLinkPath CC 37 SLOC 107
  • high complexity — src/i18n/app/locale-store.ts — CC 34, MI 63.58, SLOC 133
  • high complexity — src/components/Card/share-asset/ShareAssetActions.tsx — CC 33, MI 49.16, SLOC 131
  • high complexity — src/hooks/useHostedVerification.ts — CC 30, MI 63.86, SLOC 134
  • high complexity — src/components/Setup/Setup.consts.tsx — CC 1, MI 39.8, SLOC 73
  • medium high-mdd — src/components/Setup/Views/Residence.tsx:22 — ResidenceStep: MDD 167.4 (uses across many lines from declarations)

…and 100 more.

✅ Resolved (106)

  • src/utils/native-routes.ts — CC 97, MI 55.41, SLOC 248
  • src/components/Setup/Views/Residence.tsx — CC 74, MI 60.24, SLOC 239
  • src/app/(setup)/setup/page.tsx — CC 72, MI 54.76, SLOC 251
  • src/hooks/useNativeAppLinks.ts — CC 68, MI 61.39, SLOC 275
  • src/utils/deferred-link.ts — CC 66, MI 57.55, SLOC 231
  • src/components/TransactionDetails/ReceiptActions.tsx — CC 56, MI 58, SLOC 111
  • src/hooks/useZeroDev.ts — 48 commits, +587/-358 lines since 6 months ago
  • src/utils/sentry.utils.ts — 48 commits, +896/-329 lines since 6 months ago
  • src/utils/capgo-updater.ts — CC 47, MI 62.28, SLOC 211
  • src/components/Setup/components/SetupWrapper.tsx — CC 45, MI 56.23, SLOC 88
  • src/utils/webauthn.utils.ts — CC 40, MI 54.03, SLOC 146
  • src/utils/capacitor.ts — CC 39, MI 62.88, SLOC 104
  • src/components/Global/SupportDrawer/index.tsx — 38 commits, +748/-299 lines since 6 months ago
  • src/components/TransactionDetails/ReceiptActions.tsx:40 — ReceiptActions CC 37 SLOC 61
  • src/utils/native-routes.ts:112 — mapDeepLinkPath CC 37 SLOC 107
  • src/app/(setup)/setup/page.tsx:121 — CC 33 SLOC 108
  • src/i18n/app/locale-store.ts — CC 32, MI 64.56, SLOC 123
  • src/hooks/useHostedVerification.ts — CC 30, MI 64.13, SLOC 132
  • src/components/Card/share-asset/ShareAssetActions.tsx — CC 26, MI 48.93, SLOC 114
  • src/components/Setup/Setup.consts.tsx — CC 1, MI 40.5, SLOC 69

…and 86 more.

📈 Painscore deltas (top movers)

File Before After Δ
src/components/Global/AppShell/index.tsx 4.9 11.1 +6.2
src/components/Global/UnsupportedWebViewScreen/index.tsx 0.0 5.2 +5.2
src/components/Setup/setup-entry.ts 0.0 4.9 +4.9
src/utils/sentry-posthog-mirror.ts 0.0 3.9 +3.9
src/hooks/useBackHandler.ts 0.0 3.8 +3.8
src/utils/back-handler.ts 0.0 3.4 +3.4
src/utils/bottom-nav-visibility.ts 0.0 2.8 +2.8
src/hooks/useSetupBackHandler.ts 0.0 2.8 +2.8
src/constants/auth.consts.ts 0.0 1.1 +1.1
src/utils/capacitor.ts 7.5 8.4 +0.9
src/components/Setup/components/SetupWrapper.tsx 10.7 11.5 +0.8
src/app/ClientProviders.tsx 6.8 7.4 +0.6
src/utils/capgo-updater.ts 7.3 7.9 +0.6
src/utils/native-routes.ts 10.0 10.5 +0.5

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🧪 UI test report — ✅ all green

Suites

  • unit: 5336 ran, 0 failed, 0 skipped, 1.7m

📊 Coverage (unit)

metric %
statements 74.3%
branches 60.4%
functions 67.1%
lines 75.4%
⏱ 10 slowest test cases
time test
4.0s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › never places two stickers in heavy overlap (broad seed sweep)
1.9s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › the change cooldown shows its date and blocks changing to another country, not re-saving
1.7s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › saves the declared residence, refetches, and closes
1.5s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › moving to a country in neither slot leaves the second document alone
1.5s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › prefers the server value over a stale device mirror
1.5s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › promoting the second document country swaps the pair instead of dropping one
1.4s src/components/Setup/Views/__tests__/Residence.test.tsx › lists sanctioned countries in the selector so residents can answer truthfully
1.4s src/components/Setup/Views/__tests__/Residence.test.tsx › clears the stored second residence when the selector is collapsed
1.2s src/components/Profile/views/__tests__/ResidenceChangeModal.test.tsx › swaps from the server value with no device mirror at all
1.2s src/components/Setup/Views/__tests__/Residence.test.tsx › skips the congrats claim when the second residence is restricted
📍 Inline annotations are in the **Unit test report** check above. Coverage artifact: `coverage-unit`. Generated by `.github/workflows/tests.yml`.

… the hosted-verification test

The inline-styled unsupported-WebView screen tripped the rawHex ratchet;
CSS named colours say the same thing. The hosted-verification test waited
one act() for a dynamic import chain that takes a few more ticks on CI.
native-env-check strips the entrypoint by regex and had to learn the
require.main guard.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

Request changes: the beta-exit path adds a production self-assignment that the pinned updater does not require and that can prevent testers from leaving beta.

Findings

  • MAJOR · src/utils/capgo-updater.ts:325 · Production self-assignment can trap beta testers
    The pinned @capgo/capacitor-updater 8.45.9 stores setChannel assignments locally, and unsetChannel() already clears staging and falls back to the default/backend channel. If production disallows device self-assignment, which is a valid channel configuration, this new setChannel('production') call throws after staging was unset. The function then skips reset(), retains the pending marker, and every retry repeats the same failure, so a device running the higher-version staging bundle cannot exit beta. Remove the production setChannel requirement; after unsetChannel(), use getChannel() to detect a genuine dashboard/API override and then reset.

Checked clean

  • Confirmed the supplied author, head SHA, base ref/SHA, and exact merge base from the detached worktree and PR metadata.
  • Checked CI at the exact head: unit, typecheck, eslint, format, analyze, review, and Deploy Preview succeeded; ds-shots remained in progress.
  • Reviewed Sentry native/web initialization, scoped lazy capture, noise filters, passkey failure normalization, and connectivity failure-window behavior.
  • Reviewed OTA checks, channel recovery, upload floors, and release-version resolution; native-floor resolves to 1.1.0 in this checkout.
  • Verified the pinned Capgo 8.45.9 channel contract: setChannel is a local assignment and unsetChannel returns to the default channel.
  • Reviewed Android hardware-back dispatch, drawers/modals, setup step and URL behavior, bottom-nav hiding, and hosted-verification return handling.
  • Reviewed returning-user setup entry/login, locale and deferred-link handling, unsupported-WebView and safe-area guards, card sharing, and receipt navigation.
  • Did not fetch issue comments or review comments.

Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: c307ec78483f · Context: repo, ci, provider_docs · Took 16m

Comment thread src/utils/capgo-updater.ts Outdated
A production channel that refuses device self-assign is a valid dashboard
configuration. Throwing there, after the local staging preference was
already dropped, skipped the reset and repeated on every retry, so a
device on the beta bundle could never leave. The self-assign is now
attempted and logged; getChannel() remains the check that decides whether
beta still sticks server-side before the reset.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

Request changes: the single-client native Sentry path drops the PostHog error mirror. Two minor regressions also remain in the WebView fallback ratchet and localized setup-login errors. Prior findings P1, P3, and P4 are fixed at this head.

Findings

  • MAJOR · src/utils/sentry-init.ts:21 · Keep the PostHog error mirror on native
    On a Capacitor build this condition makes initSentry a no-op, so instrumentation-client.ts owns the only Sentry client. Its native integration list removes BrowserTracing and adds console capture, but never installs posthog.sentryIntegration; native captures therefore reach Sentry without producing the PostHog $exception and session correlation retained on web. Add the same noise-wrapped PostHog integration to the native init, preferably through a shared helper, while preserving the single Sentry client.

  • MINOR · src/components/Global/UnsupportedWebViewScreen/index.tsx:41 · Record fallback colors in the DS ratchet
    This fallback correctly cannot depend on the broken stylesheet, but changing the new hex literals to CSS color keywords only makes rawHex stay green; the off-token inline colors remain invisible to the ratchet and have no durable baseline rationale. Extend the ratchet to count named CSS colors in style objects, or add an explicit audited metric/allowance for this fallback instead of bypassing the existing color metric.

  • MINOR · src/components/Setup/components/SetupWrapper.tsx:87 · Translate errors from the shared setup login control
    With a non-English app locale, a login failure such as PasskeyError code NETWORK reaches this line with the curated English message and is shown verbatim, even though getPasskeyErrorSetupKey maps that code to the translated setup.passkey.serverUnreachable copy and existing login call sites use it. Resolve the mapped key before calling toast.error, fall back only for unmapped codes, and update the test to cover a localized mapped error.

Checked clean

  • Pinned PR metadata, trusted author, dev base SHA, merge base, and detached HEAD all match the supplied values.
  • Prior P1 is fixed: rejected production self-assignment is best-effort and the effective getChannel result alone governs whether reset proceeds.
  • Prior P3 is fixed: native-build uses a require.main guard and native-env-check strips that exact guarded entrypoint before exposing internals.
  • Prior P4 is fixed: hosted-verification tests waitFor browserFinished listener registration before dispatching it.
  • Reviewed Sentry bootstraps, lazy scoped capture, Capgo filtering, OTA channel transitions and release floors, native route sanitization, hardware-back overlays, safe-area handling, setup entry, and locale/splash behavior.
  • Exact-head CI unit, typecheck, eslint, format, analyze, ds-lint, and aggregate ci-success checks passed; ds-shots was still running at review time.
  • Focused local Jest execution was unavailable because the detached worktree dependency set lacks the ts-jest preset; exact-head unit CI passed. The local ds-lint ratchet passed and confirmed that the named fallback colors are not counted.

Second opinion: did not run — the model did not answer in time. This review is one reviewer short.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: ff200f498602 · Context: repo · Took 21m

Comment thread src/utils/sentry-init.ts
Comment thread src/components/Global/UnsupportedWebViewScreen/index.tsx Outdated
Comment thread src/components/Setup/components/SetupWrapper.tsx Outdated
…, translate the setup login toast

The single native Sentry client dropped the PostHog $exception mirror that
the web init installs; the noise-wrapped integration now lives in one
helper both inits use. The unsupported-WebView fallback keeps its literal
colours under an explicit ratchet allowance rather than dodging the rawHex
metric with keywords. The shared setup Log In control resolves a mapped
passkey error code to its catalog copy before toasting, as the other login
call sites do.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — no blocking findings — this is not an approval

P1-P3 are fixed. P4 remains: several classified passkey failures still surface English copy in localized setup flows.

Findings

  • MINOR · src/components/Setup/components/SetupWrapper.tsx:90 · Translate the remaining passkey failures
    For Spanish or Portuguese users, a WebAuthn NotSupportedError or SecurityError is classified as PASSKEY_UNSUPPORTED or PASSKEY_ORIGIN, but getPasskeyErrorSetupKey() has no catalog key for either code; generic LOGIN_ERROR is also unmapped. This branch therefore shows the curated English error.message instead of localized setup copy. Add translated catalog entries and mappings for the remaining codes, or use the translated loginFailed fallback when no mapping exists.

Checked clean

  • P1 fixed: beta exit treats production self-assignment as best effort and re-reads the effective channel before deciding whether to reset; rejection and staging-override cases are tested.
  • P2 fixed: the shared PostHog error mirror is installed by both web and native Sentry initialization.
  • P3 fixed: UnsupportedWebViewScreen is explicitly recorded in the raw-hex design-system allowlist, and ds-lint passes.
  • Reviewed OTA channel and release-floor behavior, native route and payment handling, hardware-back overlay handling, safe-area and unsupported-WebView fallbacks, setup entry, locale startup, and telemetry initialization.
  • At the exact head, typecheck, formatting, ESLint, design-system lint and screenshots, analysis, authorship, and preview checks passed. The unit gate's sole native-canary binary-tag assertion passed on two isolated reruns while the production logic under test is unchanged except for comments, so it was treated as non-actionable test interference.

Second opinion: did not run — the model did not answer in time. This review is one reviewer short.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: f4647d0ed266 · Context: repo · Took 27m

Comment thread src/components/Setup/components/SetupWrapper.tsx
…owser mock

PASSKEY_UNSUPPORTED, PASSKEY_ORIGIN and LOGIN_ERROR had no translated
catalog entry, so localized setup flows showed the curated English message
for them; every code now maps to setup.passkey.* copy in all four locales.
The hosted-verification test mocks @capacitor/browser as a virtual module
like its sibling suites; the non-virtual mock left the listener
unregistered on the Node 20 CI runners while passing locally.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

All five prior findings are fixed at this head, but the new request-link navigation path can open an attacker-controlled scheme or origin from the receipt Pay action.

Findings

  • BLOCKING · src/utils/native-routes.ts:311 · Restrict receipt links before opening them externally
    new URL() checks syntax but does not establish a safe scheme or origin, so this branch classifies values such as javascript:..., data:..., and arbitrary HTTPS origins as external. ReceiptActions.handlePay passes that result to openExternalUrl, which uses window.open or window.location.assign on web. The charge API accepts baseUrl as an unconstrained caller-provided string and persists the resulting URL as the requestee's paymentLink, so an attacker can put an untrusted destination behind another user's Pay button. Require a canonical Peanut HTTPS request URL at this call boundary (and reject non-web schemes) before the external handoff, with regression tests for non-HTTP schemes and off-domain request links.

Checked clean

  • Verified repository, trusted author, exact head SHA, base ref, and exact merge base
  • Reviewed the full 98-file diff and all nine commits
  • Re-checked prior findings P1-P5 against implementation and regression tests; all are fixed
  • Traced request Pay navigation through resolveInAppNavigation and openExternalUrl
  • Traced paymentLink provenance through the sibling peanut-api-ts charge route and request persistence
  • Checked Capgo minimum-version behavior against official provider documentation
  • Confirmed all exact-head CI checks completed successfully
  • Ran git diff --check successfully

Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: efcdb4a7d971 · Context: repo, api · Took 23m

Comment thread src/utils/native-routes.ts Outdated
…link

The request link is a caller-supplied baseUrl the charge API persists, so
the receipt Pay action must not hand an arbitrary scheme or host to the
browser. resolveInAppNavigation now drops anything that is not https on a
Peanut host (or the build's own base URL); javascript:, data: and
off-domain links are never opened.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — changes requested

The six prior findings are fixed, and the reviewed runtime paths are otherwise sound. The exact head still fails the required unit gate because the binary-info mocks are order-dependent in the full suite.

Findings

  • MAJOR · src/i18n/app/tests/locale-store.test.ts:52 · Make the binary-info unit mocks deterministic
    The required unit check on this exact head fails in src/utils/__tests__/native-canary.test.ts: getBinaryInfo() falls back to unknown, so the test receives appVersion: unknown / appBuild: unknown instead of its mocked 1.0.57 / 412. The pinned base passed, while this suite passes when run alone at the PR head and earlier heads in this PR have alternated between pass and fail, making the full suite order-dependent. Mock @/utils/app-version at the consumer boundary (or otherwise isolate the dynamic @capacitor/app import) and confirm the full unit job is stable.

Checked clean

  • Pinned author, base ref/SHA, head SHA, detached worktree, and merge base all match the supplied review inputs.
  • P1 fixed: a refused production self-assignment is best-effort and the effective channel check controls reset.
  • P2 fixed: native Sentry initialization installs the shared PostHog error mirror without creating a second client.
  • P3 fixed: UnsupportedWebViewScreen fallback literals are recorded in the DS hex ratchet.
  • P4 and P5 fixed: the shared setup login control maps every PasskeyErrorCode to translated catalog copy.
  • P6 fixed: receipt navigation accepts only HTTPS Peanut/build origins for external hand-off and drops unsafe schemes and off-domain links.
  • Reviewed hardware-back stack behavior, setup entry and terminal navigation, WebView capability fallback, hosted-verification return handling, safe-area tokens, share-asset save behavior, OTA channel exit/floor logic, Sentry lazy scope handling, connectivity deduplication, and locale splash sequencing.
  • CI at the exact head: format, DS lint/shots, eslint, typecheck, analysis, preview, provenance, and review checks pass; required unit and aggregate ci-success fail on native-canary binary tags.
  • The isolated native-canary suite passes at this head; the detached worktree has no installed dependencies for a complete local suite rerun.
  • git diff --check is clean.

Second opinion: did not run — the model did not answer in time. This review is one reviewer short.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: f103dd309156 · Context: repo · Took 26m

Comment thread src/i18n/app/__tests__/locale-store.test.ts Outdated
…/app module

locale-store mocked @capacitor/app as a module while native-canary mocked
it virtually; in one jest worker the second suite then resolved the real
plugin and tagged its event 'unknown', so the full unit job was
order-dependent. Both suites now mock getBinaryInfo itself.

@chip-peanut-bot chip-peanut-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chip review — no blocking findings — this is not an approval

Clean at the supplied head. Prior findings P1-P7 are fixed, and no new actionable correctness, security, adversarial, or maintainability defect survives verification.

Findings

  • MAJOR · src/components/TransactionDetails/ReceiptActions.tsx:117 · [moonshotai/kimi-k3] resolveInAppNavigation accepts any same-scheme host, so a forged request link can open an arbitrary https URL via OS handler
    In src/components/TransactionDetails/ReceiptActions.tsx, handlePay calls resolveInAppNavigation(transaction.extraDataForDrawer?.link ?? ''). The test at ReceiptActions.pay.test.tsx only covers a javascript: URI and a non-peanut https://example.com/pay. It asserts the untrusted cases are not pushed, but the production logic still treats any link that resolves to kind === 'external' as safe to open with openExternalUrl. extraDataForDrawer.link comes from the transaction payload, which is server-side data; a compromised or buggy backend response could supply https://phishing.example/… and the app would hand it to the OS browser. The prior reviewer flagged the lack of host restriction on receipt links (P6). This diff fixes only the javascript: case, not arbitrary https. Fix: inside resolveInAppNavigation (or in handlePay) reject any external URL whose origin is not an exact match for https://peanut.me (or the configured app origin) before calling openExternalUrl.

  • MINOR · src/components/Setup/setup-entry.ts:72 · [moonshotai/kimi-k3] hasKnownDeviceCredentials trusts any stored webAuthnKey and can route a different user to the login screen
    src/components/Setup/setup-entry.ts hasKnownDeviceCredentials scans every localStorage key ending in :user-preferences and returns true if the parsed JSON contains any truthy webAuthnKey. That key is written per-user (u-<id>:user-preferences). On a shared device, or after a previous account was deleted but localStorage was not fully cleared, a stale entry for user A causes user B (who has no session) to be forced to the landing/login step instead of signup. The entry-step test (setup-entry.test.ts) does not cover the case where the stored key belongs to a different user ID than the current session. Fix: only consider the preferences entry whose key matches the current (or last-known) user ID, or store the passkey marker under a single device-scoped key that is cleared on logout.

  • MINOR · src/components/Global/Modal/index.tsx:30 · [moonshotai/kimi-k3] useBackHandler in Modal can call onClose after visible flips false, leaking a late close event
    In src/components/Global/Modal/index.tsx the useBackHandler callback is registered with visible as the enable flag, but the callback body captures onClose and preventClose by closure. If the parent flips visible from true to false in the same commit that a back press fires (e.g. React 18 concurrent batching), the handler can run once more after the modal is logically closed and invoke onClose a second time. The test only covers the synchronous case. Fix: guard the callback with a ref to the latest visible value, or check visible inside the handler before calling onClose.

Inline anchors unavailable for 1 finding(s); the findings remain in this summary.

Checked clean

  • Confirmed the detached worktree HEAD, merge base, trusted author, base ref, and base SHA match the supplied review target.
  • Rechecked P1: leaving beta now treats production self-assignment refusal as best-effort and relies on the effective-channel read before reset.
  • Rechecked P2: the shared noise-wrapped PostHog error mirror is installed in both web and native Sentry initialization without adding a second native client.
  • Rechecked P3: UnsupportedWebViewScreen is explicitly recorded in the design-system raw-hex ratchet.
  • Rechecked P4 and P5: the shared setup login control maps every PasskeyErrorCode to translated catalog copy, with non-Passkey errors retaining the fallback.
  • Rechecked P6: receipt Pay navigation only opens HTTPS Peanut or configured preview origins externally; unsafe schemes, HTTP, and off-domain links are rejected in native and web paths.
  • Rechecked P7: locale-store mocks getBinaryInfo at the consumer boundary, and the exact-head unit check succeeds.
  • Reviewed Sentry initialization, lazy scoped captures, noise filtering, connectivity dedupe, and passkey-server error normalization.
  • Reviewed Capgo version floors, update staging, beta join/leave failure states, and official Capgo channel and minimum-version semantics.
  • Reviewed native back-handler ordering and cleanup, drawer/modal behavior, safe-area handling, WebView fallback, hosted-verification return handling, and native route mapping.
  • Reviewed setup entry and login routing, locale/splash gating, deferred locale restoration, receipt navigation, and native share/save behavior.
  • Exact-head CI succeeded for the aggregate gate, unit, typecheck, eslint, format, ds-lint, human-authors, bot-approval, analyze, and deployment preview checks; ds-shots remained in progress but was not blocking the successful aggregate gate.

Second opinion by moonshotai/kimi-k3: 3 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.

Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.

Exact head: 6a406a049a43 · Context: repo, provider_docs · Took 14m

Comment thread src/components/TransactionDetails/ReceiptActions.tsx
Comment thread src/components/Setup/setup-entry.ts
@innolope-dev
innolope-dev merged commit e8d5ace into dev Sep 2, 2026
23 of 25 checks passed
@chip-peanut-bot

Copy link
Copy Markdown
Contributor

This pull request was already closed when the review finished, so these findings are follow-up work rather than a gate.

Chip review — no blocking findings — this is not an approval

Clean at the pinned head. All nine prior findings are fixed or refuted by the current code and regression coverage; no new actionable correctness, security, adversarial, or slop finding survived verification.

Checked clean

  • Confirmed the detached worktree head, PR author, base ref/SHA, and merge base match the supplied review target.
  • Reviewed the complete 99-file diff and its commit sequence across Sentry, OTA/release workflows, native navigation, setup entry, i18n, and tests.
  • Rechecked P1-P9: production channel reassignment is best-effort, the native PostHog mirror is installed, the WebView color allowance is ratcheted, every passkey code maps to catalog copy, receipt links are restricted to HTTPS Peanut/build origins, binary-info mocks are at the consumer boundary, and stored credentials only choose the landing screen.
  • Adversarially checked OTA leave/retry marker states, effective-channel confirmation, release-version native floors, strict request-link parsing, hardware-back handler ordering, WebView bypass/safe-area behavior, and locale splash gating.
  • Exact-head CI is green for unit, typecheck, eslint, format, design-system lint/shots, human-authors, bot-approval, and the aggregate ci-success gate.
  • A focused local Jest rerun could not start because this detached worktree has no installed Jest binary; no test result was inferred from that failed command.

Second opinion: did not run — the model did not answer in time. 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: 6a406a049a43 · Context: repo · Took 25m (queued 4h32m)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant