feat(sentry): Phase 11 toggle rework, metrics layer + PII scrubbers

[gmaclennan]

Implements the Sentry Phase 11 workblock (tracking #74) in one branch / one PR, plus the user.id half of #78 and the review fixes from the pre-merge review. Spec: docs/sentry-integration-plan.md §11.1–§11.8, §9b.1, §9b.2, §9b.5.

Part of #74 (tracking umbrella — remaining children stay open: #77, #78, #80–#83, #190, #191)
Closes #75
Closes #76
Closes #161

---

#75 — Toggle rework (§11.1, §11.5, §11.6, §11.7, §11.2.b)

• Rename captureApplicationData → applicationUsageData everywhere: JS get/set, native bridge methods, the on-device stored key, the Expo plugin field, and the Node CLI flags. Deprecated aliases forward for one minor release; a one-shot stored-key migration runs on first prefs open on both platforms.
• New debug toggle: get/setDebugEnabled (JS), setDebugEnabled (native), sentry.debug + sentry.debugEnabledAtMs slots, --debug argv, debugDefault plugin field. 72h auto-off (§11.5 specced 24h; widened during review): at process start the debug reader flips debug=false if switched on >72h ago and queues a comapeo.debug.auto_disabled breadcrumb; re-enable refreshes the window.
• Device classification (§11.2.b): DeviceTags.{kt,swift} bucket the device low/mid/high by RAM + cores and compute <platform>.<major>. Passed to JS via sentryConfig.deviceTags and to Node via --deviceClass / --osMajor / --platformTag.
• tracesSampleRate is now owned by native and mirrored by the other layers: 1.0 while the debug window is on, otherwise the plugin-configured rate (0 when unset — the expected production config). This resolves the four-init-site divergence documented in Sentry: captureApplicationData → tracesSampleRate gate diverges across the four init sites #161; the policy is documented in docs/sentry-integration.md §8/§9.

#76 — Metrics layer (§11.2, §11.3, §11.6, §11.8)

• New backend/lib/metrics.js + src/sentry-metrics.ts: thin wrappers around Sentry.metrics.* that inject platform + units, no-op when Sentry is off, and run a forbidden-tag filter on every emission.
• One metric per measurement (the planned .by_device mirrors were collapsed — the new Sentry metrics product bills by volume, not cardinality, so device tags ride the primary metrics and group-by does the slicing). The RPC method attribute is gated on applicationUsageData for privacy, not cardinality.
• Wired end-to-end: RPC durations (client + server), boot-phase timings + outcome, 60s heap + event-loop-delay sampler (per-window max), state transitions, storage-size bucket, and the pre-existing comapeo.app.exit. Sync-session / shutdown / transport-health emitters ship unwired — call sites tracked in Sentry: sync-session transaction + bg/fg breadcrumbs (usage tier) #80 and Sentry: wire the remaining diagnostic metric emitters (shutdown phases, ipc.errors, telemetry.forwarding_failures) #190, and their rows in the §9.2 tier table are marked accordingly.
• RPC hooks split on both sides: always record the metric; create a span only under debug, recording the metric while the span is active so it links to the trace. Hooks never capture issues — RPC rejections are the caller's decision. Backend consoleIntegration moved behind debug.

PII scrubbers (§9b.1, §9b.5 — the bulk of #77)

• Mirrored scrubbers in src/sentry-scrub.ts (RN beforeSend, ahead of the host's chain) and backend/before-send.js (Node event processor), kept in sync by shared cases in test-support/scrubber-cases.js. Walks message, exception text, extra, contexts, breadcrumb message + data, span data, and structured logs.
• Redacts rootKey markers and coordinate markers — lat / lng / lon / latitude / longitude, in key-value, JSON-quoted, and object-key forms.
• HTTP breadcrumb URLs reduced to host-only (§9b.5).
• The broad 22-char base64 rule is deliberately disabled (it matched trace ids, exception type names, and error_class tags); Sentry: narrower scrub rule for bare rootkey-shaped tokens (base64-22) #77 is re-scoped to the narrower design, so it stays open.

user.id — root user ID + monthly rotation (§9b.2 / §11.4, the user.id half of #78)

• Identity anchors on a short root user ID (XXXX-XXXX-XXXX, base32 without I/L/O/U, 60 bits, secure RNG) generated lazily and stored in on-device settings so uninstall resets identity. Short and unambiguous so a user can hand-copy it from a screen.
• Sentry user.id is always a derived hash (sha256("<root>|<salt>"), 16 hex chars): salt is the UTC YYYY-MM month by default (rotates monthly, cross-month unlinkable) or "permanent" under applicationUsageData (stable, cohort analysis works). The raw root ID is never sent.
• Same value on all three SDKs per launch: native derives once, applies to its own scope, exposes sentryConfig.userId to JS (Sentry.setUser, locked), and passes --sentryUserId to the backend's initialScope.
• getRootUserId() exposed from the /sentry sub-export for a debug/about screen; support can recompute historical monthly IDs from a shared root ID. Kotlin/Swift derivations share pinned test vectors.
• The §9b.9 anchor resets remain in Sentry: reset exit-telemetry anchors on toggle cycle (§9b.9) #78 (re-scoped).

Review fixes

• Scrubbers: added the lon field (@comapeo/schema's actual field name) and quote handling for JSON-serialized coordinates.
• Preference setters update the in-memory live view only after the native write resolves, so a rejected write can't leave the getters reporting an opt-out that never persisted.
• Docs reconciled: 72h window everywhere, the actual tracesSampleRate policy in the three places that described the old gate, §8 rewritten to what the shipped scrubbers do, unwired metric rows flagged in the tier table.

Checks run locally

• npm run lint, npm run build — pass
• npm test (jest, src) — 69/69
• npm run backend:test — 73/73
• swift test (ios/) — 118/118
• ./scripts/run-instrumented-tests.sh --unit-only (Android JVM) — pass (incl. new SentryUserIdTest, prefs root-ID tests)

Deferred (tracked)

• Narrow bare-token (base64-22) scrub rule — Sentry: narrower scrub rule for bare rootkey-shaped tokens (base64-22) #77
• Toggle-cycle exit-anchor resets — Sentry: reset exit-telemetry anchors on toggle cycle (§9b.9) #78
• Sync-session transaction + syncSession metric wiring, bg/fg breadcrumbs — Sentry: sync-session transaction + bg/fg breadcrumbs (usage tier) #80
• Remaining diagnostic emitters (shutdown phases, ipc.errors, telemetry.forwarding_failures) — Sentry: wire the remaining diagnostic metric emitters (shutdown phases, ipc.errors, telemetry.forwarding_failures) #190
• Forbidden-tag filter on native-SDK metric emissions — Sentry: forbidden-tag filter for native-SDK metric emissions #191

[gmaclennan]

Phase 11 review fixes — a119f7e

Applied the confirmed findings. Scrubber changes are kept mirrored between src/sentry-scrub.ts (RN) and backend/before-send.js (Node).

Fixed

• Numeric lat/lng as object fields bypass the scrubber (major) — scrubValue is now key-aware (SENSITIVE_KEY_PATTERN): {latitude, longitude, lat, lng} fields are redacted regardless of value type, at every nesting level. Both files.
• 22-char-exact base64 boundary misses longer keys/ids (major) — quantifier widened to {22,} in SCRUB_PATTERNS and FORBIDDEN_METRIC_VALUE_PATTERNS in both files (rootKey@22, keypair public keys@43, z-base-32 ids@~52). Comments updated; floor kept at 22 (not 21).
• RN scrubEvent left breadcrumb path+query intact (major) — RN breadcrumb loop now calls scrubBreadcrumb (host-only reduction), symmetric with Node; RN scrubBreadcrumb also runs data through scrubValue. Docstring updated for §9b.5.
• scrubEvent never walked event.request (minor) — added a request branch (url→host, query_string/headers/cookies/data scrubbed) to both files; docstrings updated.
• comapeo.rpc.client.send_ms never emitted (major) — now emitted via rpcClientSendMetric on the always-on path and the debug-span path in src/ComapeoCoreModule.ts.
• debug 24h auto-off breadcrumb drained before queued (major, both platforms) — Android ComapeoCoreService.onCreate now reads debug/usage prefs before SentryFgsBridge.init; iOS didFinishLaunching calls readDebugEnabled() before initFromConfig/consume(). (Native — not compiled here; see below.)
• Hollow "drops forbidden tag NAMES" test (major + minor dupes) — replaced with a real integration test driving a forbidden name/value through a new metrics.__testInternals seam; fails if the name-filter branch is removed.
• RN metrics layer had no tests (major) — added src/__tests__/sentry-metrics.test.js (platform injection, by_device split, real isForbiddenMetric, off-switch, usage gating, rpcStatusFor).
• RN rpcHook debug-on/off coverage missing (major) — added src/__tests__/rpc-client-hook.test.js (always-record-metric, debug-gated span, error path).
• Added src/__tests__/sentry-scrub.test.js plus backend regressions (lat/lng-object, base64 43/52, event.request).

Documented, not code-changed

• Main-process auto-off breadcrumb is undeliverable on Android (minor) — took the offered doc-only fallback: documented the cross-JVM drop in DebugAutoOff rather than adding JS-bridge plumbing (out of proportion for a minor timeline-marker loss). Flagging for a maintainer call on whether to wire the JS drain later.

Skipped

• "PR deletes no tests" / rebase premise (nit) — no code change; addressed by fast-forwarding the branch to origin before working. git diff origin/main...HEAD is the correct three-dot review diff.

Checks

• npm run lint — pass
• npx tsc --noEmit (src) — pass
• npm test (jest) — 4 suites / 35 tests pass. (Note: a stale cross-worktree symlink for babel-preset-expo in node_modules had to be repaired locally; not a repo change.)
• backend node --test lib/**/*.test.mjs — 50 pass / 0 fail
• backend npm run types fails only on pre-existing implicit-any in *.test.mjs fakes (identical on baseline; the new tests follow the existing fake-SDK pattern).

Reviewer please double-check (native, not compilable here)

• android/.../ComapeoCoreService.kt: pref-read order moved above SentryFgsBridge.init.
• ios/AppLifecycleDelegate.swift: readDebugEnabled() before initFromConfig.

[gmaclennan]

Code review (xhigh, workflow-backed)

I merged main into this branch and ran a multi-agent review over the Phase 11 diff plus the merge resolution. Two conflicts were resolved (ComapeoCoreService.kt — combined main's deferred-backend-init + process-name detection with Phase 11's applicationUsageData/debug/deviceTags; and a trivial both-added interface conflict in ComapeoCoreModule.ts). All suites pass (jest 39/39, backend 51/51, tsc + eslint clean).

I've already pushed two fixes that needed no decision:

• test(sentry): the merge made notification-permissions.test.js load the real ComapeoCoreModule, which now pulls the metrics layer and trips an import cycle — stubbed ./sentry-metrics like the test's other transitive deps.
• perf(sentry): gated sampleStorageSize's recursive disk walk on metrics.isEnabled() (finding 6 below).

The rest below need your call or touch the PII scrubber, so I'm flagging rather than editing.

---

1. RPC error capture is now gated behind debug (off by default) — likely a regression

backend/lib/sentry.js:371 and src/ComapeoCoreModule.ts:344

On main, every RPC-handler rejection was sent to Sentry as an issue whenever Sentry was initialised (i.e. at the diagnostics tier, on by default). After this change, captureException only lives inside the debug-gated span path (sentry.js:427, ComapeoCoreModule.ts:403). The default (debug: false, and debug auto-expires after 24h) takes the early !debug branch, which records duration/error counters only and returns — so a throwing handler (e.g. a regression in observation.create) produces no Sentry issue, no stack trace, no grouping. On-call loses the error visibility the diagnostic tier previously had; only the comapeo.rpc.server.errors count moves.

If decoupling issue-capture from per-RPC tracing was intentional noise reduction, worth saying so explicitly. If not, captureException should run in the always-on path (gated on Sentry being up), independent of debug.

2. PII scrubber's broad token regex over-matches legitimate Sentry IDs

The base64-ish rule …[A-Za-z0-9_-]{22,}… (src/sentry-scrub.ts:48, mirrored in backend/before-send.js) is applied in three places where the matched value isn't PII:

• contexts.trace.trace_id gets redacted (sentry-scrub.ts:141). A trace_id is 32 hex chars, so it matches and becomes "[redacted]". This breaks crash↔trace correlation — the exact feature this PR adds — and since Sentry/Relay validates trace_id as 32-hex, an invalid value can get the trace context stripped or the event dropped.
• Long exception type names get redacted (sentry-scrub.ts:133 runs ex.type through scrubString). FailedPreconditionError, BlockNotAvailableError, etc. are single tokens ≥22 chars, so distinct error classes all collapse to one [redacted] issue title — losing grouping/alerting for the domain errors most worth tracking.
• Long error_class metric tags get dropped (errorClassFor → metrics.rpcServerError, sentry.js:451/377). The same pattern in FORBIDDEN_METRIC_VALUE_PATTERNS drops any metric whose tag value is ≥22 word chars, so comapeo.rpc.server.errors silently records nothing for longer-named errors — the error-rate dashboard under-counts precisely those.

These three share one root cause: the token rule is meant for secrets/opaque keys but also matches Sentry's own identifiers and PascalCase class names. Suggested direction (your call on the exact shape, since it's the PII boundary): exclude the trace context's id fields (trace_id/span_id/parent_span_id) and the exception type/error_class fields from the token rule, rather than running them through free-text scrubbing. Whatever you pick needs to land in both the RN and backend copies. A test asserting a real 32-hex trace_id survives would lock this in (today only contexts.geo is covered).

3. Unguarded promise chain in the backend !debug path can exit the process

backend/lib/sentry.js:373 — Promise.resolve(next(request)).then(onFulfilled, onRejected) has no trailing .catch. If a metrics.* call inside the rejection handler throws, the derived promise rejects unhandled, which your process-level handler routes to handleFatal('runtime') → process.exit(1) — taking down in-flight RPCs and sync. The RN sibling already guards the equivalent chain with .catch(noop) (ComapeoCoreModule.ts:354); the backend should match. (Folds naturally into whatever you do for #1, since it's the same branch.)

4. Hand-mirrored scrubber/metrics modules will drift

src/sentry-scrub.ts ↔ backend/before-send.js are byte-for-byte copies of ~200 lines (patterns, forbidden-tag sets, scrub*/isForbiddenMetric); src/sentry-metrics.ts ↔ backend/lib/metrics.js duplicate the metric primitives + status classifier (rpcStatusFor/statusFor). The stated reason (different module systems) is thin — both are ESM graphs (Metro and rollup both follow import), so a shared core .js (+ .d.ts for the TS side) could serve both. The concrete risk: a new secret/coordinate shape added to one copy and forgotten in the other is a one-sided PII leak that each file's own tests still pass. Not a blocker, but the duplication is load-bearing for privacy, which raises the stakes.

Lower priority

• src/sentry-metrics.ts:69 (+ metrics.js): distribution/count/gauge are three near-identical wrappers repeating the sentryUp → withPlatform → isForbiddenMetric → emit gate; a private emit(kind, …) collapses each trio and keeps the §11.8 safety gate in one place across the six call sites.
• android/.../ComapeoPrefs.kt:192: the 24h debug auto-off breadcrumb is handed off via a process-local static (DebugAutoOff) that a later SDK-init drains. On Android the main process and the :ComapeoCore backend process are separate OS processes with separate memory, so a static set in one isn't visible in the other — on the common cold-start ordering the auto_disabled breadcrumb is lost. Already documented in-code as a deferred limitation; noting it so it isn't forgotten.

---

Findings verified by independent adversarial agents; 5 candidates were refuted and dropped (e.g. a Hermes-lookbehind concern — Hermes does support lookbehind). The import cycle in note (test fix above) only bites under eager module evaluation like jest's; Metro's inlineRequires likely defers it in production, but worth confirming the main entry (index.ts → ComapeoCoreModule first) loads cleanly without importing /sentry first.

[gmaclennan]

Update — findings addressed (pushed):

• Convert date timestamps to Date objects over RPC #1 (RPC error capture gated behind debug): confirmed unintended. captureException is now called on the always-on path in both the backend rpcHook and the RN client hook, gated only on Sentry being initialised, independent of debug. Added the missing .catch on the backend chain (Android Testing Infrastructure & Bug Fixes #3) so a throw from a metrics call can't escalate to process.exit. New tests lock in capture-on-debug-off (and no-capture when Sentry is down). No double-capture: debug-on still captures once in the span path.
• Integrate into comapeo-mobile #2 (PII scrubber over-matching): the broad {22,} base64 token rule is disabled in both the RN scrubber and the backend mirror, per the decision to discuss a narrower rule with the team first. This fixes all three symptoms at the root — trace_id, exception type names, and error_class metric tags are no longer redacted. The targeted root_key= and lat/lng rules stay. Bare tokens are unscrubbed until a replacement lands; documented in-code and in the tests so it isn't silently forgotten.
• Add iOS support & test infrastructure #6 (storage walk): gated on metrics.isEnabled() (pushed earlier).

Still open for the team: #4 (duplicated scrubber/metrics modules), #10 (the metric-wrapper dedup), #9 (Android cross-process breadcrumb, already documented as deferred), and the eventual narrower token-scrub rule.

[gmaclennan]

Follow-up on the earlier review thread:

RPC error capture — removed from the hooks entirely. Tracing of the actual flow confirmed the RN client hook sees every RPC failure (handler errors come back deserialized with the backend stack preserved, plus client-only TimeoutError/ChannelClosedError), while the backend rpcHook only ever sees a subset (handler errors) — so capturing in both double-counts. More importantly, the hook is the wrong place to decide what's report-worthy: an RPC rejection is often expected control flow (e.g. NotFound) that shouldn't auto-create a Sentry issue. So both hooks are now metrics + tracing only; captureException is gone from both. Error reporting is left to the calling application at the call site. Genuine backend fatals are still covered by the global uncaughtException/unhandledRejection handlers (captureFatal).

Storage-size metric — the recursive-walk concern is filed as #178 (replace the JS walk with Android StorageStatsManager.queryStatsForUid, with a full plan + pros/cons). The interim metrics.isEnabled() gate stays in this PR.

Still open for the team: the duplicated scrubber/metrics modules, the metric-wrapper dedup, and the eventual narrower token-scrub rule (broad base64-22 rule remains disabled).

[gmaclennan]

Tighten this comment

[gmaclennan]

Is this the right abstraction at this point, now we have added more values to it? Take a step back and think through other abstractions over SharedPreferences that might involve less boilerplate and reduce code needed for each additional pref that is added.

[gmaclennan]

Tighten this comment and make it clearer what the consequences of the current gap are.

[gmaclennan]

Change debug auto-off to 72h (on both android and ios)

[gmaclennan]

Same questions here as in the kotlin side

[gmaclennan]

similar to kotlin, take a step back and thing whether this remains the best abstraction for these preferences - as we add more values to the prefs it starts to become a bit unweildy.

[gmaclennan]

This can be removed from the readme I think. Does this API still exist?