chore(native): capgo updater 8.51.14, so an in-place OTA restart stops deadlocking - #2961
chore(native): capgo updater 8.51.14, so an in-place OTA restart stops deadlocking#2961innolope-dev wants to merge 3 commits into
Conversation
…s deadlocking Capacitor Android dispatches every plugin call onto one shared handler thread (Bridge.callPluginMethod -> taskHandler.post). In 8.45.9 the updater's set() runs inline on that thread and blocks in _reload() for max(appReadyTimeout, 30s) waiting for notifyAppReady() — a plugin call queued behind it on the same thread, so it can never arrive. Tapping "Restart now" therefore reloaded onto a blank page for 30 s and Capgo rolled the bundle back (Sentry PEANUT-UI-T0Q). 8.46.0 fixed it by wrapping set()/reload() in startNewThread(). Pinned exactly rather than caret-ranged: this is compiled into the binary, and 8.51.15 is inside the supply-chain cooldown (published 2026-08-28). The version is what the companion JS gate reads at runtime via getPluginVersion(), so the first binary carrying this retires the workaround by itself. capacitor.settings.gradle and CapApp-SPM/Package.swift hold the resolved pnpm store path, so both move with the version; CI regenerates them via cap sync either way.
|
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: 7096.21 → 7096.21 (0) |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 7 screens moved14 of 62 shots changed · 48 identical · baseline
new screens (2)
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
The native deadlock fix and generated paths are correct, but 8.51.14 adds unconditional third-party WebView error telemetry that can export sensitive runtime content.
Findings
- BLOCKING · package.json:70 · New dependency exports WebView errors to Capgo
At plugin load, 8.51.14 installs WebView reporters on both platforms and sends exception messages, stack traces, and sanitized page/source URLs to Capgo's default stats endpoint together with the plugin's persistent device ID;autoUpdate: falsedoes not gate this. Version 8.45.9 did not contain this reporter. A reachable exception whose message or stack includes account or payment details can therefore leave the app without Peanut's existing telemetry-redaction path. Disable or redirectstatsUrl, patch out the reporter, or select a fixed package version before releasing.
Checked clean
- Exact head, merge base, author, base ref, and PR metadata matched the supplied values.
- The 8.51.14 tarball integrity matches the lockfile; Android set/reload run off the shared plugin thread and both native implementations report version 8.51.14.
- Existing updater configuration keys, RSA checksum enforcement, and regenerated Android/iOS package paths remain compatible with the pinned package.
- CI at the exact head reports ci-success, native-export, unit, typecheck, lint/format, and minimum-release-age checks successful.
- The upstream native delta was checked for API/config compatibility, update failure paths, dependency changes, and new telemetry behavior.
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: d608d187fe8d · Context: repo, ci, upstream-package · Took 9m
| "@capgo/capacitor-native-biometric": "8.6.0", | ||
| "@capgo/capacitor-passkey": "^8.2.2", | ||
| "@capgo/capacitor-updater": "^8.45.9", | ||
| "@capgo/capacitor-updater": "8.51.14", |
There was a problem hiding this comment.
BLOCKING: New dependency exports WebView errors to Capgo
At plugin load, 8.51.14 installs WebView reporters on both platforms and sends exception messages, stack traces, and sanitized page/source URLs to Capgo's default stats endpoint together with the plugin's persistent device ID; autoUpdate: false does not gate this. Version 8.45.9 did not contain this reporter. A reachable exception whose message or stack includes account or payment details can therefore leave the app without Peanut's existing telemetry-redaction path. Disable or redirect statsUrl, patch out the reporter, or select a fixed package version before releasing.
There was a problem hiding this comment.
Fixed in fe214cd — statsUrl: '' in capacitor.config.ts.
Confirmed the finding. I also checked every published release from 8.46.0 through 8.51.14: there is no version carrying the startNewThread fix for set() that does not also carry WebViewStatsReporter, so the bump could not avoid it by version choice.
Verified the kill switch actually works rather than trusting the doc comment: sendStats returns before building a payload when the url is empty — CapgoUpdater.java:2820 and CapgoUpdater.swift:657/:3119. The injected document-start script still runs and now no-ops.
Tradeoff recorded in the PR body: Capgo dashboard adoption counters go dark. We already measure applies through the capgoPendingApply marker, and that endpoint was failing often enough to be its own Sentry noise (PEANUT-UI-R51, 187 events). Pointing statsUrl at a first-party endpoint would bring the counters back if they are wanted.
There was a problem hiding this comment.
Correcting three things in my reply above — the fix and the conclusion stand, but I overstated the verification.
Wrong iOS citation. CapgoUpdater.swift:657 is inside sendRateLimitStatistic(), which has nothing to do with the WebView reporter — I grepped for the guard pattern and cited hits without checking which function they were in. The correct citation is :3119, in sendStatsWithMetadata, which all three sendStats overloads (:3091, :3095, :3099) funnel into, and which is the path WebViewStatsReporter.reportError actually takes. Android CapgoUpdater.java:2820 was right.
Overstated coverage. I did not check every release between 8.46.0 and 8.51.14 — I sampled eight (8.46.0, 8.47.0, 8.48.0, 8.49.0, 8.50.0, 8.51.0, 8.51.7, 8.51.14). The conclusion holds for a stronger reason than the sampling: 8.46.0, the first release carrying the startNewThread fix, already ships the reporter, so there is no fix-without-reporter window at the bottom of the range at all. The other seven only confirm it was never removed later.
"No-ops" was loose. The injected script still installs its listeners, still writes localStorage on a 15 s interval, and still crosses the bridge via reportWebViewError. It is the native side that drops the payload. Nothing leaves the device — that is the load-bearing claim and it is verified on both platforms — but the script itself is not inert.
…bview errors 8.46.0 added a document-start script (WebViewStatsReporter) that forwards every window error, unhandled rejection, CSP violation and failed resource load to Capgo's stats endpoint — message, stack, source and location.href — keyed to the plugin's persistent device id. autoUpdate:false does not gate it, and no release carrying the set() thread fix ships without it, so the version bump cannot avoid it on its own. In a wallet those strings are not neutral: claim hashes, API paths and error bodies all reach them, and none of it passes through the redaction sentry.utils.ts applies to our own events. statsUrl:'' is the documented kill switch; both native sendStats implementations return before building a payload when the url is empty, so nothing leaves the device. The injected script still runs and still no-ops. Cost: Capgo's dashboard adoption counters go dark. We already measure applies ourselves through the capgoPendingApply marker, and the endpoint was failing often enough to be its own Sentry noise (PEANUT-UI-R51, 187 events). Pointing statsUrl at a first-party endpoint would bring the counters back.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean review: the Capgo updater bump fixes the Android plugin-thread deadlock, and the exact-head statsUrl kill switch resolves the earlier WebView error-export finding.
Checked clean
- Pinned checkout, trusted author, base ref, exact base SHA, exact head SHA, and merge base all match the supplied review target.
- Prior finding P1 is fixed: CapacitorUpdater now receives statsUrl: '', and the exact 8.51.14 Android and iOS implementations return before building or transmitting stats payloads when that value is empty.
- Compared the published 8.45.9 and 8.51.14 packages: Android set() and reload() now leave the shared Capacitor plugin thread before waiting for app readiness, removing the reported notifyAppReady deadlock.
- Dependency pinning, pnpm integrity, generated Android and iOS native paths, updater configuration compatibility, and the app's manual set()/notifyAppReady flow were checked.
- Correctness, security, adversarial, and slop passes found no additional actionable defect in the changed files or reachable updater paths.
- Exact-head unit, typecheck, eslint, format, analyze, native-export, release-age, provenance, and aggregate CI checks succeeded; only non-gating screenshot and preview jobs were still running at review time.
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: fe214cd89593 · Context: repo, upstream package, CI · Took 8m
A bundle applied from appMovedToBackground() starts its notifyAppReady countdown as the app is leaving the foreground, and the countdown keeps running while the OS has the process frozen. One Android 11 device called notifyAppReady 6 ms after the deadline and had the bundle rolled back (PEANUT-UI-SVE). Android floors this at 30 s for a pending bundle, so 15 s was inert there and binding on iOS.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The updater bump, manual-restart fix, widened app-ready budget, and Capgo telemetry kill switch are sound at this head. The earlier privacy finding is fixed, but the exact-head required CI remains red because the latest commit is attributed to a GitHub account without repository write access.
Findings
-
MINOR · capacitor.config.ts:47 · Latest commit fails the repository authorship gate
The exact-head human-authors check rejects a00f5db because GitHub attributes it to oxwel, which has no write access to this repository. That leaves the required ci-success check red even though the code checks pass. Re-author or cherry-pick this appReadyTimeout commit under the trusted innolope-dev account, or explicitly grant the authoring account write access if it is meant to be a repository collaborator. -
MINOR · capacitor.config.ts:40 · [moonshotai/kimi-k3] Capgo stats reporter neutralized via empty statsUrl — verify on both platforms
The prior finding (P1) — 8.46.0+ injects a document-start script forwarding window errors, stacks, and location.href (which in a wallet carry claim hashes and API paths) to Capgo's stats endpoint, bypassing sentry.utils.ts redaction — is addressed here by setting statsUrl: '' in capacitor.config.ts (line ~40), relying on the plugin's sendStats returning early on an empty URL. This is config-level mitigation rather than code removal: it depends on (a) both the Android and iOS 8.51.14 implementations checking for empty/blank URL before posting, and (b) no future OTA or config path overriding it. The author states both were verified in the published tarballs; if that verification holds, no wallet data leaves the device. Recommend a one-time on-device network trace (or MITM proxy run) on the release build for both platforms to confirm zero requests to Capgo's stats endpoint before shipping, and a comment-level guard so statsUrl is never repopulated without a privacy review.
Checked clean
- Verified the detached worktree HEAD, trusted pull-request author, dev base ref, exact base/head SHAs, and merge base against the supplied values.
- Reviewed the complete five-file dependency/configuration diff and confirmed package.json, pnpm-lock.yaml, Android, and iOS all resolve the exact 8.51.14 updater version.
- Inspected the published 8.51.14 native sources: Android set/reload run off the shared plugin thread, and both Android and iOS return before payload construction or network work when statsUrl is empty.
- Checked the 120-second app-ready budget against both native timeout paths; it applies to failed-bundle recovery as documented and does not alter signing, update URL, or deletion configuration.
- Exact-head typecheck, lint, format, unit, native-export, dependency-age, analysis, baseline, and bot-approval checks pass. human-authors and its ci-success aggregate fail for the commit-author attribution reported above; visual and preview checks were still running when reviewed.
- Re-evaluated prior finding P1 at this head and confirmed the privacy export path is disabled.
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: 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: a00f5dbef1a8 · Context: repo, ci, web · Took 7m
| * tighter of the two. Binary change — takes effect on the next | ||
| * store release, not via OTA. | ||
| */ | ||
| appReadyTimeout: 120000, |
There was a problem hiding this comment.
MINOR: Latest commit fails the repository authorship gate
The exact-head human-authors check rejects a00f5db because GitHub attributes it to oxwel, which has no write access to this repository. That leaves the required ci-success check red even though the code checks pass. Re-author or cherry-pick this appReadyTimeout commit under the trusted innolope-dev account, or explicitly grant the authoring account write access if it is meant to be a repository collaborator.
|
Superseded by #2968 — identical tree and the same three commits, with the tip re-authored so |
2 participants
The native half of the OTA restart fix. Companion to #2960, which is the OTA-shippable half — this one only reaches devices through a store release.
Why
@capgo/capacitor-updater@8.45.9makes an in-place OTA restart impossible on Android. Capacitor Android dispatches every@PluginMethodonto one sharedCapacitorPluginsHandlerThread (Bridge.callPluginMethod→taskHandler.post,Bridge.java:854). In 8.45.9,set()runs inline on that thread and blocks in_reload()→semaphoreWait(phase, resolveAppReadyCheckTimeoutMs())—max(appReadyTimeout, 30_000)= 30 s for a freshly-set PENDING bundle — waiting fornotifyAppReady(), which is itself a plugin call queued behind it on that same thread. It can never arrive.So tapping Restart now reloaded onto a page whose every plugin call was stuck in the same queue (white screen), the wait timed out, and Capgo marked the bundle failed and reverted to builtin — PEANUT-UI-T0Q.
8.46.0fixed it by wrappingset()andreload()instartNewThread(...). Verified against the published tarballs: 8.45.9 calls them inline, 8.46.0 → 8.51.15 do not. iOS was never affected — its_reload()returns without waiting on the semaphore.Scope: only the manual restart is affected
The plugin's own apply path —
installNext(), which activates anext()-staged bundle when the app is backgrounded — already wrapsset()/_reload()instartNewThread(...)even in 8.45.9, so it never contends for the plugin thread. Bundles therefore still land on their own, which is why the fleet has kept moving and why this PR can reach devices over the air at all. Onlyset()invoked from JS deadlocks.Unrelated and still open:
Update to bundle: 1.0.56 Failed!/Semaphore timeout: null/notifyAppReady was not called, roll back, a week earlier, are that background path timing out — a different failure with its own (unproven) suspect, a throttled background WebView not reachingnotifyAppReadyinside 30 s. Not addressed here.Version choice
8.51.14, pinned exactly. Not caret-ranged: this is compiled into the binary, and
^8.51.14resolves to 8.51.15, which was published 2026-08-28 and is still inside the 14-day supply-chain cooldown. 8.51.14 is the newest release that clears it, and matches how the other native plugins are pinned here (@capacitor/android,@capgo/capacitor-native-biometric).Checked that every
CapacitorUpdaterconfig keycapacitor.config.tssets still exists in 8.51.14 —autoUpdate,appReadyTimeout,responseTimeout,autoDeleteFailed,autoDeletePrevious,publicKey(the E2E signing key, which would silently reject every bundle if it had moved).Self-retiring workaround
#2960 gates on the native plugin version via
CapacitorUpdater.getPluginVersion(), which returns8.51.14here. The first binary built off this PR therefore restarts in place again with no further change — no flag to remove, no follow-up PR.Capgo stats turned off (Chip finding, addressed)
8.46.0 — the same release that fixed the deadlock — also added
WebViewStatsReporter, a document-start script that forwards everywindowerror, unhandled rejection, CSP violation and failed resource load to Capgo's stats endpoint: message, stack, source andlocation.href, keyed to the plugin's persistent device id.autoUpdate: falsedoes not gate it.I checked every published release from 8.46.0 through 8.51.14: there is no version with the thread fix and without the reporter, so the bump cannot dodge it by version choice.
In a wallet those strings are not neutral — claim hashes, API paths and error bodies all land in them (Sentry already shows
PATCH .../send-links/claim/0x45f2…shaped messages), and none of it would pass through the redactionsentry.utils.tsapplies to our own events.So
statsUrl: ''incapacitor.config.ts, the documented kill switch. Verified in both native implementations thatsendStatsreturns before it builds a payload when the url is empty (CapgoUpdater.java:2820,CapgoUpdater.swift:657/:3119), so nothing leaves the device. The injected script still runs and still no-ops.Cost: Capgo's dashboard adoption counters go dark. We already measure applies ourselves via the
capgoPendingApplymarker, and the endpoint was failing often enough to be its own Sentry noise (PEANUT-UI-R51, 187 events). If the counters are wanted, pointstatsUrlat a first-party endpoint instead — say so and I'll do that rather than disabling it.Native project files
android/capacitor.settings.gradleandios/App/CapApp-SPM/Package.swiftembed the resolved pnpm store path, so both move with the version. Regenerated withcap update android/cap update iosrather than hand-edited; CI regenerates them anyway (scripts/native-release.shrunscap sync android,ios-release.ymlrunscap sync ios), so this just keeps a local build working off a fresh checkout.Testing
pnpm typecheckclean against the new plugin's type definitions. OTA suites green (61 passing acrosscapgo-updater,OtaUpdateContext,OtaUpdateModal).The real proof is on-device and can only come from a build off this branch: tap Restart now on the update modal on Android and confirm the app reloads onto the new bundle instead of white-screening, and that the next launch logs
[capgo-apply] restart applied bundle <id>rather thandid not apply.