feat(cdp): look up a team's verified organization members by Silthus · Pull Request #128944 · PostHog/posthog

Silthus · 2026-11-04T09:34:37Z

Problem

A team new to Workflows cannot send a single email until it verifies its own sending domain.
The sandbox sender may deliver only to verified members of the team's organization, so the worker needs a member lookup that fails closed.
Part 2 of the Node stack, split from #128250: #128943 (SES route) → this PR (member lookup) → #128945 (sandbox message) → #128946 (delivery). The product stack starts at #128947.

Merge danger

Note

🔵 Merge risk: low · two-way door (merge-risk rubric)

Nothing calls the lookup until #128946, so merging this PR changes no send.
What limits it: one new service, one helper and their test. No existing code changes.
Revert: git revert the merge commit. Nothing to clean up.

Changes

Nothing a person sees changes yet. #128946 calls the lookup.
getBlockedRecipients returns every address that is not a verified member, and flags the unverified members among them.
A member counts as verified only when is_email_verified is true. A null value counts as unverified.
A member counts only while the user is active and the organization is not deactivated. Addresses match after trim and lowercase.
Each call runs one fresh query with a 3 second statement timeout, raced against a 3 second deadline.
A pooled connection that arrives after the deadline goes back to the pool before BEGIN. A connection still busy at the deadline is destroyed.
Any error or timeout throws, so the caller skips the send.

How did you test this code?

CI report: the comment-density lines state pg pool and deadline behavior and the fail-closed rule for a removed member, so each answers why. The duplication scan did not run: its runner killed jscpd for memory.
Live run at this head: Not run yet.
organization-members.service.test.ts runs against the real test Postgres. It allows a padded or uppercase verified member, and flags unverified and null-verified members.
The same test blocks a removed member, an inactive member, a member of an inactive organization, and an outsider.
A controllable pool client shows that a late connection sends no SQL, and that a stalled BEGIN gets its client destroyed.

Earlier runs at older heads, not this head

Live run at older head (c36ef47). Nothing calls the lookup at this layer, so a script called it from the head build with a real PostgresRouter, against the kit database and the team's real organization.

Call	Result
Three verified members	None blocked
Unverified member	Blocked, flagged unverified
Address outside the organization	Blocked
Member address in upper case with spaces	Not blocked
Member deactivated by another session a moment before	Blocked on the next call
Organization `is_active` false, then NULL	Every member blocked, then open, as in the web middleware
Another session holds `ACCESS EXCLUSIVE` on the membership table	Fails after 3.0 s with the statement timeout; no lookup left running

Earlier live run through the send path, at d6b91d7

Run on a devbox at d6b91d7, the Node stack top with this PR's head 5195b18, merged with the product stack top 9379fbf. Same setup as #128946: private Django and Node cdp-api, a seeded org, and a local server that records each SES request. One Node process ran throughout.

Step	Result
Send to an unverified member	Skipped
Mark that member verified in Postgres, send again without a restart	Sent
Deactivate the verified member, send again	Skipped
Hold an `ACCESS EXCLUSIVE` lock on the membership table, send	Skipped after 3.0 s: "could not check organization members". No SES request. The stalled connection closed instead of going back to the pool, and its session never showed as `idle in transaction`
Release the lock, send again	Sent
One send to members in To, CC and BCC	2 lookups, before preparing and before delivery, one `SELECT` each

Not proven live: a lookup that gets its connection after the deadline. That needs an exhausted Node pool. The stall test covers it.
The lock send went straight to the Node cdp-api, because Django's own auth reads the locked table.

Test rationale: each case catches a lookup that lets a non-member through or hangs a send. No existing test covers member lookups in the worker.

Release status

No feature flag controls this change

Docs update

None. The "Sandbox delivery" section in #128946 describes the check.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /writing-tests, /writing-code-comments, /stacking-prs, /writing-pr-descriptions

More context for AI agents

