Repository navigation
Conversation
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a wizard branch:
Add Results will be posted here when complete. |
PostmarkMessageBase's constructor sets TrackLinks to None, so a .NET send tracks links only when the code sets it, as postmarker's sends do. The rewrite adds captureEngagement: false only when Postmark tracked neither. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ilgun's error rewrite axios with a permissive validateStatus and got with throwHttpErrors: false answer a failed status without throwing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ull follows the server Only an SDK message whose code never sets TrackLinks keeps the constructor's None; null and plain HTTP sends follow the server's setting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Links default The positional SendMessageAsync extension builds a PostmarkMessage, so its links were not tracked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…send leaves TrackOpens out Only TrackLinks has an SDK default of None in postmarker and .NET; TrackOpens defaults to null in both and follows the server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PostHog answers a used-up quota with status 429 and code quota_limited, which no retry fixes, so a retry or wait on 429 checks the code too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d's Node rethrow err.code keeps SendGrid's status for the old catch; err.posthogCode lets a 429 branch skip quota_limited. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-js keeps PostHog's code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and keep suppression writes in step with PostHog's list Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…from PostHog's suppression list Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ression list, and name Mailgun's suppression writes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… no imported list still holds the address, on every path of Step 5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…domains whose sites move Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s that mirrored additions made Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ain text off A dashboard copy shows no plain text both for a template without one and for one whose plain text is an empty string. Ask which, or read the template from the API; when nobody can tell, the site stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion and keep the answer A dashboard copy has no key to read the template from the API, so the question offers to make one. The answer goes into the template's plan note, so a copy made again in Step 7 asks nothing new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nly in Step 3 The template's note records its plain text however the template was read, so Step 7 never asks again, and the key is offered only in Step 3, before Step 5 asks the person to delete it. A site stays whenever neither the answer nor the API read settles it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PostHog's bulk add takes only bounce, complaint or manual, and writes its own reason text, such as "Imported (bounced)". The removal guard now keys on that prefix, and the import maps each Resend origin to the reason value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…them PostHog's list records no source an app could check before a removal, a bulk add skips an address removed earlier, and an app can't call the import tool. PostHog blocks a transactional send only to an address that hard-bounced, and records that itself. So each such call becomes a "Before cutover" item. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PostHog blocks a transactional send only to an address that hard-bounced, so the weekly re-import limits mail to bounced addresses, not complaints. A call that removes an address becomes an "After cutover" item the person acts on when a moved send fails with recipient_suppressed. A call that adds one names the action before the deploy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-import only now names bounces alone, and an app call that adds an address also records it where the moved sends can skip it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A moved send reaches every address an app's add call put on the provider's list, and an app-side flag misses past adds and survives a reactivation. So Step 3 asks once which sites rely on the call, and keeps them on the provider. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Step 3 asks which sites rely on the provider's list for addresses added by hand or by the app, and a non-answer keeps every site when the app adds addresses itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the list The suppression-list question gets its own Step 3 section, asks only about sites that would move otherwise, keeps them on a non-answer, and says what would let them move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e work The question now comes right after the field checks, so a site it keeps needs no template copy or PostHog template, and the plan note carries what would let the site move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
posthog-rs's async email send waits between retries with tokio::time::sleep and sends through reqwest, so a site on async-std or smol stays on Resend. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…472 Map Bounces, Complaints and Unsubscribes to PostHog's bounce, complaint and manual reasons, and leave out allowlists. Feed Step 3's suppression-list question with Mailgun's unsubscribes, which also stopped transactional sends. Drop the unsubscribes report count and the "which entries count" text that Step 5 no longer reads, and say that PostHog blocks a moved send only to an address that hard-bounced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed bd23423: 1 Should fix.
This is review round 6 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.
…errors httpx.HTTPError is the base of both RequestError and HTTPStatusError, so a catch of it after raise_for_status() took network failures too, unlike requests.HTTPError. Map it to a catch of every PostHog error, and httpx.RequestError alone to network failures only (ReviewHog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without raise_for_status(), a catch of httpx.HTTPError saw network failures only, so it converts like httpx.RequestError, not to a catch of every PostHog error (final read 15). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed f1b0c45: 1 Should fix.
This is review round 7 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.
…work-only A Mailgun PHP client built with NoopHydrator answers a 5xx as the response and throws HttpServerException on a network failure only, so its catch keeps only network_error and the getStatusCode() branch moves after it, instead of sending 5xx answers into the catch (ReviewHog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed b4e2c85: 2 Should fix.
This is review round 8 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.
…al requests catches A domain with archive_to posts a copy of each delivered message to a URL without o:archive-to on the send, and PostHog posts none, so its sites stay; account.md reads it from GET /v4/domains/<domain>. A requests catch of ConnectionError with Timeout leaves out other RequestException failures, so it tells one network failure from another and stays, as Step 7 says (ReviewHog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed e7b5794: 1 Should fix.
This is review round 9 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.
Mailgun's docs name no text part it makes for a send with html and no text, only t:text for a template, and PostHog makes one from the HTML whenever a send has no text. Such a site stays with `turns the plain-text part off`, as template-to-code.md says for a template (ReviewHog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus
marked this pull request as draft
November 14, 2026 17:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A person who sends with Mailgun and asks to move those sends to PostHog gets no help:
migrate-emailcovers Resend only.With this PR,
migrate-email-mailgunmoves Mailgun sends to PostHog's email send in each language where both Mailgun and PostHog have an SDK, or where the app posts to Mailgun's HTTP API, andwizard migrate mailgunruns it. Each call site either moves with the same failure behavior, rendering, and tracking, or stays on Mailgun with the reason the report names.Important
Stacked on #472, the core
migrate-emailskill. Its base staysfeat/migrate-email-all-sdksuntil #472 merges; merge that first. Each provider PR adds its own provider and lands in any order. The lists they all touch,config.yaml,AGENTS.mdand the skill test, conflict in one line each as they land.It adds one provider folder, one config variant, one
AGENTS.mdrow and the provider in the bundle test. The shared steps it uses come from #472.Nothing merges until code sends are live and usable for customers: Michael's rule for #451 and every PR stacked on it.
Impact
Sends that arrive with
migratedFrom: 'mailgun'show whether people use this skill to move off Mailgun. Read it 14 days after the Wizard runsmigrateas a family.Merge risk
Caution
🔴 Merge risk: high · two-way door until the next context-mill release ships the skill; one-way once agents run it, because a migration's changes persist in each app's code and PostHog account (senders, imported suppressions, keys, moved sends)
Tier: critical
wizard migrate mailgun. On the base, nothing runs. At HEAD the skill rewrites their Mailgun send code, and a missed rule harms silently once they deploy: a failure that used to pass now thrown (or the reverse), a refusal swallowed, a template rendered differently, open capture changed, or a suppressed address mailed. Silent harm at reach few is medium; the 559 changed lines with that row uncovered raise it one step.migrate-emailvariant and every shared step: this diff touches onlyreferences/mailgun/and one line each inconfig.yaml,AGENTS.md, and the test's provider list. The shadow diff against feat(migrate-email): move Resend sends in every PostHog SDK's language #472's head shows the new Mailgun bundle and its manifest and skill-menu entries only; every other bundle is byte-identical.Rollout
migratedFrom: 'mailgun', and their error rate, in the first weeks after the Wizard ships the variant. No alert pages on a wrong rewrite.Summary
references/mailgun/holds the three files each variant has:sdk-reference.md: Mailgun's libraries in Node.js (mailgun.jsand the oldermailgun-js), Python, Ruby, PHP, Go (v4 and v5) and Java, the HTTP API with the clients real sends use (requests, RestClient,Net::HTTP, curl, Guzzle,HttpClient, RestSharp), SMTP, and framework mailers. It covers how each reports errors (including thatmailgun.jsthrows a network failure as a status-400MailgunAPIError, andmailgun-phpasHttpServerException::networkError()), the EU region setting, the clients that send nothing (Ruby's test mode, Python'sdry_run), the send fields, stored templates, status reads, webhooks, and the APIs that are not sends.mapping.md:htmlsend withouttext(turns the plain-text part off, since PostHog would add a text part), batch sends,%unsubscribe_url%and its siblings,o:deliverytimeand the othero:options, customh:headers,v:values ando:tag,amp-html, MIME sends, mailing list recipients, SMTP and framework mailers,cc,bcc,attachmentandinline, and any field the table doesn't name.recipient-variableson a send to one address moves: the code fills%recipient.<name>%before the send. AReply-Towith several addresses becomes a list. AnIdempotency-Keymoves when the code bounds its length and characters.captureEngagement: PostHog adds its open pixel to every email, and counts clicks only for a template made in PostHog that is not marked sensitive. Untracked opens follow feat(migrate-email): move Resend sends in every PostHog SDK's language #472's Step 3: this folder names the fields and domain setting that mean no open pixel or nobody knows. Conflicting fields keep the site, and so does a domain whose unsubscribe footer is on or not checked: Mailgun adds that footer and PostHog doesn't. A domain whosearchive_toposts a copy of each delivered message to a URL, or whose setting isn't checked, keeps its sites too, since PostHog posts no copy.template-to-code.md): Handlebars values,if/unless/each/with/equalbecome code with Handlebars truth written out. A template made in PostHog, with Liquid, only when the person asks for one. Agoengine version, other helpers, partials, a header that holds{{,textorhtmlnext totemplate, an inheritedReply-Towith a name, and anequalwhose value can be a string in one send and a number in another keep the site.HttpServerExceptioncatch keepsnetwork_error, a Guzzle 7RequestExceptioncatch lets it through, Ruby's status0becomesnetwork_error, and a catch of one kind of network failure (Python'sTimeoutalone, .NET'sHttpRequestException,rest-client'sExceptionWithResponse) keeps the site. A plain HTTP send is read with its client's options, and an option or call that throws on some failed statuses only keeps the site, such as RestSharp'sThrowOnAnyErrorandPost, which pass a404. A status branch lets the statuses it never checked pass as before. Telling one kind of network failure from another keeps the site, as Step 7 says. Under "HTTP", a timeout on the request counts as the site's own deadline, which Step 7 keeps;mailgun.js's clienttimeoutis axios's, which the SDK can't keep, so its sites stay. A branch on400readsvalidation_error(plusnetwork_errorformailgun.js), and a branch on429skipsquota_limited. Node.js throws an error withtype,status,details,nameandcode.$workflows_email_*event. A click maps only for a template made in PostHog that is not sensitive, so a site whose clicks feed a handler moves only there. A handler that matches events by message id, av:value, or a tag keeps its sites on Mailgun, and so does one that acts ontemporary_failorunsubscribed, which have no PostHog event.account.md: Mailgun's SPF, DKIM, tracking andMXrecords next to PostHog's, a read-only API key that the person deletes at the end of Step 5, and the US or EU host for every read. It reads the template version a site sends (domain and account templates, checked by id when the site names one) and the domain's tracking settings. It imports the bounces, complaints and unsubscribes of the domains whose sites move, with PostHog'sbounce,complaintandmanualreasons, and leaves out allowlists. Since PostHog blocks a moved send only to an address that hard-bounced, it names Mailgun's unsubscribes in Step 3's suppression-list question, so a site that must not mail them stays (relies on Mailgun's suppression list).Evidence
581b5d7:vitest run scriptspasses 344 tests, the section-pointer check on this folder included, in 30 files;node scripts/build.jsbuilds with a clean tree.build.ymlruns only on PRs intomain; this PR's base isfeat/migrate-email-all-sdksuntil feat(migrate-email): move Resend sends in every PostHog SDK's language #472 merges. The lane gate above ran on HEAD.4260cda, before the core moved templates into the code) classified an 11-site fixture as expected; no run covers this head.090ac92: 287 bundles on the base, 288 at581b5d7; onlymigrate-email-mailgun(new),manifest.jsonandskill-menu.json(its entries and the build timestamp) differ.fleet proof gateat581b5d7:review: HEAD reviewed in final read 13 by gpt-6.1-sol;shadow-diff: evidence at HEAD; the judge passes proof, owed, review, risk and quality;gate: passwith the live-run, prod-shape and CI gaps that Michael accepted.migrate-email-skill.test.js):mailgunin the provider list runs the shared checks on the Mailgun bundle: every reference resolves, its three files and the eight rewrite files are present, no other provider's folder ships, and the template-in-code contract holds.Decisions
open pixel added: the person agreed; no or no answer stays (open tracking off), an unknown domain setting stays (open tracking not checked); opens tracked moves with a note when clicks were tracked; tracking never maps tocaptureEngagementtemporary_failorunsubscribedstays (its <event> events feed a webhook handler), as Resend does for its eventsbounce,complaintandmanual; allowlists left out. The unsubscribe count for the report is gone: Step 3's suppression-list question decides those sites, naming Mailgun's unsubscribesnetwork_errordoesn't say which kindproject-getand real-SDK testsprojects-get, and no PostHog mock404finding (read 9 widened it toThrowOnAnyError, checked against RestSharp's source); reads 7, 8, 10, 11, 12 and 13 came back clean (13 on the fifth core merge). After #472 merged: read 14 on the main merge and the suppressionreasonmapping, then reads 15 to 18, each on one ReviewHog or read fix (httpxHTTPError,NoopHydrator, domainarchive_towith a partialrequestscatch, anhtmlsend withouttext); 14, 16, 17, 18 and 19 came back cleanThrowOnAnyErrorandPost/PostAsyncpass a404by default, so they keep the site as clients that throw on some failed statuses only (fleet judge, final read 9)Involvement: ReviewHog, 4 rounds on this work after the 3 earlier threads (12 threads fixed, all resolved); qa-swarm, 2 panel rounds (round 1: fresh-eyes, correctness, risk, design, anti-overengineering, sweep, and the author's team perspective; round 2: the same without sweep) and 13 final reads. 13 swarm findings fixed, 3 rejected (the shadow diff, and two asks for agent-execution fixtures). Proof: live-run, prod-shape and CI recorded as gaps, which Michael accepted.
🤖 Agent context
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5, high). Reviews on GPT-6.1 Sol (high), the fresh-eyes lens on Opus 5.5.🤖 Generated with Claude Code