[Silthus]

Problem

An app sending a receipt from code can't attach the PDF, which the send contract allows.

Changes

• A code send delivers its files through SES. A file with a contentId shows inline.
• The bytes live in cyclotron_email_attachments (#127995), so retries and Kafka snapshots never carry them.
• A missing or changed file fails the send. It never sends without its files.
• The worker reads and deletes only the rows of the job's own team. An email step can't list files itself, so only an accepted code send has them.
• A database error while loading the files reschedules the send with backoff, like an SES throttle. The 7-day sweep bounds the retries.
• The worker loads the files after every check that can skip or delay the send, and keeps them out of the job state that a reschedule writes back.
• The API refuses every file type that SES or Gmail blocks: the union of both lists. Validation errors never repeat a value from the request.
• The idempotency fingerprint leaves out the file bytes, so the same key with other files counts as a reuse.
• The worker deletes the rows only after it writes the job's final outcome. A failed enqueue keeps them, because the job may be queued anyway. The janitor sweeps leftovers after 7 days. Metrics track the table size.
• A worker whose job lock moved to another worker keeps the rows while the job is still available or running, so the new owner can still send the files.
• Mechanical: the attachment store and the conversion watchers share one LazyPool helper, which opens the Cyclotron pool on first use. Conversion watchers keep their shutdown order: no new pool, then drain in-flight writes, then close.

flowchart LR
  A[cdp-api]:::phRed --> J[(cyclotron_jobs.state<br/>send content)]:::phGray --> W{{email worker}}:::phBlue --> S[SES]:::phRed
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
  classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;

Loading

flowchart LR
  A[cdp-api]:::phRed --> J[(cyclotron_jobs.state<br/>file metadata only)]:::phGray --> W{{email worker}}:::phBlue --> S[SES]:::phRed
  A --> F[(cyclotron_email_attachments)]:::phGray --> W
  W -. delete on terminal outcome .-> F
  JN{{janitor}}:::phBlue -. sweep after 7 days .-> F
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
  classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;

Loading

Warning

Merge danger: medium. Deploy #127995 first. To roll back, turn the flag off and wait for 0 here, because an older worker drops queued files:

SELECT count(DISTINCT a.job_id) FROM cyclotron_email_attachments a
JOIN cyclotron_jobs j ON j.id = a.job_id WHERE j.status IN ('available', 'running');

How did you test this code?

Test rationale: tests cover binary files to SES, a retry after an SES throttle, a missing file, cleanup per outcome, and no bytes in the state.

• Live run: both SDKs delivered byte-identical files to maildev, and a body over 5 MiB got a 400 with nothing queued.
• Cross-version: old jobs run on new workers.
• Hogbox preview: the optional preview did not come up at 3bf8a16, though every job of its workflow passed. It failed at cb6469d too, because hogland had no capacity to place it. This PR changes no frontend, so the preview shows nothing it adds.
• Prod shape: no stored record in production reaches a changed branch other than the no-files path. #127995 created the table in both regions, and this PR adds its only writer, so the table holds no rows yet.

Prod-shape excerpt

1. cyclotron_email_attachments rows: none. #127995 created the table, and this PR adds its only writer.
2. Queued email jobs: all queued by master, so all take the no-files branch.
3. Queued workflow jobs: cleanup deletes only when state.send lists attachments.
4. Conversion watcher rows: writes keep their statement.

The full logs live in private proof storage.

- Shadow diff: master's conversion watcher service and this one ran the same ten scenarios against a test database. The stored rows, metrics and shutdown drain came out byte-identical.

Release status

• This change is behind a feature flag and is not available to users

Docs update

• None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

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

More context for AI agents

• Session: T3 Code slice thread "Attachments" of map Map: send code-managed email templates with one PostHog call Silthus/posthog#461, Claude Code.
• Decisions: Michael chose the Cyclotron side table over S3 or inline state. The table moves to S3 if its metrics show it grows too large.
• Follow-ups: the alert rule lives in the charts, outside this repo: max(cdp_email_attachments_table_bytes) > 5e9 for 30m.
• Decisions: the migration moved to feat(cdp): add the email attachments table to cyclotron #127995, because migrations-separation refuses it beside service code.
• Decisions: an old worker sends a queued attachment job without its files. The flag closes the deploy window, and the rollback drain query closes the rollback window.
• Read closely: email-attachments.service.ts, the store in cdp-api.ts, and the cleanup in cdp-cyclotron-worker-hogflow.consumer.ts.

🤖 Generated with Claude Code

[github-actions]

🤖 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) — clean

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) — clean

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.

✅ Comment density — 1% of added code lines are comments (8 of 980)

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/conversion-watchers/conversion-watchers.service.ts	8	16

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

❌ Hogbox preview — build failed

The preview didn't come up for commit 3bf8a16. See the build log for the failing step. It'll retry on the next push.

_{Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this section is the status.}

[posthog]

PostHog Review · Standard · reviewed f79130a: 1 Must fix, 4 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.

[posthog]

P1 · Set BASE64 transfer encoding for SES attachments

For binary PDFs or inline images, this attachment object omits ContentTransferEncoding, so SES defaults to SEVEN_BIT, which cannot safely represent their bytes in the delivered MIME message. The SDK's base64 encoding of RawContent only handles transport to SES. Set ContentTransferEncoding: 'BASE64' on every attachment to preserve binary content.

[Silthus]

Fixed in 8095678. Every SES attachment carries ContentTransferEncoding: 'BASE64', and the email service test checks a PDF with bytes outside ASCII.

[posthog]

P2 · Skip locked attachment rows during expiry cleanup

