feat(messaging): configure push identity verification in channel setup - #74143
Conversation
…setup modals Adds an identity verification control (disabled / optional / required) to the Firebase and APNs setup modals, writing the push_identity_verification key the device registration endpoint already reads. Until now the mode could only be set by editing the integration config directly. Also lets the APNs signing key be uploaded as a .p8 file instead of only pasted, since that is the form Apple hands it out in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
21.25 MiB | 🟢 -38.4 KiB (-0.2%) |
posthog-app/src/lib/components/Shortcuts/utils/DebugCHQueriesImpl.js |
33.4 KiB | 🔺 +18.2 KiB (+119.1%) |
posthog-app/_parent/products/data_catalog/frontend/DataCatalogScene.js |
16.8 KiB | 🔺 +16.8 KiB (new) |
posthog-app/_parent/products/mcp_analytics/frontend/MCPAnalyticsScene.js |
156.6 KiB | 🔺 +16.0 KiB (+11.4%) |
posthog-app/src/scenes/inbox/InboxScene.js |
684.7 KiB | 🔺 +4.8 KiB (+0.7%) |
posthog-app/_parent/products/managed_migrations/frontend/ManagedMigration.js |
27.4 KiB | 🔺 +3.3 KiB (+13.6%) |
posthog-app/src/scenes/onboarding/Onboarding.js |
814.4 KiB | 🔺 +3.0 KiB (+0.4%) |
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/ActionEditorScene.js |
23.4 KiB | 🔺 +1.1 KiB (+5.0%) |
posthog-app/_parent/products/workflows/frontend/WorkflowsScene.js |
76.9 KiB | 🔺 +1.1 KiB (+1.4%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.24 MiB · 22 files | 🔺 +439 B (+0.0%) | ███░░░░░░░ 27.6% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.09 MiB · 3,015 files | 🔺 +6.0 KiB (+0.1%) | ████████░░ 83.3% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.6 KiB | ../node_modules/.pnpm/posthog-js@1.407.6/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.6/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.2 KiB | src/lib/api.ts |
| 94.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.18 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.18 MiB · 17 files | 🔺 +273 B (+0.0%) | ████░░░░░░ 38.2% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 717.2 KiB | dist/toolbar/toolbar-app-JFACJRNE.css |
| 545.8 KiB | dist/toolbar/chunk-chunk-ZSVWA4JG.js |
| 484.3 KiB | dist/toolbar/chunk-chunk-CQF6YMXB.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-OQGOYLCD.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-4JHCNGTW.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-OMSYGMWC.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-KDWBUS5S.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +2.22 MiB (+0.2%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1362.69 MiB · 🔺 +2.22 MiB (+0.2%)
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Creating a SQL insight with a variable and overriding it on a dashboard (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
✅ Backend coverage — all changed backend lines covered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (63 / 63)
All changed backend lines are covered ✅
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.5% |
8,593 / 21,741 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
██████████████░░░░░░ 69.7% |
31,432 / 45,123 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
cdp |
████████████████░░░░ 81.0% |
3,146 / 3,883 |
signals |
████████████████░░░░ 81.1% |
23,842 / 29,387 |
wizard |
█████████████████░░░ 84.8% |
935 / 1,102 |
data_modeling |
█████████████████░░░ 85.3% |
7,343 / 8,611 |
notebooks |
█████████████████░░░ 86.0% |
7,794 / 9,060 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
cohorts |
█████████████████░░░ 86.9% |
5,648 / 6,496 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 88.3% |
12,036 / 13,635 |
exports |
██████████████████░░ 88.4% |
6,950 / 7,862 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
engineering_analytics |
██████████████████░░ 89.4% |
6,441 / 7,202 |
visual_review |
██████████████████░░ 89.5% |
5,837 / 6,522 |
conversations |
██████████████████░░ 89.6% |
17,234 / 19,241 |
alerts |
██████████████████░░ 90.0% |
4,342 / 4,827 |
mcp_analytics |
██████████████████░░ 90.2% |
2,883 / 3,198 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
error_tracking |
██████████████████░░ 91.0% |
10,925 / 12,004 |
slack_app |
██████████████████░░ 91.1% |
9,554 / 10,492 |
stamphog |
██████████████████░░ 91.1% |
4,056 / 4,450 |
marketing_analytics |
██████████████████░░ 91.2% |
12,058 / 13,222 |
mcp_store |
██████████████████░░ 91.9% |
4,257 / 4,634 |
product_analytics |
███████████████████░ 92.5% |
5,849 / 6,321 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
early_access_features |
███████████████████░ 92.6% |
1,287 / 1,390 |
notifications |
███████████████████░ 92.6% |
1,017 / 1,098 |
ai_observability |
███████████████████░ 92.8% |
15,328 / 16,517 |
surveys |
███████████████████░ 93.1% |
5,771 / 6,197 |
web_analytics |
███████████████████░ 93.2% |
14,826 / 15,906 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
legal_documents |
███████████████████░ 93.8% |
1,628 / 1,736 |
workflows |
███████████████████░ 93.8% |
6,943 / 7,399 |
endpoints |
███████████████████░ 94.2% |
8,655 / 9,192 |
tracing |
███████████████████░ 94.5% |
2,670 / 2,826 |
skills |
███████████████████░ 94.6% |
3,158 / 3,337 |
review_hog |
███████████████████░ 94.6% |
6,912 / 7,303 |
messaging |
███████████████████░ 94.7% |
2,885 / 3,048 |
experiments |
███████████████████░ 95.4% |
25,415 / 26,637 |
logs |
███████████████████░ 95.5% |
10,435 / 10,928 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
revenue_analytics |
███████████████████░ 96.3% |
1,887 / 1,960 |
feature_flags |
███████████████████░ 96.4% |
17,371 / 18,023 |
replay_vision |
███████████████████░ 96.4% |
15,861 / 16,455 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
access_control |
███████████████████░ 96.9% |
870 / 898 |
customer_analytics |
███████████████████░ 97.1% |
9,798 / 10,086 |
warehouse_sources |
███████████████████░ 97.2% |
346,738 / 356,639 |
data_catalog |
████████████████████ 97.7% |
2,556 / 2,615 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
…ation The setup modals write push_identity_verification into the integration config, but the serializer only forwarded named provider fields, so the value was dropped before reaching the integration. Thread it through for both Firebase and APNs. Both integration_from_key helpers rebuild config from the credentials they are handed, so a credential rotation would also wipe the flag back to disabled and silently reopen device takeover. Carry the existing value forward unless the caller sets a new one, and reject unknown modes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
|
😎 Merged successfully - details. |
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status. |
|
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 3 · PR risk: 0/10 |
…etup Three issues raised in review, all reachable through the new setup UI: The setup forms defaulted the mode to disabled and always submitted it, so reconnecting an integration to rotate credentials sent an explicit disabled and bypassed the backend preservation this PR added. Seed the field from the integration's stored policy instead. Reading the existing policy and upserting it back are separate statements, so a concurrent write enabling verification could be overwritten by the stale read. Lock the row and hold it through the upsert. Enabling verification is a security policy change, so require project admin. A plain member could otherwise create a new APNs integration carrying another app's bundle_id under a different Apple team id, which creates rather than overwrites and so escaped the existing admin check, and set required on it. The push endpoint takes the strictest mode across integrations sharing a bundle_id, so that would reject the real app's registrations. Also cancel a stale .p8 read when a second file is picked, so the newer selection wins rather than whichever read finishes last. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
A value read back from config was carried forward without checking it, so a corrupted or hand-edited mode would survive every credential rotation. Drop it instead: the push endpoint already treats an unknown mode as disabled, so this changes no enforcement behavior while letting the bad value clear itself. Chose dropping over raising so a corrupted integration can still be reconnected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
select_for_update only locks a row that exists, so two concurrent first-time setups of the same integration could both read "no policy" and the later write would clobber a policy the earlier one had just set. Take an advisory transaction lock keyed on the integration's identity before the read, which covers the not-yet-created case. Keyed per integration rather than per team so it only serializes writers racing for the same one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
…tion The existing tests call integration_from_key directly, so nothing exercised the serializer that threads push_identity_verification through to it. That layer is exactly where the value used to be dropped, leaving the setup UI's toggle inert, so assert an API-level create actually persists the mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
Setting a non-disabled policy requires project admin, but the base test user is a plain member, so the new happy-path test hit that gate and got a 403. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
Every writer for an integration identity now participates in the advisory lock. Previously a request setting an explicit mode returned before acquiring it, so it could create the row between a preserving writer's read and write and have its policy dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
The overwrite check reads existing integration ids before the write, so a member racing the first setup of a push integration is still classified as a create. Submitting an explicit `disabled` could therefore land over a policy an admin had just written, with no admin check. Any explicit value now requires admin. Omitting the key stays open to members and is what connecting a channel without touching the policy does, so the setup modals only send it when it differs from what is stored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bbav6vE7Tm24KkdRhZmvNM
|
/trunk merge |
Problem
#72488 shipped opt-in identity verification for push device registration, but nothing in the UI can set it.
push_identity_verificationis read per integration by the registration endpoint, and the only way to move it offdisabledwas editing the integration config directly. So the feature existed but no customer could turn it on.Two backend problems sat behind that, both of which had to be fixed for a UI toggle to actually work:
IntegrationSerializer._build_integrationpicks named provider fields out ofconfigand passes them as keyword arguments, so anything else the client sent was dropped on the floor.integration_from_keyhelpers rebuildconfigfrom the credentials they were handed and hand the result toupdate_or_create, which assigns wholesale. So reconnecting an integration to rotate a Firebase key or APNs.p8would silently reset an enabled policy todisabled, reopening the device takeover the feature exists to prevent. This was raised on feat(messaging): optional identity verification for push subscriptions #72488 by both Greptile and veria-ai and deferred until the flag became settable.Separately, Apple hands out the APNs auth key as a
.p8file and the setup modal only accepted pasted text, so connecting meant opening the file in an editor to copy the PEM out.Changes
Frontend
PushIdentityVerificationField, used by both the Firebase and APNs setup modals: a segmented control for disabled / optional / required, writingpush_identity_verificationinto the integration config.requiredcopy warns that devices stop being able to register if the app isn't sending tokens yet, since that is the failure mode worth naming before someone selects it..p8file as well as pasted. The upload reads the file, checks it looks like a PEM private key, and fills the samesigningKeyfield, so both routes converge on one value.Backend
push_identity_verificationthrough the serializer into bothFirebaseIntegration.integration_from_keyandApplePushIntegration.integration_from_key.preserved_push_confighelper: carries the existing policy forward on an upsert unless the caller explicitly sets a new one, so a credential rotation can't drop it. Unknown modes are rejected rather than written through.Chose to preserve just this key rather than merge
configwholesale. A blanket merge would also resurrect stale provider fields that a reconnect is meant to replace (a rotatedkey_id, a changedenvironment), which is a different and worse bug.Still create-only: both modals only POST a new integration, so the mode is set when connecting a channel. Changing it later means reconnecting, which now at least preserves the setting rather than silently clearing it. A proper edit path for a connected integration is a follow-up.
APNS before and after:
Firebase modal, before and after:
How did you test this code?
Automated tests (
/writing-tests), a matching pair per integration:test_reconnecting_preserves_identity_verification— connect withrequired, reconnect with rotated credentials, assert the policy survives (without the fix it silently resets todisabled).test_rejects_an_unknown_identity_verification_mode— unknown modes are rejected, not written through asdisabled.No frontend tests (presentational wiring, no branching logic worth pinning).
pnpm --filter=@posthog/frontend fixandruff check/ruff formatclean.Local E2E on a running stack — fresh team, real Firebase + APNs credentials:
.p8upload works.integration_from_keyhelpers against local Postgres (the code path the tests assert): create persists the mode, reconnect preserves it, explicit change overrides, unknown mode raises and leaves config intact.Needs CI: the two pytest files (behavior verified via the model path, not the pytest run itself);
mypyandtypescript:checkfail locally for environmental reasons only.Automatic notifications
Docs update
No posthog.com doc yet. The customer-facing setup instructions (how a backend mints the token, and the SDK's
pushIdentityProviderhook) should land once the iOS and Android SDK releases carrying it are out, since the docs aren't useful before then.🤖 Agent context
Autonomy: Human-driven (agent-assisted). Directed by @dmarchuk, assigned as DRI.
Authored with Claude Code. Skills invoked:
/writing-user-facing-copy(field labels and per-mode help text),/writing-tests(which is what led to testing the backend and not the frontend).Started as a frontend-only change. While tracing where the toggle's value would land, found that the serializer drops unrecognized config keys, which meant the toggle would have been silently inert. That pulled the deferred config-clobber fix into the same PR, since both are the same question of whether this key survives the write path.
Built the mode control as one shared component rather than duplicating it per modal, so the explanatory copy can't drift between Firebase and APNs. For the
.p8upload, reusedLemonFileInputand kept the existing textarea rather than replacing it, so anyone already pasting keys is unaffected.Kea typegen could not run here (
typegen:filefails to resolve its own module), so the generatedMakeLogicTypeblocks in both logic files were updated by hand to match the new form fields and actions. Typegen will reconcile them.