Session: Claude Code. Review rounds 1 and 2 and the final read ran on Claude Opus 5.5 only, because the GPT reviewers were unreachable for the whole session.
Decisions: simplified from feat(cdp): send sandbox email through its ses identity to org members #128250's organization-members.service.ts: no LazyLoader, fresh option, limiter or deadline-slots.
Decisions: round 1 let null-verified members through; round 2 reverted that, because it contradicted the "verified members" decision.
Decisions: the caller owns the deadline, because #129049 shares one deadline between the slot wait and the transaction.
Decisions: the singleton team array and team_id column stay, because #129049 batches teams.
Follow-ups: live run at this head.
Read closely: runTransactionBeforeDeadline in nodejs/src/cdp/utils/deadline-transaction.ts, and lookupMembers in organization-members.service.ts.

Stack decisions, moved here from the visible body:

Question	Answer	Who	Where
Who gets a sandbox email	Verified members of the team's organization, checked at send time over To, CC and BCC	👤 Human	decision record
Sender identity	Fixed From address on a dedicated PostHog domain, "Acme via PostHog" display name, no From or Reply-To override	👤 Human	decision record
Where it can send	Workflow email steps and test sends only, not broadcasts or transactional email	👤 Human	decision record
How the sender is stored	One managed email integration per team, provider `sandbox`, created only by PostHog code	👤 Human	decision record
SES setup	One shared identity, configuration set and tenant for all sandbox sends	👤 Human	decision record
Landing order	Three-PR skeleton, then the flag to our team, then caps and polish (replaced landing the twelve-layer stack at once)	👤 Human	decision record
Node gate	Environment switch `WORKFLOWS_SANDBOX_SENDER_ENABLED`, no feature flag in CDP Node code	👤 Human	decision record
Member check in the skeleton	Keep it, although the ask was a skeleton with no limits	👍 Accepted default	decision record
Behavior with everything off	No change while the flag and switch are off	👍 Accepted default	decision record
Earlier PRs	Close #127149 to #127153, #127136 and #127137, each linked to its successor	👍 Accepted default	decision record
PR shape	Two native stacks of small PRs: Node (4) and product (5)	👍 Accepted default	decision record
Member lookup cache in the skeleton	Dropped; every check is one fresh lookup, the cache returns with the caps stack	👍 Accepted default	decision record
Member lookup that gets its connection late	Runs no query; the cap on waiting lookups returns with the cache	🤖 Agent	decision record
Tier budget	Sandbox sends never spend the team's tier budget	🤖 Agent	decision record
When members are checked	Before preparing the send and again fresh before delivery	🤖 Agent	decision record
Member lookup fails	Skip the send with a log	🤖 Agent	decision record
Blocked-send events	At most one per team, reason and hour	🤖 Agent	decision record
How a team gets the sender	Opening Channels with the flag on creates it	🤖 Agent	decision record
Managed sender guards	Nobody can edit, verify, delete or default the sandbox sender	🤖 Agent	decision record
Quoted member addresses	Skip the send, and canonicalize in the caps stack instead of respinning the skeleton	🤖 Agent	decision record
Member lookup connection at its deadline	Takes the client itself: a late client sends no SQL, a busy one is destroyed	🤖 Agent	decision record

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-11-04T09:34:53Z

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

⚠️

Duplication (Python) — 27 new duplicated blocks (worst 1017 tokens)

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️

