Repository navigation
feat(node): set a recipient's email topic preferences - #5511
Conversation
…Preferences
posthog.emails.setPreferences({ to, topics, allMarketing }) opts one recipient out of
email topics, or back in, in one request to /api/projects/@current/messaging/preferences/
with the client's project secret key. It resolves to { data, error } like send and get,
and retries network errors, rate limits and an unavailable service.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coherence📄 0 spec file errors · ⛔ 0 broken rules · 🚪 2 unchecked inputs · 🐛 0 bugs that came back What is Coherence?Coherence compares the code with its spec files ( This check read 44 parts of the code and 7 rules. Note These findings do not make this check fail. None of these findings are in files that this pull request changes. You do not need to fix them in this pull request. 🚪 Unchecked inputs (2)
What this means and how to fix itInput from outside this code comes in at these places. The spec does not say what checks that input. 10 more places were already like this when Coherence was added. This list does not show them. To close one of these, find the place by its name in the
To get the line to add, run |
Node SDK acceptance v2Commit: 0f9e590 Capture v0✅ 83 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1✅ 148 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Note 🤖 Claude Opus 5.5 responding on behalf of Michael Decisions for PostHog/posthog#127204 and #5511:
|
…try claim - An unknown topic now carries PostHog's `unknown_topic` code, so a caller can tell it from an instance without the preferences API. - The JSDoc says a retry that lands after a newer change undoes it, and that `to` must match the address Workflows sends to. - The preferences example test replies with the documented order, and the shared EmailSender docs cover preferences changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…arketing opt-out Opting in to one topic after an all-marketing opt-out lifts that opt-out on the server, so the example reply keeps `all_marketing: true`. The `topics` JSDoc now says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A transactional topic opted back in leaves `allMarketing` as it was, so the `topics` JSDoc names marketing topics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rences Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New commits were pushed — dismissing the stamphog approval from an earlier head. This PR no longer qualifies for automatic review.
There was a problem hiding this comment.
Not approved — escalated to a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The public API and authenticated preference-write path need independent assurance; the author's discussion comment does not provide it. Request a current-head review from Harley Alexander, another teammate, or an agent reviewer covering the contract and write behavior before re-requesting.
- Risky territory without independent assurance: Public API contracts — packages/node/src/emails/types.ts and packages/node/src/exports.ts expose a new recipient-preferences method and its public request and response types.; Data ingestion or write paths — packages/node/src/emails/emails.ts posts recipient subscription changes to the messaging preferences endpoint and retries transient failures.; Auth or security-sensitive surface — packages/node/src/emails/emails.ts adds a secret-key-authenticated operation that changes a recipient's email consent preferences.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 205L, 6F substantive, 327L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (327L, 9F, two-areas, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ 5b40c38 · reviewed head 5b40c38 |
…rences setPreferences now names the messaging_preference:write scope that the server requires since the scopes split. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SDK performance: posthog-nodeBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
…rences # Conflicts: # packages/node/references/posthog-node-references-latest.json # packages/node/src/client.ts # packages/node/src/emails/emails.ts # packages/node/src/exports.ts
There was a problem hiding this comment.
PostHog Review · Standard · reviewed ee18856: 1 Should fix.
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.
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The current code leaves @PostHog's substantive retry-ordering concern unresolved, and no qualifying current-head review covers the risky changes. Fix the concern and request review from Vincent (Wen Yu) Ge, Harley Alexander, or another teammate before re-requesting.
- PostHog Review (Standard) reviewed the current head: 1 finding, posted on the PR.
- posthog[bot] reviewed the current head.
- @PostHog on packages/node/src/emails/emails.ts: the retry-ordering concern remains unfixed. Lines 223-225 replay the same preferences POST through withRetries, whose loop waits and retries after network failures; the request has no deduplication key or ordering protection. An accepted opt-in with a lost response can therefore be replayed after a newer successful opt-out and overwrite it. Disable automatic retries for this operation or implement server-backed replay protection, then address the thread before re-requesting.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 213L, 6F substantive, 497L/11F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (497L, 11F, two-areas, feat) |
| stamphog 2.8.0 | .stamphog/policy.yml @ ee18856 · reviewed head ee18856 |
There was a problem hiding this comment.
Approved.
The public API and authenticated write path have independent assurance from the current-head PostHog Review. No concrete unresolved review concern or other policy refusal ground is present; the scanning-status discussion comment is not an eyes reaction.
- PostHog Review (Standard) reviewed the current head: no findings.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 213L, 6F substantive, 497L/11F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (497L, 11F, two-areas, feat) |
| stamphog 2.8.0 | .stamphog/policy.yml @ 0f9e590 · reviewed head 0f9e590 |
Main added emails.setPreferences (#5511), whose reply parser reads Reply.payload. This branch changed Reply to carry the raw text, so parsePreferencesReply now parses that text like the other reply parsers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Impact
messaging preferences api setevents from the server route.Caution
🔴 Merge risk: high · one-way door (merge-risk rubric)
emails.setPreferencescontract for good.not_founduntil the server route, feat(messaging): set a recipient's topic preferences with a project secret key posthog#127204, is live. A caller who ignoreserrorkeeps mailing someone who opted out.Changes
posthog.emails.setPreferences({ to, allMarketing?, topics? })saves a recipient's choices in one call.trueopts in,falseopts out.{ data, error }and never rejects, likeemails.send.dataholds the saved preferences by topic key. Opting back in to one marketing topic after an all-marketing opt-out returnsallMarketing: true.messaging_preference:writescope. Keys withhog_flow:writestill work.emails.send. A 404 does not.code: unknown_topic. A plainnot_foundmeans the PostHog instance has no preferences API yet.api_error.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
/tdd,/merge-risk.emails.send, with a thinner contract.emails.spec.tsdrivessetPreferenceswith a mockedfetch: request shape, omitted fields, 404, 503 retry, missing key, invalid input.unknown_topicfor an unknown topic, and refused empty input before any request.sendandgetscenarios against one scripted local app host. Requests and results are identical.Live run output
Decisions
emails.sendsetPreferencesemails.sendInvolvement: 2 questions answered, design pass, 1 GPT-6.1 Sol review round (2 fixed, 1 rejected), QA swarm panel of 7 lenses (Opus 5.5 fresh eyes, GPT-6.1 Sol for the rest, SDK spec conformance as the team lens: 5 fixed, 1 rejected, 1 follow-up), proof judge (1 fixed), 4 GPT-6.1 Sol final reads with no findings, live run and shadow diff captured.
🤖 Generated with Claude Code