R4: declared, templated, golden-tested mailers; per-mail send transactions

.claude/skills/wave-run/SKILL.md

@@ -62,10 +62,10 @@ subagents, whose context is thrown away.
   branch>` (the only git command it may run). Merge each finished worktree by committing there and
   cherry-picking onto the wave branch; resolve shared files (a registry, an allowlist) by script, and
   conflicts in code by hand.
-  A worktree has no `cmd/web/node_modules` and no built `cmd/web/dist` (both untracked), and the E2E tests
-  serve `cmd/web/dist`: a task that doesn't change assets links the main checkout's
-  (`ln -s /home/user/pcom/cmd/web/dist cmd/web/dist`); one that does links `node_modules` the same way and
-  builds once. Parallel worktrees share golangci-lint's lock: "parallel golangci-lint is running" means
+  A worktree has no `cmd/web/node_modules` and no built `cmd/web/dist` (both untracked). The E2E tests
+  serve stub assets and need neither; only the browser tests (`make test-ui`) need a build. A task that
+  runs them links `node_modules` from the main checkout and lets `make test-ui` build. Don't link the main
+  checkout's `dist`: it exists only after a `make test-ui` there, and a dangling link breaks E2E. Parallel worktrees share golangci-lint's lock: "parallel golangci-lint is running" means
   retry, not a lint failure. Delete finished worktrees (`git worktree remove`) once merged; their copies
   of the tree show up in every repository-wide grep.
   With a worktree of its own, an agent may run `make test-ui` freely. After merging parallel tasks, run the

.env.example

@@ -32,6 +32,7 @@ HSTS=false
 STATIC_CACHE=false
 MEDIA_PERMA_CACHE=false
 SHOW_ERRORS=true
+DEV_ROUTES=true
 REPORT_PANICS=false
 LOG_LEVEL=debug
 

AGENTS.md

@@ -14,6 +14,7 @@ the one for the area you touch:
 | How pcom behaves on purpose: accounts, login, visibility, feeds, comments | `docs/product.md` |
 | Layers: handlers, services, repositories, service errors, the arch test; configuration and tooling | `docs/architecture.md` |
 | Markdown rendering, view types, custom renderers | `pkg/markdown/AGENTS.md` |
+| Adding or changing a mail: inputs, samples, templates, goldens | package doc of `pkg/mail/mail.go`, item 6 in `docs/testing.md` |
 | RSS feed fetching and image budgets | `pkg/feedops/AGENTS.md` |
 | Writing tests: libraries, test DB, factories, mocks, E2E and browser tests, ground rules | `docs/testing.md` |
 | User guide, website, screenshots | `website/`, `docs/guide/` |
@@ -44,7 +45,8 @@ longer holds rather than annotating it. Wave names, dates and incident stories b
 **No magic numbers.** A limit or tunable value (a length cap, a page size, a timeout) is a setting in
 `pkg/config` with its default there, handed to the service as an option; the service exports the same default
 for callers built without configuration, and tests use that constant, not the literal. Pattern:
-`Limits.ProfileAboutMaxLength` → `accounts.WithProfileAboutMaxLength`.
+`Limits.ProfileAboutMaxLength` → `accounts.WithProfileAboutMaxLength`. A new setting also appears in
+`web serve --help`, whose golden the same diff rewrites (`UPDATE_GOLDEN=1 go test ./cmd/web -run TestHelp`).
 
 ## Reading code economically
 
@@ -91,4 +93,5 @@ for callers built without configuration, and tests use that constant, not the li
   the report prints, or re-run the single test with `-v`.
 - Golden files: after `UPDATE_GOLDEN=1`, check `git diff --stat`, not the contents.
 - Pipe unavoidable noisy commands (`docker compose`, `yarn`, `gh run view`) through `tail -n 40` or `grep`.
+  Never pipe a quiet target: the pipe takes `tail`'s exit status, so a failure no longer stops a `&&` chain.
 - Don't paste code or logs into reports to a coordinator.

cmd/web/client/html/dev--mail-index.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <title>Mail previews</title>
+    <link rel="stylesheet" href="{{ static_asset "main.css" }}" />
+  </head>
+  <body>
+    <main class="prose">
+      <h1>Mail previews</h1>
+      {{ range .Mails }}
+      <h2>{{ .Name }}</h2>
+      <ul>
+        {{ range .Samples }}
+        <li><a href="/dev/mail/{{ .Name }}">{{ .Name }}</a></li>
+        {{ end }}
+      </ul>
+      {{ else }}
+      <p>No mails are registered.</p>
+      {{ end }}
+    </main>
+  </body>
+</html>

cmd/web/client/html/dev--mail-sample.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <title>{{ .Sample }} - mail preview</title>
+    <link rel="stylesheet" href="{{ static_asset "main.css" }}" />
+  </head>
+  <body>
+    <main class="prose">
+      <p><a href="/dev/mail">All mails</a></p>
+      <h1>{{ .Sample }}</h1>
+      <p>Mail: {{ .Mail }}</p>
+      {{ if .Err }}
+      <p>The sample failed to render: {{ .Err }}</p>
+      {{ else }}
+      <h2>Subject</h2>
+      <p>{{ .Subject }}</p>
+      <h2>Text</h2>
+      <pre>{{ .Text }}</pre>
+      <h2>HTML</h2>
+      <iframe sandbox srcdoc="{{ .HTML }}" title="HTML body" style="width: 100%; height: 32rem; border: 1px solid currentColor;"></iframe>
+      {{ end }}
+    </main>
+  </body>
+</html>

cmd/web/serve.go

@@ -44,7 +44,7 @@ func (c *serveCmd) Execute([]string) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	dbSender := dbsender.NewSender(repo.New(db), mailjet.NewSenderFromConfig(&cfg.Mail.Mailjet))
+	dbSender := dbsender.NewSender(repo.New(db), mailjet.NewSenderFromConfig(&cfg.Mail.Mailjet), dbsender.WithRetryIntervals(cfg.Mail.RetryIntervals))
 
 	go dbSender.RunPoller(ctx, cfg.Mail.PollInterval)
 
@@ -105,6 +105,7 @@ func appConfig(cfg config.Serve, staticAsset app.StaticAssetFunc) app.Config {
 		SecureCookies:         cfg.Web.SecureCookies.On(),
 		HSTS:                  cfg.Web.HSTS.On(),
 		StaticCache:           cfg.Web.StaticCache.On(),
+		DevRoutes:             cfg.Web.DevRoutes.On(),
 		ShowErrors:            cfg.Web.ShowErrors.On(),
 		ReportPanics:          cfg.Web.ReportPanics.On(),
 		ProfileAboutMaxLength: cfg.Limits.ProfileAboutMaxLength,

cmd/web/testdata/serve_help.golden

@@ -55,6 +55,9 @@ Help Options:
                                               [$LOG_LEVEL]
           --enable-pprof=                     Serve net/http/pprof on :8081
                                               (default: false) [$ENABLE_PPROF]
+          --dev-routes=                       Serve the /dev pages (mail
+                                              previews); local development only
+                                              (default: false) [$DEV_ROUTES]
 
     Mail:
           --sender-address=                   From address of every mail
@@ -65,6 +68,11 @@ Help Options:
           --email-poll-interval=              How often the mail queue is sent
                                               (default: 10s)
                                               [$EMAIL_POLL_INTERVAL]
+          --email-retry-intervals=            Waits between attempts to send a
+                                              mail; a mail fails after one
+                                              attempt more than there are waits
+                                              (default: 10s, 60s, 30m)
+                                              [$EMAIL_RETRY_INTERVALS]
 
     Mailjet:
           --mj-api-key-public=                Mailjet API key public

docker-compose.yml

@@ -131,6 +131,7 @@ services:
       STATIC_CACHE: "false"
       MEDIA_PERMA_CACHE: "false"
       SHOW_ERRORS: "true"
+      DEV_ROUTES: "true"
       REPORT_PANICS: "false"
       LOG_LEVEL: debug
       PCOM_WATCH_POLL: ${PCOM_WATCH_POLL:-}

docs/architecture.md

@@ -27,6 +27,17 @@ Pure packages stay where they are (`pkg/markdown`, `pkg/links`,
   `sender.Sender`. The queue drops a repeated (type, unique id), so a
   feature that notifies again about the same object needs its own key, and
   its test sends two events in a row.
+- **Mail content** is declared in `pkg/mail`, one file per mail: a typed input,
+  samples built from in-memory models, and templates in `pkg/mail/templates`
+  (subject and text through `text/template`, HTML through `html/template`).
+  Escaping is `html/template`'s job; a `template.HTML` field is the one way
+  trusted HTML (a rendered markdown body) enters. `mail.All()` is the only
+  list of mails: the golden test and the `/dev/mail` preview read it. The
+  package doc of `pkg/mail/mail.go` is the how-to for adding a mail.
+- **Delivery.** `dbsender` sends each due mail in its own transaction, so one
+  failure doesn't resend the others; the waits between attempts are
+  `--email-retry-intervals`, and a mail fails after one attempt more than
+  there are waits.
 - **Errors** (`pkg/service`, which re-exports gogo's `apperr`; code uses the
   `service` names): `ErrNotFound` (also for "exists, but you may not see
   it"), `ErrForbidden`, `ErrNeedsLogin`, `ErrConflict`, and

docs/archive/history/r4.md

@@ -0,0 +1,79 @@
+# R4 — Mailers (done 2026-10-07, branch `claude/zealous-shannon-ieo14d`)
+
+**Built.** All 14 mails are declared in `pkg/mail`, one file per mail: a typed input with a `Header()`
+method, samples built from in-memory models, and three embedded templates (subject and text through
+`text/template`, HTML through `html/template`), parsed at init. `declare(...)` registers each mail, so adding
+one touches no shared list, and `mail.All()` returns them sorted by name. `TestMails_Goldens` renders every
+sample into `testdata/<sample>.golden`, and each constructor has a `_MatchesSample` test against its
+sample. `Outgoing` was folded into `mail.Envelope`. The five admin notifications moved from `pkg/admin`
+into `pkg/mail` as `Admin…`; `pkg/admin` keeps only `ClonedCustomRecovery`, whose report the router passes
+to `AdminPageFailure` so `pkg/mail` doesn't import gin. The `/dev/mail` preview lists every sample and
+renders its subject, text and sandboxed HTML. It is mounted only with `--dev-routes`/`DEV_ROUTES`, which
+is off by default and on in `docker-compose.yml` and `.env.example`. `dbsender` now sends each due mail in
+its own transaction (`ListDueEmailIDs`, then `LockDueEmail ... FOR UPDATE SKIP LOCKED` per mail), and
+its retry schedule is `--email-retry-intervals` (default `10s,60s,30m`, the same four attempts as before).
+
+**Decisions.**
+- **Whitespace may change; content may not.** Every pre-existing golden differed only in whitespace when
+  it moved to templates (`git diff -w` empty), including the admin goldens, compared by hand with the old
+  text/html pairs. Two later changes were deliberate: samples use `SampleSite` with root
+  `https://pcom.test`, so links became absolute, and the post-prompt-answer sample's post id became
+  `post-2`, so it can't be confused with the prompt's post id.
+- **The preview is behind a flag, not an admin route.** The owner chose a dev-only flag that production
+  never sets over an admin page or a CLI that writes files.
+- **No job queue in R4.** Mail already entered `outgoing_emails` in the same transaction as its change, so
+  a queue would mostly help the feed poller. It became its own wave, R8, which took over open question Q9.
+  R4 fixed only the sender's real defect: one transaction for the whole batch, so a failed update or
+  commit resent every mail already sent.
+- **Constructors kept their signatures**, so the services didn't change. The exceptions were the
+  `Envelope` rename and the admin mails' move. The latter dropped an unused `site` parameter from two
+  admin constructors.
+
+**What went wrong.**
+- S0 added two settings and broke `cmd/web`'s `serve --help` golden; M0 reported it. AGENTS.md now says a
+  new setting rewrites that golden in the same diff.
+- The worktree recipe linked the main checkout's `cmd/web/dist`, which didn't exist, and the dangling link
+  broke E2E in every worktree. Each agent worked around it. E2E serves stub assets and needs no build; the
+  `wave-run` skill now says so.
+- The coordinator piped `make lint-q` through `tail` in an `&&` chain, so a lint failure (the shared lock)
+  didn't stop the commit and cherry-pick. A later `make check-q` passed. AGENTS.md now forbids piping a
+  quiet target.
+- Three parallel tasks each wrote the same `sampleEnvelope` test helper; the clash at merge moved it into
+  `golden_test.go`.
+- The assertion audit found the comment mails' samples built through the constructors' own mapper, which
+  made their `_MatchesSample` test circular. It also found an empty `SampleSite` that let a constructor
+  ignore its site, the same id for a post and a prompt's post, a send-error test that exercised no mail
+  code (it became a rollback test in the posts service), and redundant admin asserts. All were fixed, each
+  proven by a mutation; the rule is now item 6 in `docs/testing.md`. Two pre-existing weak tests in
+  `pkg/admin/panics_test.go` became #218.
+- `UPDATE_GOLDEN=1` doesn't create a missing golden, so deleting the goldens to find orphans told nothing.
+  `docs/testing.md` says so.
+- In this sandbox, three browser tests fail as documented (older Chromium, no outbound network); CI's
+  `browser` job decides them.
+
+**Left out.** The job queue (R8). No `pkg/mail/AGENTS.md`: the package doc of `mail.go` holds the
+conventions, and the AGENTS.md table points there. The branch is the session's designated branch rather
+than `refactor/r4-mailers`, because the session could push nowhere else.
+
+**Docs and skills changed.**
+- `docs/architecture.md`: mail content (declaration, escaping, registry) and delivery (one transaction per
+  mail, the retry setting).
+- `docs/testing.md`: item 6 rewritten as the rule for mail goldens and constructor tests.
+- `docs/running.md`: the `/dev/mail` preview and `DEV_ROUTES`.
+- `docs/gogo-extraction.md`: the mail definitions now live in `pkg/mail/mail.go`; the queue entry points
+  at R8.
+- `AGENTS.md`: a table row for mails, the serve-help golden on new settings, never piping a quiet target.
+- `wave-run` skill: worktrees need no `dist` for E2E; don't link the main checkout's.
+- Plan: R8 added (and Q9 moved into it), R4's line dropped from the index, R7 no longer mentions R4.
+
+**Cost.**
+- **Coordinator:** 1 session, 112 turns, peak 236k context, 135k of tool results; flags: `cat`×6.
+- **Subagents:** 9 (M0 strong; M1 cheap; M2–M6 and the audit fixes mid; the audit strong), 204 turns and
+  548k of tool results between them. None needed a retry or escalation. The heaviest took 85k of results
+  (M4).
+- **Across the wave:** `cat`×42, raw builds×1.
+- **Skills:** `wave-run`, `wave-close` and `test-failure` once each in the coordinator. No subagent used a
+  skill, including `frontend-htmx`, which M5's prompt named.
+- **Compared with R5** (228 turns, peak 363k): the coordinator used half the turns and a third less peak
+  context. The contract task went to a subagent instead of the coordinator, and verification relied on
+  mutation checks and one audit rather than reading tests.

