Skip to content

feat(emails): send transactional email and read its status - #406

Merged
Silthus merged 14 commits into
mainfrom
feat/emails-send
Nov 5, 2026
Merged

Silthus merged 14 commits into
mainfrom
feat/emails-send

Conversation

@Silthus

@Silthus Silthus commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

A Go backend can't send transactional email through PostHog today. posthog-go has no emails API, while posthog-node and posthog-python send by template and, in PostHog/posthog-js#5509 and PostHog/posthog-python#1171, by rendered content too. This PR adds Client.Emails(). Send takes a template made in PostHog, by its ID, or content your code rendered, named by a key. Get reads the email's status by its message ID. Part of map Silthus/posthog#461.

result, err := client.Emails().Send(ctx, posthog.CodeEmail{
	To:          "jane@example.com",
	DistinctID:  "user_123",
	TemplateKey: "password-reset",
	Subject:     "Reset your password",
	HTML:        renderedHTML, // React email: render it to HTML in your app first
})

Impact

Pays into Grow Workflows through workflows_emails billed actions, which count API sends (measured on the team's Workflows usage dashboard). In scope: Messaging, transactional email. Expected move: small, up, as it opens API sends to Go backends. Run cost: n/a.

Caution

Merge risk: medium, one-way door once released.

What changes

  • Client.Emails() returns Emails, with Send(ctx, Email) and Get(ctx, messageID). A no-op client's Emails refuses every call with client_disabled.
  • Two forms, one Send. Email is sealed and has two types, in the style of Enqueue(Message):
    • TemplateEmail{To, TemplateID, TemplateData, DistinctID, IdempotencyKey, MaxRetries} posts template_id and message_data.
    • CodeEmail{To, TemplateKey, Subject, HTML, Text, From, ReplyTo, CaptureEngagement, DistinctID, IdempotencyKey, MaxRetries} posts a flat body. CaptureEngagement is a *bool, so it is on unless set to false.
  • Requests go to POST /api/projects/@current/messaging/send/email/ and GET /api/projects/@current/messaging/emails/{id}/ on the app host, with Config.SecretKey as the bearer token: the project secret API key that the flag poller already uses. The app host is Config.AppHost, or the app host of a PostHog Cloud Endpoint (us.i goes to us.posthog.com, eu.i to eu.posthog.com), matched without regard to case, a trailing slash or port 443. Any other endpoint, such as a proxy, needs AppHost: email requests never go to Endpoint.
  • Arguments are checked before any request, as Node and Python check them: the recipient, the template ID, the template key, the subject and body, ReplyTo, the idempotency key and MaxRetries. The client state comes next: disabled, closed, no app host, no secret key.
  • Every send carries an idempotency key, a UUID v4 unless you pass one, kept across retries. Network errors, idempotency_key_in_flight, 429 except quota_limited, and 503 retry up to Config.EmailRequestMaxRetries (default 2) or a send's MaxRetries, each 0 to 9 like the client's other retry options. The wait is 2^n s with jitter, at most 30 s, or Retry-After when longer. A Retry-After over 30 s fails at once. Get retries the same way, as both reference SDKs do.
  • Errors are *EmailError values with PostHog's Code, Message, Why, Fix, DocsURL, Field, StatusCode and RetryAfter, plus a coarse Name from the spec's table. errors.Is finds ErrSDKDisabled, ErrClosed and context.DeadlineExceeded through Unwrap. An error never holds the secret key or the request body.
  • Warnings in a 202 are logged and returned in SendEmailResult.Warnings.
  • Close waits for email requests in flight within its context, then cancels them.
  • New Config fields: AppHost, EmailRequestTimeout (default 15 s), EmailRequestMaxRetries and MigratedFrom, which sets X-PostHog-Migrated-From. Validate checks each.
  • Out of scope for now: local templates (TemplateData on a code send, the content hash, the upload by key), headers, tags, and any rendering dependency. Cc, bcc and attachments come in #408, stacked on this PR. The app renders the email and passes HTML.

Spec divergence. PostHog/sdk-specs openspec/specs/emails/spec.md covers the template form and the status read. The code form and CaptureEngagement go beyond it, matching posthog-node in PostHog/posthog-js#5509 and posthog-python in PostHog/posthog-python#1171.

💚 How did you test it?

Test seams: Client.Emails().Send and .Get on a real client built with NewWithConfig, against an httptest server or a fake transport. The tests pin each body and header, each refusal before a request, each error code and Name, retries, Retry-After, the request timeout, a cancelled context, a closed or disabled client, and Close during a retry wait. Three Example functions document the API.

Gate at 80ee4a5 passes: gofmt -l, go mod tidy with no diff, go vet ./..., go test -race ./..., bin/api-diff against the regenerated api/public-api.txt, and the otel and posthogmcpsdk modules. A full go test -race ./... under Go 1.21.13, the module's floor, passes too.

Live run at 8e181df against a local PostHog stack: Django at PostHog/posthog#127123 (51409d9), cdp-api and the email and hogflow workers at PostHog/posthog#127122 (b5889c9), and maildev. A Go program drove the branch through a recording proxy. All six cases passed:

Case Observed
TemplateEmail by ID with TemplateData 202 with an ID; the mail rendered the data
CodeEmail with HTML, Text, From, ReplyTo, CaptureEngagement: false 202; the mail had that subject, html, text, from and reply-to; the body carried capture_engagement: false
The same send twice with one IdempotencyKey Same ID; Replayed false, then true; one mail
From on an unverified domain *EmailError sender_not_verified, validation_error, HTTP 400; no mail
Get on the code send's ID sent, with CreatedAt and StatusAt
A wrong secret key *EmailError authentication_failed, missing_api_key, HTTP 401

Cross-version run at 8e181df against master's server (762afdf), without the two server PRs. Both send forms get a 400 required on template, because master still takes the nested body. Get reads a status once it exists: it gets a not_found for the first seconds, then sent. An unknown ID gets not_found.

The raw request and response pairs, the maildev readbacks and the driver are saved with fleet proof. After it, 5417c2d closes a client in one unit test, and 8cef8a9 and c7d0a9c make an answer cut off mid-body a retryable network error for every status that keeps its Retry-After (found by PostHog Review and the fleet review). The live cases never hit those paths. Then a QA swarm panel (round 7: fresh eyes on Claude, eight lenses on GPT-6.1 Sol) led to 80ee4a5: the 0 to 9 retry range, Cloud host matching without regard to case or port 443, RetryAfter kept on an accepted reply without a message ID, uuid.Validate in place of a regex, one cancellation path in request, and the AppHost doc. Each fix has a test that fails without it. Those paths don't change what the live cases sent or received. The narrow round 8 on the fixes found nothing in the code.

Shadow diff at 80ee4a5 against main (fee6566): the same 26 inputs (capture with and without feature flags, identify, alias, group identify, exception, every Enqueue type, remote and local flag evaluation with PersonalApiKey and SecretKey, Flush, Close twice, Enqueue after Close) give the same requests, return values, errors and log lines on both. Each side matches four repeat runs, and a planted change shows up in the diff. So nothing outside Emails() changes, Close included.

fleet proof gate passes at 80ee4a5: live run, cross-version and shadow diff, CI green, review round 8, judge pass.

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Question Answer Who Where
Scope of the port Template form, code form, status read; local templates and extra fields wait 👤 Human comment
React email rendering The app renders and passes HTML; no rendering dependency 👤 Human comment
Release order Not before the server's code form deploys 👤 Human comment
Shape of Send One Send(ctx, Email), sealed Email, two types 🤖 Agent comment
Which key authenticates Config.SecretKey, as in Node and Python 🤖 Agent comment
Does Get retry Yes, as both SDKs and the spec do 🤖 Agent comment
Adding Emails to Client Minor release, by the Flush precedent 🤖 Agent comment
Email without a cloud endpoint missing_app_host until AppHost is set 🤖 Agent comment
Today's server takes only the nested body Keep the contract's flat body and release after PostHog/posthog#127123 deploys; no nested fallback 🤖 Agent comment

Involvement: no grilling rounds; reviews: one GPT-6.1 Sol design read, six fleet proof review rounds (GPT-6.1 Sol), then a QA swarm panel of nine lenses and a narrow round on its fixes, which found nothing in the code; three PostHog Review passes, every finding fixed or answered on its thread; live run and cross-version run against a local PostHog stack.

  • Written by Claude Code, Claude Opus 5.5 (claude-opus-5-5), as the posthog-go slice of map Map: send code-managed email templates with one PostHog call Silthus/posthog#461, directed by Michael.
  • Reviews: a GPT-6.1 Sol design read; its two minor findings (one retrying request path, a racy Close test) are fixed in 78c6e6c. A fleet proof review round (GPT-6.1 Sol) found three edge cases: a nil *TemplateEmail panicked, a context ending during a retry wait lost its cause, and a 2xx with trailing bytes after valid JSON passed. All three are fixed in 0ffc1f6, each with a test that fails without the fix. The Go 1.21 race job then caught goccy/go-json's MarshalNoEscape reading a stack-built body as garbage. The email bodies now encode with encoding/json, HTML unescaped, in 8e181df, and a full go test -race ./... under Go 1.21.13 passes. A third review round on the fixes found nothing.
  • QA swarm (round 7): six findings fixed in 80ee4a5. Rejected: the flag poller sending SecretKey to Endpoint (unchanged on main: local evaluation already does it whenever SecretKey is set; the AppHost doc now says so), a shadow diff for Close (nothing is replaced; the drain is a no-op without email requests), and an encoder error branch Go requires. Follow-up for sdk-specs: spec coverage for the code form, CaptureEngagement and MigratedFrom.

🤖 Generated with Claude Code

Add Client.Emails(), a port of posthog.emails from posthog-node and
posthog-python. Send takes a TemplateEmail (a template made in PostHog,
by ID) or a CodeEmail (content the app rendered, named by a key), posts
the flat body to /api/projects/@current/messaging/send/email/ on the app
host with the project secret key, and retries with one idempotency key.
Get reads /api/projects/@current/messaging/emails/{id}/. Close waits for
email requests in flight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus Silthus self-assigned this Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

posthog-go Compliance Report

Date: 2026-11-05 11:04:18 UTC
Duration: 169290ms

✅ All Tests Passed!

111/111 tests passed


Capture_V1 Tests

✅ 94/94 tests passed

View Details
Test Status Duration
Endpoint And Method.Targets V1 Endpoint ✅ 9ms
Endpoint And Method.Does Not Use Legacy Endpoints ✅ 17ms
Required Headers.Has Authorization Bearer Header ✅ 7ms
Required Headers.Has Content Type Json ✅ 7ms
Required Headers.Has Posthog Sdk Info Format ✅ 6ms
Required Headers.Has Posthog Attempt Header ✅ 7ms
Required Headers.Has Posthog Request Id ✅ 6ms
Required Headers.Has Posthog Request Timestamp ✅ 7ms
Required Headers.Has User Agent ✅ 6ms
Body Format.Body Has Created At And Batch ✅ 6ms
Body Format.No Api Key In Body ✅ 7ms
Body Format.No Sent At In Body ✅ 6ms
Event Format.Event Has Required Root Fields ✅ 6ms
Event Format.Event Uuid Is Valid ✅ 16ms
Event Format.Event Timestamp Is Rfc3339 ✅ 7ms
Event Format.Distinct Id Is String ✅ 6ms
Event Format.Distinct Id At Root Not Properties ✅ 6ms
Event Format.Custom Properties Preserved ✅ 6ms
Event Format.Set Properties Preserved ✅ 7ms
Event Format.Set Once Properties Preserved ✅ 6ms
Event Format.Groups Properties Preserved ✅ 6ms
Event Format.Sdk Generates Uuid If Not Provided ✅ 6ms
Event Format.Event Has Required Root Fields Batch ✅ 9ms
Event Format.Event Uuid Is Valid Batch ✅ 9ms
Event Format.Event Timestamp Is Rfc3339 Batch ✅ 9ms
Event Format.Distinct Id Is String Batch ✅ 8ms
Event Format.Distinct Id At Root Not Properties Batch ✅ 19ms
Event Format.Custom Properties Preserved Batch ✅ 9ms
Event Format.Set Properties Preserved Batch ✅ 9ms
Event Format.Set Once Properties Preserved Batch ✅ 9ms
Event Format.Groups Properties Preserved Batch ✅ 9ms
Event Format.Sdk Generates Uuid If Not Provided Batch ✅ 9ms
Batch Behavior.Multiple Events In Single Batch ✅ 31ms
Batch Behavior.Batch Envelope Smoke ✅ 20ms
Batch Behavior.Flush With No Events Sends Nothing ✅ 4ms
Batch Behavior.Flush At Triggers Batch ✅ 1008ms
Batch Behavior.Created At Reflects Batch Creation Time ✅ 8ms
Deduplication.Generates Unique Uuids ✅ 15ms
Deduplication.Different Events Same Content Different Uuids ✅ 9ms
Deduplication.Preserves Uuid On Retry ✅ 5133ms
Deduplication.Preserves Timestamp On Retry ✅ 5136ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5134ms
Deduplication.No Duplicate Events In Batch ✅ 32ms
Header Behavior On Retry.Attempt Header Starts At One ✅ 6ms
Header Behavior On Retry.Attempt Header Increments On Retry ✅ 10139ms
Header Behavior On Retry.Request Id Preserved On Retry ✅ 5138ms
Header Behavior On Retry.Different Requests Have Different Request Ids ✅ 2014ms
Header Behavior On Retry.Request Timestamp Changes On Retry ✅ 5136ms
Response Format Validation.Success Response Has Uuid Keyed Results ✅ 7ms
Response Format Validation.Success Response Has Ok For Each Event ✅ 29ms
Response Format Validation.Success No Retry After When All Ok ✅ 27ms
Response Format Validation.Success Retry After Present When Retry Events ✅ 130ms
Response Format Validation.Success No Retry After When Drop Only ✅ 28ms
Response Format Validation.Response Echoes Request Id ✅ 6ms
Retry Behavior.Retries On 408 ✅ 5135ms
Retry Behavior.Retries On 500 ✅ 5136ms
Retry Behavior.Retries On 503 ✅ 5136ms
Retry Behavior.Retries On 504 ✅ 5135ms
Retry Behavior.Retryable Errors Have Retry After ✅ 2133ms
Retry Behavior.Respects Retry After On Retryable Error ✅ 8134ms
Retry Behavior.Does Not Retry On 400 ✅ 2133ms
Retry Behavior.Does Not Retry On 401 ✅ 2133ms
Retry Behavior.Does Not Retry On 402 ✅ 2134ms
Retry Behavior.Does Not Retry On 413 ✅ 2132ms
Retry Behavior.Does Not Retry On 415 ✅ 2133ms
Retry Behavior.Non Retryable Errors Have No Retry After ✅ 2133ms
Retry Behavior.Implements Backoff ✅ 15144ms
Retry Behavior.Max Retries Respected ✅ 15143ms
Partial Batch Handling.Handles 200 Full Success ✅ 2009ms
Partial Batch Handling.Handles 200 With All Ok ✅ 3034ms
Partial Batch Handling.Does Not Retry Dropped Events ✅ 3031ms
Partial Batch Handling.Does Not Retry Limited Events ✅ 3033ms
Partial Batch Handling.Prunes Ok Events On Partial Retry ✅ 5136ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry ✅ 5136ms
Partial Batch Handling.Retries Only Retry Events From Partial ✅ 5136ms
Partial Batch Handling.Partial Retry Preserves Uuids ✅ 5137ms
Partial Batch Handling.Partial Retry Attempt Header Increments ✅ 5133ms
Partial Batch Handling.Partial Retry Request Id Preserved ✅ 5134ms
Partial Batch Handling.Respects Retry After On Partial ✅ 5136ms
Partial Batch Handling.Unknown Result Treated As Terminal ✅ 3032ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry ✅ 3033ms
Compression.Sends Gzip Content Encoding ✅ 17ms
Compression.No Content Encoding When Disabled ✅ 7ms
Compression.Compressed Body Is Decompressible ✅ 17ms
Error Handling.Does Not Retry On Unknown 4Xx ✅ 2132ms
Event Options.Cookieless Mode Override ✅ 7ms
Event Options.Disable Skew Correction Override ✅ 7ms
Event Options.Process Person Profile Override ✅ 6ms
Event Options.Product Tour Id Override ✅ 7ms
Event Options.Unset Options Omitted ✅ 7ms
Event Options.Options Override In Batch ✅ 8ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties ✅ 17ms
Geoip And Historical Migration.Historical Migration Set In Body ✅ 7ms
Geoip And Historical Migration.Historical Migration Absent By Default ✅ 7ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 16ms
Request Payload.Flags Request Uses V2 Query Param ✅ 17ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 18ms
Request Payload.Flags Request Omits Authorization Header ✅ 17ms
Request Payload.Token In Flags Body Matches Init ✅ 17ms
Request Payload.Groups Round Trip ✅ 18ms
Request Payload.Groups Default To Empty Object ✅ 16ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 17ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 17ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 17ms
Request Lifecycle.No Flags Request On Init Alone ✅ 3ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 7ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 30ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 18ms
Retry Behavior.Retries Flags On 502 ✅ 17ms
Retry Behavior.Retries Flags On 504 ✅ 18ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 18ms

One request method sends, retries and classifies a reply that isn't a 2xx,
so Send and Get parse a successful reply once. The Close test checks the
retry reached the server before Close returned, instead of a race on the
result channel.

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

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 53dd2f6: 2 Must fix, 3 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

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.

Comment thread emails.go Resolved
Comment thread posthog.go Resolved
Comment thread emails.go Resolved
Comment thread emails.go Resolved
Comment thread emails.go Resolved
@Silthus

Silthus commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decisions for this PR:

  • 👤 Scope of the port Match posthog-node and posthog-python: template form, code form, status read. Local templates, cc, bcc, headers, tags and attachments wait for their server slice.
  • 👤 React email rendering The app renders and passes HTML; the SDK takes no rendering dependency.
  • 👤 Release order Never merge before the server's code form deploys; a code send gets a 400 until then.
  • 🤖 Shape of Send One Send(ctx, Email) with a sealed Email and two types, TemplateEmail and CodeEmail, as Enqueue(Message) works; not two methods.
  • 🤖 Which key authenticates Config.SecretKey, the project secret API key the flag poller uses, as one secret key serves both in Node and Python; no separate emails key.
  • 🤖 Does Get retry Yes, with the client's retry limit, following both SDKs and the sdk-specs emails spec.
  • 🤖 Adding Emails to Client A minor release, as Flush was added; the changeset says a custom Client needs the method.
  • 🤖 Where email goes without a cloud endpoint Refuse with missing_app_host until Config.AppHost is set, so a custom ingestion proxy never receives the secret key.
  • 🤖 Today's server takes only the nested template.id body Keep the flat body of the contract, as posthog-python's main (feat: send a flat body from emails.send posthog-python#1155) and posthog-node (feat(node): send email content from code with emails.send posthog-js#5509) send it, and release only after feat(messaging): send an email with inline html from code posthog#127123 deploys. A nested fallback would serve a shape that is going away. The cross-version run against master shows every send gets a 400 until then; Get already works.

Silthus and others added 4 commits November 2, 2026 16:05
…ng bytes

- Send refuses a nil *TemplateEmail or *CodeEmail with invalid_form instead
  of panicking.
- When the context ends during a retry wait, the error wraps ctx.Err(), so
  errors.Is finds context.DeadlineExceeded.
- A reply that isn't JSON as a whole carries no fields, so a 2xx with
  trailing bytes is an api_error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
goccy/go-json's MarshalNoEscape hides its argument from escape analysis, so
a body built on the stack can move while it encodes. The Go 1.21 race tests
caught it reading the template body as garbage and panicking. The email
bodies now use the standard library encoder with HTML escaping off, and a
test pins the HTML as written on the wire.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus marked this pull request as ready for review November 2, 2026 18:01
@Silthus Silthus added the stamphog Request AI approval (no full review) label Nov 2, 2026
@Silthus
Silthus requested a review from a team as a code owner November 2, 2026 18:01

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 5417c2d: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

Comment thread emails.go Resolved

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — this change needs a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

Two substantive concerns from @PostHog remain unresolved, so its current-head commented review does not provide concern-free assurance for the risky changes. Address the interface compatibility concern in posthog.go and failed-response reads in emails.go, then request a fresh review from @PostHog or a human 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 posthog.go:156: Client still requires the additional Emails method, while .changeset/emails-send.md selects a minor release; external implementations lose interface compatibility. RELEASING.md:35-39 distinguishes backward-compatible features from breaking changes, and no independent acceptance of this concern is present.
  • @PostHog on emails.go:504-511: response-body read failures are preserved only for successful HTTP statuses. A truncated 409 response loses its retryable error code and becomes a non-retryable API error, so the transport-recovery concern remains unfixed.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 868L, 5F substantive, 1937L/9F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (1937L, 9F, two-areas, feat)
stamphog 2.7.0 .stamphog/policy.yml @ 5417c2d · reviewed head 5417c2d

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Nov 2, 2026
A dropped connection after a 409's headers lost idempotency_key_in_flight
and became a non-retryable api_error. A failed body read now is a network
error for every status, so the client retries it with the same
idempotency key.

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

Silthus commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 8cef8a9: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

Comment thread emails.go Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus Silthus added the stamphog Request AI approval (no full review) label Nov 2, 2026
@Silthus

Silthus commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed c7d0a9c: no findings. This pull request is large, so the review ran in 4 parts with less depth than usual.

stamphog[bot]

This comment was marked as outdated.

- Cap EmailRequestMaxRetries and a send's MaxRetries at 9, as MaxRetries
  and FeatureFlagRequestMaxRetries are, so one send can't retry for hours
  and hold Close.
- Match a PostHog Cloud endpoint written with upper case or the default
  :443 port to its app host.
- Keep Retry-After on a 2xx answer that lacks its message ID or status.
- Check UUIDs with google/uuid instead of a second pattern.
- Merge the caller's context and the shutdown context once per request,
  which removes the separate shutdown branch of the retry wait.
- Move the reasons in function bodies into named helpers, and correct the
  AppHost doc: SecretKey also feeds local flag evaluation on Endpoint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stamphog
stamphog Bot dismissed their stale review November 4, 2026 07:14

New commits were pushed — dismissing the stamphog approval from an earlier head. This PR no longer qualifies for automatic review.

@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 80ee4a5: no findings. This pull request is large, so the review ran in 4 parts with less depth than usual.

stamphog[bot]

This comment was marked as outdated.

… reads

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stamphog
stamphog Bot dismissed their stale review November 5, 2026 07:48

New commits were pushed — dismissing the stamphog approval from an earlier head. This PR no longer qualifies for automatic review.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — escalated to a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

The disclosed changes enter public-contract, credential-handling and write-path territory, but all supplied reviews are on older commits and no owning-team or strong-familiarity assurance is reported. Request a current-head review covering these areas from Codex, Claude or a human teammate before re-requesting.

  • Risky territory without independent assurance: Public API contracts — posthog.go adds Emails to the exported Client interface, requiring downstream custom implementations to add a method; emails.go, emails_error.go and config.go also introduce exported types and configuration.; Auth or security-sensitive surface / secrets — emails.go sends a secret API key in bearer-authenticated requests and adds app-host selection and redirect handling to control where that credential goes.; Data ingestion or write paths — emails.go introduces transactional email POST requests with recipient content, idempotency keys and automatic retries.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 913L, 5F substantive, 2065L/9F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (2065L, 9F, two-areas, feat)
stamphog 2.7.0 .stamphog/policy.yml @ 6a05673 · reviewed head 6a05673

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Nov 5, 2026
…cut-off reply

Email requests carry the secret key, so Validate now refuses an http
AppHost unless the new AllowInsecureAppHost option is set, as in
posthog-dotnet.

A reply that breaks off mid-body keeps its HTTP status on the
network_error. Such an error stays retryable whatever its status: the
cut-off body can't say what PostHog decided, and the retry under the
same idempotency key gets the full answer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread emails.go Resolved
@veria-ai

veria-ai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

@Silthus

Silthus commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Cross-family review of head 40ee07b by GPT-6.1 Sol (high effort), since Claude wrote the code. Result: no findings.

Scope, the three areas from the last StampHog run:

  • Public API: Client.Emails(), TemplateEmail, CodeEmail, the new config options including AllowInsecureAppHost, and error inspection through errors.As and StatusCode. OK.
  • Secret key: which requests and headers carry Config.SecretKey, app host choice, redirects and TLS, and that the key never reaches a log, an error string or a panic. OK.
  • Send and status write paths: request building and validation, idempotency keys across retries, retry and Retry-After handling, timeouts, cancellation, shutdown, and reply parsing. OK.

The first pass on 6a05673 found two real defects, both fixed in 40ee07b:

  • An http AppHost sent the secret key without TLS. It now needs AllowInsecureAppHost, as in posthog-dotnet.
  • A reply cut off mid-body lost its HTTP status. It now keeps the status, and every network_error stays retryable under the same idempotency key.

Three other first-pass findings don't apply to this PR. The flag poller sending SecretKey to Endpoint and %+v of Config printing the key are both behavior on main. The template_id and message_data body is one the send endpoint accepts. go vet and go test -race ./... pass on Go 1.21, and CI is green on 40ee07b.

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

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — this change needs a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

Independent review is reported for the current head, but @veria-ai's substantive credential-isolation concern remains supported by the code. Address that thread with credential isolation or trusted-host routing and a regression test, then request a teammate or agent review before re-requesting.

  • PostHog Review (Standard) reviewed the current head: no findings.
  • @veria-ai on emails.go:297, reiterated by @veria-ai[bot]: the credential-isolation concern remains unfixed. The email sender uses effectiveSecretKey(), and posthog.go:325-339 also starts the flag poller with that key and Endpoint; featureflags.go:829-830, 860-867 and 1207 send an authenticated definitions request to that endpoint independently of AppHost. Although the poller predates this change, the new email feature still requires credentials that can reach an ingestion proxy, even when email requests refuse to run. Isolate email credentials from flag polling or route secret-bearing polling to the trusted app host, add a regression test using distinct ingestion and app hosts, and obtain independent acceptance of this thread before re-requesting.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 932L, 5F substantive, 2094L/9F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (2094L, 9F, two-areas, feat)
stamphog 2.7.0 .stamphog/policy.yml @ 40ee07b · reviewed head 40ee07b

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Nov 5, 2026
When Config.AppHost is set, the local evaluation poll of /flags/definitions
and GetRemoteConfigPayload now go to it instead of Endpoint, so a reverse
proxy at Endpoint never receives the secret key. Without AppHost, both keep
using Endpoint as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus and others added 2 commits November 5, 2026 10:29
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus merged commit 03e1647 into main Nov 5, 2026
34 checks passed
@Silthus
Silthus deleted the feat/emails-send branch November 5, 2026 12:10
Silthus added a commit that referenced this pull request Nov 5, 2026
#406 landed on main as a squash, with its last fixes (transactional_email
scopes, https app host, secret-bearing flag requests to the app host). This
branch now differs from main by its own change only: Cc, Bcc and
Attachments on a CodeEmail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants