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

[gmaclennan]

Implements the Sentry Phase 11 workblock (tracking #74) in one branch / one PR. Spec: docs/sentry-integration-plan.md §11.1–§11.8, §9b.1, §9b.5, Phase 5.

Part of #74 (tracking umbrella — remaining children #78–#83 stay open)
Closes #75
Closes #76
Closes #77

---

#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 kept for one minor release (JS get/setCaptureApplicationData, native setCaptureApplicationData, --captureApplicationData argv, plugin captureApplicationDataDefault with a warn-don't-error path).
• One-shot stored-key migration (old → new, runs once 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.
• 24h auto-off (§11.5): at process start the debug reader flips debug=false if switched on >24h ago and queues a comapeo.debug.auto_disabled breadcrumb; re-enable refreshes the window. Both platforms.
• Device classification (§11.2.b): new 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 now derived from debug (1.0/0).

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

• New backend/lib/metrics.js + src/sentry-metrics.ts: wrappers around Sentry.metrics.* injecting platform on every metric + units, attaching device_class/os_major only on the .by_device mirrors, no-op when Sentry is off.
• Inventory recorded: RPC durations + error counts (client + server), boot-phase timings + outcome, sync-session metrics, a 60s backend memory + event-loop-delay sampler, state transitions, storage size bucket. (Sync-session / IPC-error / telemetry-forwarding / OS-low-memory helpers are present in the metrics module; see deferral note below for the call sites not yet wired.)
• 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 (§11.3). src/ComapeoCoreModule.ts (onRequestHook) + backend/lib/sentry.js (rpcHook).
• tracesSampleRate driven by debug; backend consoleIntegration moved behind debug.
• beforeSendMetric forbidden-tag filter on the JS + Node metrics paths (shared regex list). Native SDKs: see deferral note.

#77 — PII scrubbers (§9b.1, §9b.5, Phase 5)

• One shared regex list (rootKey markers, 22-char base64, lat/lng markers) used by both RN (src/sentry-scrub.ts) and Node (backend/before-send.js), each pointing at the other.
• RN: real scrubber registered as beforeSend before the host's chain; URL-scrubbing beforeBreadcrumb.
• Node: backend/before-send.js registered as a Sentry.addEventProcessor in lib/sentry.js's init.
• Scrubs every text field: message, exception text, extra, contexts, breadcrumb message + data, span description + data (§9b.1).
• HTTP breadcrumb URLs reduced to host-only (§9b.5).

Scrubber false-positive trade-off

The substring scrubber over-redacts by design — leaking a real project secret costs far more than a stray [redacted]:

• 22-char base64 matches CoMapeo rootKey / hashed-project-id shapes but also any unrelated 22-char URL-safe base64 token (some JWT segments, git blob fragments, nonces).
  • aGVsbG8td29ybGQtMTIzNA → [redacted] (real rootkey shape)
  • bm90LWEtcmVhbC1rZXktMQ → [redacted] (harmless, false positive)
• lat/lng markers redact the trailing number: latitude: -12.34 → latitude: [redacted]; even longitude: unknown-style prose loses its value.
• HTTP URL host-only: https://cloud.comapeo.app/projects/abc?token=x → https://cloud.comapeo.app — path/query detail lost, "all requests to host X failing" diagnosability kept.

The same patterns gate the metrics beforeSendMetric filter (forbidden tag names + base64-22 / lat-lng tag values).

---

Checks run locally

• npm run lint — pass (0 errors, 0 warnings)
• npx tsc --noEmit (src) — pass
• npm test (jest, src) — pass (18/18)
• npm --prefix backend ci --ignore-scripts then node --test 'lib/**/*.test.mjs' — pass (47/47)
• backend rolldown build — pass; verified loader.mjs keeps @sentry/node-core behind the gated dynamic import.

Could NOT run locally (rely on CI)

• Kotlin unit tests (ComapeoPrefsTest, SentryConfigTest, new DeviceTagsTest) — no Android SDK / Gradle here. Cover the stored-key migration, 24h auto-off boundaries, and device-classification boundaries (exactly 3 GB RAM, exactly 4 cores). Need the CI emulator/Gradle to execute.
• Swift unit tests (ComapeoPrefsTests, SentryConfigTests, new DeviceTagsTests) — no Xcode here. Same coverage. Need CI/Xcode to execute.
• backend npm run types is red on main already (pre-existing polywasm declaration gap, unrelated); the new production files type-clean.

Deferred (called out explicitly)

• beforeSendMetric on the native (Android/iOS) SDKs is not yet wired — only the JS + Node metric paths run the forbidden-tag filter today. The native exit metric is the only native metric currently emitted and is already low-cardinality by construction; the native hook is a small follow-up.
• Sync-session / IPC-error / telemetry-forwarding / OS-low-memory metric call sites are not all wired yet (the metrics.js helpers exist; sync-session lifecycle plumbing is a Phase 5 item). The always-on RPC, boot, memory, state, and storage metrics are wired end-to-end.

🤖 Generated with Claude Code

[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).