docs/gogo-extraction.md

@@ -18,7 +18,7 @@ Roughly ordered by payoff:
 | Image media server | `pkg/media/server` (+ `storage/s3`) | libvips resizing by named class, caching server, concurrency limiter, S3 storage behind `MediaStorage`. Self-contained apart from `media.ErrNotFound`. **Highest value.** |
 | Goldmark extensions | `pkg/markdown/mdext/*` | linkrenderer (target/rel by view), lazyload, headershift, videoembed (YouTube), blocktags (gallery). They depend on `types.HTMLView` and `types.Replacer`, which would need to become generic options. |
 | CSP nonce middleware | `pkg/util/ginhelpers/csp` | Parametrize the source lists (today they're built from env at init). |
-| Mail definitions and preview | R4 (planned) | Typed definitions with sample factories, an `html/template` layout, golden helpers and a preview handler. Design it so it imports nothing from pcom, and extract it right after R4. |
+| Mail definitions and preview | `pkg/mail/mail.go`, `pkg/web/app/routes_dev.go` | Typed definitions with samples, embedded `text/template`/`html/template` parts, a registry, and the `/dev/mail` preview. `mail.go` depends on pcom only through `links.Site` in `SampleSite`. |
 | Webpack/asset manifest loader | `LoadStaticManifest` in `pkg/web/app/funcmap.go` | `static_asset` funcmap with CDN prefix. |
 | Template helpers | `toMap` in `pkg/web/app/funcmap.go`, `renderHumanTime` (`pkg/util/date`) | A small `gogo/tmplfuncs`. |
 | Postgres session store | `pkg/pgsession` | Wraps `antonlindstrom/pgstore` (stale) for gin-contrib/sessions, plus user-in-context. Replace pgstore when extracting. |
@@ -29,6 +29,6 @@ Roughly ordered by payoff:
 | Timezone list | `pkg/util/tz.go` | IANA zones for settings dropdowns. |
 | JSON action helper | `pkg/web/app` | `jsonAction[T]`: bind, call, `{explanation}` on error. Pairs with gogo's `API` renderer. |
 | RSS fetcher | `pkg/feedops/reader` | Size and time-limited fetch with MIME validation. Maybe; it's pcom-specific today. |
-| DB-backed mail queue | `pkg/mail/sender/dbsender` | **Don't extract as is.** R4 considers replacing it with a general job queue with a transactional outbox. Extract that instead, if it happens. |
+| DB-backed mail queue | `pkg/mail/sender/dbsender` | **Don't extract as is.** R8 plans to replace it with a general job queue; extract that instead, if it happens. |
 | Boolean switch setting | `pkg/config.Switch` | A go-flags boolean that can default to true (`--x`, `--x=false`, `X=false`; empty is an error). Belongs next to `settings.Secret` as `settings.Switch`. |
 | tommy test container | `pkg/testutil/tommy` | One tommy container per test binary (Mailjet API, S3 bucket, read-back API URLs). Belongs next to `testcontainers/postgres` as `testcontainers/tommy`. |

docs/implementation-plan.md

@@ -10,7 +10,7 @@ support read directly. Rules for reading and verifying code cheaply are in `AGEN
 
 Where the modernization is going:
 
-- declared, golden-tested mailers (R4);
+- one job queue for mail and feeds (R8);
 - structured logging through one [zap](https://github.com/uber-go/zap) logger, passed explicitly (R7);
 - no unmaintained or duplicate dependencies (R6);
 - shared plumbing in [gogo](https://github.com/can3p/gogo) (`docs/gogo-extraction.md`).

docs/open-questions.md

@@ -8,10 +8,6 @@ When the owner answers, the answer goes where it applies (`docs/product.md` for
 `docs/architecture.md` for how it is built, the wave file for what a wave builds) and the question is
 deleted here. The reasoning goes into the history of the wave that acts on it.
 
-- **Q9. The RSS poller downloads feeds and images inside the DB transaction**
-  that holds the feed row lock (`refreshFeeds` in `pkg/service/feeds/poller.go`).
-  With `GlobalImageDownloadTimeout` per item, a transaction can stay open for minutes. Restructure when
-  moving email and feed work to a general job queue?
 - **Q10. Audit and idempotency for mutations (narrowed 2026-09-24).** The
   layering is decided (`docs/architecture.md`): services take an actor, check authorization,
   and HTTP, API and CLI are transports over them. Still open: should every

docs/plan/r7.md

@@ -10,7 +10,7 @@ to `log/slog`, and 17 `fmt.Print*` calls, spread over `cmd/web`,
 `pkg/markdown` and `cmd/scripts` (now the `cmd/web` subcommands).
 
 Most of the code that logs is in services, and the log level is already a
-setting. R7 can run alongside R4 only if the tasks own disjoint files.
+setting.
 F7's translation worker logs too: whichever of F7 and R7 merges second
 converts it.