Skip to content

feat(cdp): add the upload route for transactional email files - #131833

Merged
trunk-io[bot] merged 218 commits into
feat/node-store-upload-auth-scopefrom
feat/node-store-upload-route
Nov 11, 2026
Merged

trunk-io[bot] merged 218 commits into
feat/node-store-upload-auth-scopefrom
feat/node-store-upload-route

Conversation

@Silthus

@Silthus Silthus commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Merge gate: passed. fleet proof gate passes on e7a3351 (live run, review at HEAD, CI green, judge fail accepted by standing rule). Layer 2 of 4 in the node-store attachment stack: #133482, this PR, #133495, then #131944. The stack merges from its top PR; #131567 is live in both regions and the env settings are merged.

Problem

  • A developer sending files with a transactional email needs a place to upload them before the send names them (PostHog/michaelr-context#77).
  • cdp-api has no route that hands out upload URLs.

Note

🟡 Merge risk: medium · two-way door

  • A new cdp-api route; every team gets field_not_accepted while the allowlist stays empty.
  • Switch-off: empty CDP_EMAIL_ATTACHMENTS_TEAMS; fix forward.

Changes

  • POST /api/projects/{team_id}/transactional_email/attachments/uploads answers each file present or upload with a signed PUT.
  • Order: scoped token with its own attachment_upload kind, allowlist, 1 to 10 files, the 40 MB message estimate, then the store.
  • A team's upload bytes per UTC day are counted in Redis when the route issues upload URLs, not when files are stored; over the cap it gets 429 upload_quota_exceeded. Present files and refused requests do not count.
  • A store or Redis outage answers 503 delivery_unavailable and logs a warning with the team and the error.
  • Every request lands on cdp_email_attachments_upload_requests_total by outcome, including unauthorized and not_configured, so a token mismatch with Django shows up.

How it works

---
title: Upload, then send
---
sequenceDiagram
    participant C as caller
    participant A as cdp-api
    participant B as bucket
    participant W as email worker
    rect rgba(9, 105, 218, 0.15)
    Note over C,B: upload #35;131833
    C->>A: sends the sha256 and size of each file
    A->>B: HEADs team/<id>/<sha256>
    A-->>C: answers present, or a PUT URL signed for that size and hash
    C->>B: PUTs the bytes
    end
    rect rgba(26, 127, 55, 0.15)
    Note over C,B: send #35;131944
    C->>A: sends, naming each file by its sha256
    A->>B: HEADs each file, copies any with under 7 days left
    A-->>C: answers 202 and queues a job without bytes
    end
    W->>B: GETs each file by sha256, at most 6 days later
Loading
  • This PR acts on cdp-api's upload route, AttachmentUploads.prepare, in the upload span of the flow above.
  • Before, cdp-api had no route that hands out upload URLs, so a send had no stored file to name.
  • Now, the caller sends each file's sha256 and size, and cdp-api answers present or a PUT URL signed for that size and hash.
  • The bucket key is the hash, and the bucket checks the hash on the PUT, so a send can name a file by hash alone.
  • A file counts as present only with a day and 15 minutes left, longer than the 15 minutes an upload URL lives.
  • One Redis script adds and checks a team's daily upload bytes, so concurrent requests cannot pass the cap.

How did you test this code?

  • The tests run outside-in: the route over real HTTP, through the internal-auth exemption from feat(cdp): let the attachment upload route through without the internal api secret #133482, against the dev bucket and Redis, including the cross-team case. Seven mutation checks each fail a test.
  • A cdp-api built from its settings answers uploads with the idempotency Redis as its counter, and 503 without it.
  • A burst of concurrent requests near the daily cap lets the one that fits through and refuses the rest.
  • Live run, real cdp-api over HTTP with the dev bucket and Redis: upload, present, another team, each refusal, the daily cap, and a 503 with its warning when object storage is unreachable. The request metric counts every outcome.
  • Not run: a production read of the stored files and quota counters. Nothing writes either before this stack deploys, and the allowlist stays empty after it, so the post-deploy watch takes the first read.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

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

Question Answer Who Where
Which token may ask for upload URLs? Only one with kind attachment_upload 🤖 Agent PostHog/michaelr-context#77
What if the upload counter cannot be read? 503 delivery_unavailable, so the cap holds 🤖 Agent PostHog/michaelr-context#77
Deduplicate a charge replayed on reconnect? No; a rare replay over-charges once and fails toward the cap 🤖 Agent PostHog/michaelr-context#77
Bound how many uploads a team may prepare at once? No; the store calls carry timeouts, the daily cap holds, and the Django route adds the team's throttles 🤖 Agent PostHog/michaelr-context#77
Size a send by the declared or the stored file size? The stored size, read from the store at send time 🤖 Agent PostHog/michaelr-context#77
Read production storage before merge? No; nothing writes it until this stack deploys, so the post-deploy watch reads it first 🤖 Agent PostHog/michaelr-context#77

Involvement: 8 cross-model review rounds and 2 final reads (GPT-6.1 Sol panel, Opus fresh eyes), one full qa-swarm plus the stack-tip pass, PostHog Review on each head (no findings on the final head); live run captured. CodeRabbit is retired and skipped.

More context for AI agents

🤖 Generated with Claude Code

Silthus and others added 27 commits November 9, 2026 09:50
The store signs upload URLs for a file's key, size and SHA-256, accepts
files for a send, keeps them past the bucket's 37-day expiry with a
self-copy, and loads them for the worker. Nothing calls it yet.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A use that needs a file past what a self-copy can keep is refused instead
of accepted. Signing refuses credentials that would expire before the URL
even after a refresh, and gives up after the store's timeout. Refs
PostHog/michaelr-context#77.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each bucket call now ends when its signal aborts, even while the client
still waits for credentials. The retention limit applies only when a use
needs a copy, so a fresh upload keeps working through its expiry. Refs
PostHog/michaelr-context#77.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs PostHog/michaelr-context#77.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A send that lists files loads them from the attachment store and passes
them to SES as attachments. The load and the SES call share one abort
deadline. A load that fails or runs past the deadline is rescheduled up
to 10 times, then fails as attachment_unavailable. Sent and failed
engagement events carry the file count and bytes.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The SES client resolves its credentials before it checks the abort
signal, so a stalled refresh held a send with files past its deadline.
The call now settles at the deadline, as the attachment store's calls do.

A workflow's next email action no longer inherits the attachment load
retries of the email before it.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A send whose files fail to load pays the workflow and team sending limits
before the load. Its retry now carries a mark that the limits are paid, so
it does not spend them again, and the mark stays off the next email action.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the team's daily budget carries across attachment load retries.
The per-workflow pacing limits a rate, so it applies to every try:
retries that waited out a storage outage must not send all at once
when the store recovers.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- defer() hands a job back for later without touching its poison count or
  retry budget; an email job takes a fresh dequeue_seq, so it goes to the
  back of its team's turn.
- A job keeps its heartbeat until its release write settles.
- Stopping the job queue stops dequeuing, waits for the result writes
  (bounded), then closes the pool.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- One 25-second deadline, counted from the start of the stop, covers the
  batch in progress, the result writes and closing the pool. A pool close
  that waits on a blocked query no longer holds the stop.
- Each result write is tracked on its own, so one failed write no longer
  ends the drain while its siblings still write.
- A finished write removes only its own claim, not a newer claim of the
  same job.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A send whose files fail to load now carries the paid-budget mark only
when the team budget was charged on that try. A try that skipped the
charge, such as one whose tier lookup failed, leaves the retry to charge
it.

Refs PostHog/michaelr-context#77

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

A send with files leaves its batch once an in-flight registry admits it by
a memory budget sized from the pod, a heap check and a cap. A send without
room is deferred with a growing, jittered delay. Deferrals keep the poison
count, spend no retry, and show on a token metric and on the lateness
histogram as deferred.

Ticket: PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Track each storage call from its start, so a sibling failure keeps the bytes reserved.
- Release the reservation only once every call settled, with no timeout.
- Write the result after at most 3 s of waiting on abandoned calls, inside the stop deadline.
- Handle a failing send while the plain batch runs, and wait for sends before a plain-batch error.
- Run canceled jobs with files on the plain path, so a refusing worker ends them.

PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A send's promise settles once its result is written; its reservation is released in the background once every
  storage call settled, so a failed plain batch never waits on a stalled call.
- Test the canceled send with files through the real worker: it ends canceled instead of deferring.

PostHog/michaelr-context#77

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

- A deferred job heartbeats until its deferral write settles, as an admitted send does until its result write.
- A tracked storage call leaves the send's set once it settles, so its buffer is not held to the end of the send.
- The reservation covers email text outside Latin-1, which makes the SES request body two bytes per character.

PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A send naming a missing file is refused before any present file is copied.
- Each reference travels with the stored file it names, rather than a parallel array of sizes.

PostHog/michaelr-context#77

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

One failed write no longer settles the batch while its sibling writes still run, so the batch keeps heartbeating
those jobs until each write lands.

PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deferral writes share the result writes' wait, so one failed deferral no longer stops the heartbeat of the others.

PostHog/michaelr-context#77

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

# Conflicts:
#	nodejs/src/cdp/services/hogflows/hogflow-executor.service.test.ts
#	nodejs/src/cdp/services/hogflows/hogflow-executor.service.ts
#	nodejs/src/cdp/services/messaging/email.service.ts
cdp-api answers each requested file present or upload, behind the
CDP_EMAIL_ATTACHMENTS_TEAMS allowlist, which stays empty. The files must
fit the 40 MB message estimate, and a team's upload bytes per UTC day are
capped in Redis.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus added this pull request to stack #131663 November 9, 2026 13:29
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

🤖 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 — 0% of added code lines are comments (0 of 453)

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.

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

Silthus and others added 9 commits November 11, 2026 03:19
…e-in-flight-registry

The email service and email worker tests now send through master's SES
fake. The shared email delivery fakes keep the bucket proxy and the
upload helpers, and read deliveries from that fake.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A deferral writes the job state back without the persistence sanitizer.
The test runs it with the integration ref writer on and off, and checks
that the files, the queue metadata and the stored integration inputs
come back as the queue stored them.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-upload-route

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…worker's resolver

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-in-flight-registry

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-upload-route

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from feat/node-store-in-flight-registry to master November 11, 2026 05:46
@trunk-io

trunk-io Bot commented Nov 11, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

Refs PostHog/michaelr-context#77

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

Refs PostHog/michaelr-context#77

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

Refs PostHog/michaelr-context#77

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

trunk-io Bot commented Nov 11, 2026

Copy link
Copy Markdown

This pull request is queued for merge as part of 131944, which will merge 133482, 131833, 133495, 131944.

@trunk-io
trunk-io Bot merged commit 3cdc82e into master Nov 11, 2026
208 checks passed
@trunk-io
trunk-io Bot deleted the feat/node-store-upload-route branch November 11, 2026 15:57
@trunk-io

trunk-io Bot commented Nov 11, 2026

Copy link
Copy Markdown

This pull request was merged into master as part of stacked PR 131944.

@deployment-status-posthog

deployment-status-posthog Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-11 16:27 UTC Run
prod-us ✅ Deployed 2026-11-11 18:14 UTC Run
prod-eu ✅ Deployed 2026-11-11 17:23 UTC Run

This table shows the posthog release only. Hogstation shows every service that this PR reaches, Kargo services included.

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