When an attachment sweep takes longer than the cleanup interval, overlapping runOnce() calls select the same expired rows and wait on their locks. These waits consume the shared pool and block stalled-job recovery, which runs after the sweep. The catch cannot recover while queries remain blocked; select batches with FOR UPDATE SKIP LOCKED and bound their execution time.

[Silthus]

Fixed in 6f1c414. The sweep selects its batch with FOR UPDATE SKIP LOCKED, like the janitor's other cleanups, so an overlapping run skips rows another run holds.

[posthog]

P2 · Reschedule sends when attachment reads fail transiently

A transient database error from loadAttachments enters the generic catch, which marks the send finished with provider_error. executeSendEmail then deletes its attachment rows, permanently dropping an accepted email even though SES was never called. Distinguish database availability errors from missing or corrupt files, and reschedule those sends while retaining their rows.

[Silthus]

Fixed in 4a75f30. A database error while loading the files now raises EmailAttachmentsUnavailableError, which reschedules the send with backoff and keeps its rows. Only missing or changed files fail the send.

[posthog]

P2 · Bound attachment database operations with deadlines

When PostgreSQL stops responding or an attachment delete waits on a lock, this pool has no connection or query timeout. The awaited operation never finishes, blocking the worker's entire Promise.all batch while periodic heartbeats prevent stalled-job recovery; even cleanup after SES acceptance blocks completion. Configure bounded connection and statement deadlines.

[Silthus]

Not changing this here. The attachment pool connects to the Cyclotron database and matches its other pools: the worker, manager and subscription matcher pools set no connection or statement deadline either. If Postgres stops responding, the worker's heartbeats to that same database stall too, so stalled-job recovery still applies. The only other lock holder on these rows is the janitor sweep, a single statement that now skips locked rows. Deadlines for all Cyclotron pools would be a separate change.

[posthog]

P2 · Match attachment metadata length limits to SES

An attachment with a 79-character contentId passes AttachmentSchema, as does a MIME contentType longer than 78 characters. SES caps both ContentId and ContentType at 78, so the API returns 202 and queues files that the provider permanently rejects instead of reporting invalid input. Enforce the SES limits on both fields before accepting the send.

[Silthus]

Fixed in 6f1c414. contentId and contentType are capped at 78 characters, the SES limit, so the API refuses a longer value with a 400 before it queues the send.

[veria-ai]

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

[parameterai]

Blocked attachment extensions are bypassed with a trailing dot or space in the filename

A sender with a code-send key can attach an executable that the blocked-extension list is supposed to refuse. The strip added here (const { attachments: ... } = args[0]) makes the AttachmentSchema filename check in nodejs/src/cdp/services/hogflows/transactional-email.ts:151-157 the sole gate on every delivered attachment's filename, and that check derives the extension from the text after the last dot with no trimming: "invoice.exe." yields an empty extension and "invoice.exe " yields "exe ", neither of which is in BLOCKED_ATTACHMENT_EXTENSIONS (transactional-email.ts:153-154). Windows strips trailing dots and spaces when the recipient saves the file, so the delivered attachment becomes invoice.exe. The filename regexes at transactional-email.ts:148 and :140 block path separators and control characters but not a trailing space or dot.

How:

1. Call the transactional email send endpoint with a valid send key.
2. In send.attachments, include a file named invoice.exe. (or invoice.exe ) with base64 PE content.
3. AttachmentSchema passes: name.lastIndexOf('.') finds the trailing dot and name.slice(dot + 1) is "" (or "exe "), which the blocked set does not contain, so the API stores the file and queues the send.
4. SES delivers the executable; a recipient saving it on Windows gets invoice.exe.

Fix: In the filename refine at transactional-email.ts:151-157, trim trailing dots and whitespace before deriving the extension (and reject the name if the trimmed result is empty).

---

_{React with 👍 if useful or 👎 if not}

[Silthus]

Valid as a gap in this check, but no file gets past it. The public entry point is Django's code send, and #127988 refuses invoice.exe. and invoice.exe with blocked_file_type before it calls cdp-api: it strips trailing dots and spaces the way Windows does when it saves the file. cdp-api's /send is internal, and since 2a930a2 a workflow email step can't list files. Making the cdp check agree with Django's is a follow-up for #127985's schema, so this stack doesn't need another round of proofs for it.

[trunk-io]

😎 Merged successfully - details.

[posthog]

PostHog Review · Standard · reviewed dab7651: 1 Must fix.

[posthog]

P1 · Accept and forward attachments through the public send endpoint

A public code send containing attachments still fails validation: Django's SendEmailSerializer rejects the unknown field, and _send_code_email and _accepted_send never forward files. Consequently, SDK callers cannot reach this new attachment delivery path. Add attachment validation and forwarding to the public send pipeline.

[Silthus]

Out of scope for this PR. The Django side is #127988, which adds cc, bcc and attachments to SendEmailSerializer and forwards them to cdp-api. It merges after this PR, so cdp-api accepts the files before Django sends them.

[posthog]

PostHog Review · Standard · reviewed 6f1c414: 1 Should fix.

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.

[posthog]

P2 · Bound attachment content before running the base64 regex

An attachment containing 4.5 million base64 characters makes BASE64_RE.test throw RangeError before the aggregate size check runs. This fits within the router's body limit, and postTransactionalEmail returns HTTP 500, encouraging retries of permanently invalid input. Guard the encoded length before executing the regex or use constant-stack validation.

[Silthus]

Fixed in 3bf8a16. Content longer than the base64 of 512 KB is refused with a 400 before the regex runs. The refusal test now sends a 4.5M-character file, which answered 500 before the fix.

[Silthus]

/trunk merge

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-11-05 08:34 UTC	Run
prod-us	✅ Deployed	2026-11-05 08:49 UTC	Run
prod-eu	✅ Deployed	2026-11-05 08:51 UTC	Run