Repository navigation
refactor(cdp): encode and decode cyclotron jobs in one codec - #128868
Merged
Merged
Conversation
…il route A code send posts its accepted content to transactional_email/send. The route runs the same reserve, replay, refusal and enqueue steps as a template send. The accepted send binds the idempotency fingerprint, and a miss on a new workflow reads it from the primary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…flow Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ode-send Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ode-send Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A send with captureEngagement false still put the recipient's distinct_id in its SES header and pixel/redirect tracking codes. Minting still emits the v2 payload, which has no capture choice, so the SES delivery, bounce, open and click callbacks for that send captured engagement events. Every reader skips engagement capture for a code without a distinct_id, so leaving it out honors the choice on every deployed reader now. The v3 payload and its minting switch stay as they are. The send-time capture and the message assets row keep their distinct_id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The capture-off fix also changed the pixel and redirect codes that dev and test mint, but only the SES header path had a test. The tracking URL test now runs per capture choice and fails if those codes keep the distinct_id of a capture-off send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A capture-off send already mints its v2 codes without a distinct_id, and every reader skips engagement capture without one. The v3 capture-choice payload was never minted, so remove its marker, parser branch, rollout constant and the captureEngagement fields that only fed it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A queued job for a deleted workflow read the primary on every batch, one writer query per missing id. Remember ids the primary confirmed missing for 60 seconds (bounded to 10,000 ids, cleared by a reload), and read a batch's replica misses from the primary in one query while keeping the in-flight dedupe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A workflow the replica did not have yet was read from both databases on every send and worker batch until replication caught up. Keep what the primary returned, hit or miss, for 60 seconds (bounded to 10,000 ids, cleared by a reload naming the id), so later lookups read neither database; after expiry the replica is read first again. The send-route and worker tests now hide the row from the replica at the query boundary, so they cover a replica miss followed by a primary hit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A primary read that started before a reload could resolve after it and store the old state for the next 60 seconds, so a workflow disabled or paused during that read kept sending. A reload now drops the in-flight read, and a read that a reload overtook returns its rows to its own callers but stores nothing. 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>
# Conflicts: # nodejs/src/cdp/services/hogflows/actions/hog_function.ts
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A newer Django can send a field this cdp-api does not deliver, such as cc, bcc or attachments. Before this, the field was dropped, the send went out without it, and the caller saw a 202. Now the send answers 400 invalid_request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and tracking The outer send object refused unknown fields, but content, from and tracking still dropped theirs, so a newer caller could get a 202 for an email sent without them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The PR's code carries no comments; the schema's .strict() and its tests say why. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both transactional email routes verified the same audience with the same team and workflow claims, so a token Django minted for a stored-template send also worked on /transactional_email/send, where it could carry any content. The code send route now also requires the claim kind=code_send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-send Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sendEmail hand-off reads the run's accepted send from its state and lays the content, sender and reply-to over the step, typed as the email queue parameters. The workflow step no longer threads a send through the generic hog function builder, and email tracking reads the engagement choice from state.send instead of the hog function metadata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A rerun skips a stored run that carries send unless the rerun request sets replay_code_sends, which Django sets from workflows-email-from-code. This covers code sends routed through an existing template's workflow, which an origin check on the workflow misses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sent and failed events now take their distinct id from resolveEmailTrackingDistinctId, the resolver the tracking code already uses, so the separate capture-choice helper goes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tate The flow's state.send now lays over the step's hog function state every time it is built, not only when the state is new. A step saved without send, such as one an older worker started in a mixed rollout, captured engagement for a send that turned capture off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Zod's default email check refused valid addresses such as jane=tag@example.com, so a send to them failed as recipient_unreachable. The recipient check now uses the HTML5 address pattern with a dotted domain, the same pattern Django applies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The recipient pattern let a dot lead, trail or repeat in the local part, so .jane@example.com reached the provider instead of a 422. Dots now only separate the characters, as Django's pattern does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # nodejs/src/cdp/services/hogflows/hogflow-executor.service.test.ts
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The code-key and capture tests sent real email through the shared maildev, which races email.service.test.ts clearing the same inbox when both files run in one CI shard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The codec test compared the new encoding byte for byte with what the earlier adapter wrote, so a change of key order or escaping that both versions still read would fail it. It now compares the parsed JSON, and decodes both the new encoding and the earlier one into the same invocation. 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>
There was a problem hiding this comment.
Approved.
The change touches persisted job processing, with assurance supplied by owning-team authorship and added compatibility tests. The listed formal reviews are on older commits; no unresolved substantive concerns or other refusal grounds were found.
- Author wrote 18% of the modified lines and has 17 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 267L, 3F substantive, 390L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (390L, 5F, single-area, refactor) |
| stamphog 2.8.0 | .stamphog/policy.yml @ 6c01b6f · reviewed head 6c01b6f |
|
This pull request was merged into |
Deploy status
This table shows the |
This branch was successfully deployed
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
Note
🔵 Merge risk: low · two-way door (merge-risk rubric)
Landing: #129915 merges alone and its migration runs in US and EU first. #127745 merges next. Then one
/trunk mergeon #128929 lands N1 to N4.Changes
flowchart LR subgraph Before J1[janitor] --> A1[queue adapter: encode, decode, metrics] W1[worker loop] --> A1 end subgraph After J2[janitor] --> C2[job codec: encode, decode] A2[queue adapter: metrics] --> C2 W2[worker loop] --> A2 end classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class C2 phBlue; class J1,A1,W1,J2,A2,W2 phGray;How did you test this code?
Test rationale:
job-codec.test.tsfails when the stored bytes or the decoded invocation change.rerun-paginator.service.test.ts(no ClickHouse test database).Local runs
tsc --noEmit.stateparses, so every job takes the decode's parse branch. No stored row has a null or malformedstate, a cancel request, or a janitor touch today. The golden test and the shadow diff cover those branches with invented rows.Release status
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5
Skills:
/writing-tests,/writing-pr-descriptionsmasterafter it mergeddecodeJobStatereturnsv2JobToInvocationinputInvolvement: one design pass, one review round (7 lenses).
More context for AI agents
extract*helpers so its test suite stays unchanged.nodejs/src/cdp/services/cyclotron-v2/job-codec.tsagainst the removed adapter code.