Duplication (TypeScript) — 20 new duplicated blocks (worst 351 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy	Second copy	Lines	Tokens
`products/mcp_analytics/frontend/notifications/mcpAnalyticsNotificationsLogic.ts:171`	`products/product_analytics/frontend/notifications/productAnalyticsNotificationsLogic.ts:157`	77	351
`products/surveys/frontend/draftReview/surveyProvenanceLogic.ts:36`	`products/surveys/frontend/draftReview/surveyProvenanceLogic.ts:96`	57	339
`frontend/src/products.tsx:1857`	`products/product_analytics/manifest.tsx:44`	38	312
`frontend/src/scenes/insights/insightLogic.tsx:502`	`frontend/src/scenes/insights/insightLogic.tsx:553`	45	309
`frontend/src/scenes/insights/insightDataLogic.tsx:334`	`frontend/src/scenes/insights/insightDataLogic.tsx:392`	49	281
`frontend/src/scenes/insights/insightDataLogic.tsx:391`	`frontend/src/scenes/insights/insightLogic.tsx:342`	45	263
`products/logs/frontend/components/LogsSqlEditor/logsSqlEditorTrackingLogic.ts:17`	`products/tracing/frontend/components/TracingSqlEditor/tracingSqlEditorTrackingLogic.ts:17`	49	209
`products/logs/frontend/components/LogsSqlEditor/logsSqlEditorTrackingLogic.ts:17`	`products/metrics/frontend/components/metricsSqlEditorTrackingLogic.ts:18`	45	206
`products/ai_observability/frontend/playground/llmPlaygroundLogic.test.ts:321`	`products/ai_observability/frontend/playground/llmPlaygroundLogic.test.ts:442`	41	187
`products/ai_observability/frontend/playground/llmPlaygroundLogic.test.ts:272`	`products/ai_observability/frontend/playground/llmPlaygroundLogic.test.ts:324`	39	169
`products/ai_observability/frontend/modelPickerLogic.test.ts:268`	`products/ai_observability/frontend/modelPickerLogic.test.ts:499`	34	158
`common/storybook/.storybook/test-runner.ts:562`	`packages/quill/apps/storybook/.storybook/test-runner.ts:32`	41	137
`nodejs/src/logs/framework/resolve-log-buffer.ts:114`	`nodejs/src/logs/logs-ingestion-consumer.ts:627`	23	133
`frontend/src/scenes/surveys/surveyNotificationModalLogic.ts:1054`	`products/surveys/frontend/draftReview/surveyProvenanceLogic.ts:43`	13	90
`packages/quill/packages/charts/src/charts/TimeSeriesBarChart/TimeSeriesBarChart.tsx:134`	`packages/quill/packages/charts/src/charts/TimeSeriesComboChart/TimeSeriesComboChart.tsx:113`	27	85
`frontend/src/scenes/insights/insightLogic.tsx:320`	`frontend/src/scenes/insights/insightLogic.tsx:372`	16	82
`frontend/src/scenes/insights/insightDataLogic.tsx:361`	`frontend/src/scenes/insights/insightLogic.tsx:318`	15	79
`frontend/src/scenes/surveys/surveyNotificationModalLogic.ts:1083`	`products/surveys/frontend/draftReview/surveyProvenanceLogic.ts:72`	19	74
`services/mcp/src/lib/instructions.ts:208`	`services/mcp/tests/unit/instructions.test.ts:123`	37	74
`nodejs/src/servers/ingestion-logs-server.ts:195`	`nodejs/src/servers/ingestion-metrics-server.ts:127`	11	72

⚠️

Comment density — 4% of added code lines are comments (10 of 239)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File	Comment lines	Added lines
`nodejs/src/cdp/services/managers/organization-members.service.ts`	6	64
`nodejs/src/cdp/utils/deadline-transaction.ts`	4	46

This check does not block merging. It updates on every push and clears when the share drops.

posthog

PostHog Review · Standard · reviewed d5da565: 1 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

…on late A lookup that waits for a pooled connection past its deadline no longer runs its query once the connection arrives, so a slow pool does not replay abandoned lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The member lookup now takes its pooled connection itself. A connection that arrives after the deadline goes back to the pool before BEGIN, and a connection still busy at the deadline, in BEGIN or any later statement, is destroyed instead of held or reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…org-members

parameterai · 2026-11-04T19:41:10Z

Risk: No findings

This increment is a two-line fail-closed tightening: organization members with a NULL is_email_verified are now treated as unverified and blocked from sandbox sends, with the test updated to match. It strictly reduces who can receive a sandbox email and introduces no new attack surface.

_{Sentinel reviewed 9ddec86 · Review settings}

…org-members

A new stamphog review started for this PR — the fresh verdict replaces this approval.

A null is_email_verified marks an account from before email verification existed. The web app lets those accounts log in and change email as verified and never asks them to verify, so a strict check skipped every sandbox send to them with no way out. Only an explicit false now counts as unverified. Also drop rejection handlers that Promise.race already attaches, and test the deadline cases through the router's public connect, on a bare Postgres router instead of a full hub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ox-split/org-members

A new stamphog review started for this PR — the fresh verdict replaces this approval.

…org-members

A member whose account predates email verification has a null is_email_verified and never proved the address. The sandbox sender mails from a PostHog-owned identity and was decided to reach verified members only, so null counts as unverified again, as in the OAuth email_verified claim. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A new stamphog review started for this PR — the fresh verdict replaces this approval.

stamphog

Approved.

The security-sensitive lookup has owning-team authorship and targeted tests, providing the policy's assurance path. No concrete unresolved review concern or showstopper was found; the discussion bot's account of null verification does not match the current code.

Gate mechanics and policy version

Gate		Result
prerequisites	✓	all clear
deny-list	✓	no deny categories matched
size	✓	122L, 2F substantive, 266L/3F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1c-medium (266L, 3F, single-area, feat)
stamphog 2.7.0		`.stamphog/policy.yml` @ `9ddec86` · reviewed head `9ddec86`

Silthus · 2026-11-09T13:27:46Z

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Replaced by #131731 in the isolated sandbox rebuild.

feat(cdp): look up a team's verified organization members

d5da565

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pr-assigner-resolver-posthog Bot assigned Silthus Nov 4, 2026

This was referenced Nov 4, 2026

chore(cdp): build the ses route in one place #128943

Closed

feat(cdp): build a sandbox email with its identification footer #128945

Closed

feat(cdp): send sandbox email through its ses identity to org members #128946

Closed

Silthus added this pull request to stack #128953 November 4, 2026 09:35

posthog Bot reviewed Nov 4, 2026

View reviewed changes

Comment thread nodejs/src/cdp/services/managers/organization-members.service.ts Resolved

This was referenced Nov 4, 2026

feat(cdp): send sandbox email through its ses identity to org members #128250

Closed

feat(cdp): share sandbox member lookups and bound waiting ones #129049

Closed

Silthus and others added 3 commits November 4, 2026 12:36

Merge workflows-sandbox-split/ses-route into workflows-sandbox-split/…

5eb4eee

…org-members

Merge workflows-sandbox-split/ses-route into workflows-sandbox-split/…

c36ef47

…org-members

Silthus marked this pull request as ready for review November 4, 2026 19:40

Silthus added the posthog-review label Nov 4, 2026

pr-assigner-resolver-posthog Bot requested a review from a team November 4, 2026 19:42

Silthus added the stamphog Request AI approval (no full review) label Nov 4, 2026

This comment was marked as outdated.

Sign in to view

Merge workflows-sandbox-split/ses-route into workflows-sandbox-split/…

5fb1102

…org-members

This comment was marked as outdated.

Sign in to view

Silthus and others added 2 commits November 6, 2026 14:17

Merge branch 'workflows-sandbox-split/ses-route' into workflows-sandb…

ba45807

…ox-split/org-members

This comment was marked as outdated.

Sign in to view

Silthus and others added 2 commits November 6, 2026 20:13

Merge workflows-sandbox-split/ses-route into workflows-sandbox-split/…

61e6c3e

…org-members

stamphog Bot approved these changes Nov 6, 2026

View reviewed changes

Silthus marked this pull request as draft November 9, 2026 10:02

Silthus mentioned this pull request Nov 9, 2026

feat(cdp): add sandbox email delivery policy #131731

Merged

Silthus closed this Nov 9, 2026

Navigation Menu

Repository navigation

feat(cdp): look up a team's verified organization members - #128944

feat(cdp): look up a team's verified organization members#128944
Silthus wants to merge 10 commits into
workflows-sandbox-split/ses-routefrom
workflows-sandbox-split/org-members

Silthus commented Nov 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 4, 2026 •

edited

Loading

Uh oh!

posthog Bot left a comment

Uh oh!

Uh oh!

parameterai Bot commented Nov 4, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

stamphog Bot left a comment

Uh oh!

Silthus commented Nov 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Silthus commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Merge danger

Changes

How did you test this code?

Release status

Docs update

🤖 Agent context

Uh oh!

github-actions Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 CI report

Uh oh!

posthog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

parameterai Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

stamphog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Silthus commented Nov 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Silthus commented Nov 4, 2026 •

edited

Loading

github-actions Bot commented Nov 4, 2026 •

edited

Loading

parameterai Bot commented Nov 4, 2026 •

edited

Loading