IR56M completed-email distribution can double-send (non-atomic claim, now reachable via PATCH)

Why (Accounting (Diagnostics), 2026-06-29)

Found while reviewing the Codex commit batch (owner asked to fix overlooked bugs). The "completed IR56M" email (with the signed PDF attached) can be sent more than once to the recipients when two send paths run concurrently.

Flagged, not yet fixed — the safe fix is architectural and this is outbound tax-notice email, so it is left for owner sign-off rather than patched blind.

Diagnosis

sendOneIR56MDistribution (lib/taxHK/ir56m/distribution/drain.server.ts) does a non-atomic read-check-then-act:

1. if (dist.status !== 'pending') return — checked against an already-read object, not a fresh/locked read.
2. send the email via sendIR56MCompletedEmail.
3. markDistributionSent(...) — the only status write, and it happens AFTER the email is already out.

There is no atomic claim between steps 1 and 3, so two overlapping runs both see pending, both send, then both mark sent → duplicate completed emails.

It is reachable three ways into the same function, which can overlap:

• POST /api/ir56m-signing/[requestId]/distribution → drainImmediatelyIfDue → inline send (on first sign).
• PATCH (edit settings) → drainImmediatelyIfDue → inline send. This is the newer path: editing a still-due/immediate distribution sends inline, so a PATCH racing the cron (or a second PATCH/POST) is a live double-send window.
• Cron drain → drainDueIR56MDistributions → same function.

listDuePendingDistributions selects status == 'pending', so until markDistributionSent lands, the cron keeps the doc eligible — widening the window against an inline PATCH/POST send.

Proposed fix (for owner approval)

Atomic claim before sending, with crash recovery so a tax notice is never dropped (the failure mode that matters more than an occasional duplicate):

1. Add a sending status to IR56MDistributionStatus.
2. New claimDistributionForSend(id) using runTransaction: read the doc; if status !== 'pending' return claimed: false; else set status = 'sending' + sendStartedAt = serverTimestamp(). Commit. Only the transaction winner proceeds to send.
3. On success → markDistributionSent (sending → sent). On total failure → release (sending → pending) so the next drain retries.
4. Reclaim: treat a sending doc older than a threshold (e.g. 15 min) as a crashed send and return it to pending (in listDuePendingDistributions or a dedicated sweep). Without this, a process crash between claim and send would strand the notice in sending forever.
5. Audit the other guards for the new state: updateDistributionSettings / cancelDistribution already gate on status === 'pending' (a sending doc correctly can't be edited/cancelled mid-send); confirm the IR56M status UI renders sending sensibly.

Tradeoff to confirm with the owner: the reclaim threshold trades a small duplicate-risk window (if a send takes longer than the threshold) against the drop-risk of a crashed send. A larger threshold favors "never duplicate"; a smaller one favors "never drop."

Verification

• ✅ Attestation (Accounting (Diagnostics)): read AGENTS.md; checked the board by scope, not UID — T-047 phase 4 tracks the distribution feature but no task covers the send-claim race. Tracking T-140.
• Source: Accounting (Diagnostics) · https://claude.ai/code/session_01G58Y71noihrYCDEDMexmea
• Diagnosis only; no code change in this task yet.

Blast Radius (of the proposed fix, when approved)

lib/taxHK/ir56m/distribution/{drain,store}.server.ts, the IR56MDistributionStatus type, and the IR56M distribution status UI. No change to recipient lists, grace-window math, or the email template — only the send-once guarantee and a crashed-send reclaim.

2026-07-09 — Checkup (owner asked "what's up with T-140?"): race still live, fix awaits go

• ✅ Attestation (Accounting (Diagnostics)): read AGENTS.md.
• Source: Accounting (Diagnostics) · https://claude.ai/code/session_01G58Y71noihrYCDEDMexmea
• Re-verified against current main: the non-atomic claim is unchanged — drain.server.ts still checks dist.status !== 'pending' on a stale read (line ~65) and markDistributionSent remains the only status write, AFTER the email is out (line ~77). No sending state exists. The only commit touching the module since diagnosis (45b56ec7, "preview signed pdf and drain immediate edits") is the PATCH-drain path — the very change that widened the overlap window. Diagnosis stands verbatim.
• Why it sat: flagged 2026-06-29 as "left for owner sign-off rather than patched blind" — outbound tax-notice email; the fix (atomic sending claim via runTransaction + crashed-send reclaim) changes send semantics, and the reclaim threshold is a duplicate-risk vs drop-risk tradeoff the owner should pick. It was never picked up because no go was given; status stayed todo by design, not neglect.
• Ready to build on approval: the 5-step fix in "Proposed fix" above, ~half-day including a race test (two concurrent drains → exactly one send). Default recommendation if the owner just says "go": 15-minute reclaim threshold (favors never-drop; the duplicate window then only exists for sends that stall >15 min, which the send path's timeouts make rare).

2026-07-09 — FIX BUILT (owner: "Fix T-140") — ships in PR #862, deploys in the same owner-ordered sequence

• ✅ Attestation (Accounting (Diagnostics)): read AGENTS.md.
• Source: Accounting (Diagnostics) · https://claude.ai/code/session_01G58Y71noihrYCDEDMexmea
• Built exactly the proposed fix, 15-minute reclaim threshold (the recommended never-drop bias; owner ordered the fix without overriding it):
• IR56MDistributionStatus gains sending + doc field sendStartedAt.
• claimDistributionForSend — runTransaction fresh-read; pending → sending with exactly one winner; returns the fresh row so the winner acts on current recipients/settings, not its possibly-stale copy. A sending claim older than IR56M_SEND_RECLAIM_MS (15 min) is treated as crashed and re-claimed — a tax notice can be duplicated only past that threshold, never dropped.
• releaseDistributionClaim — sending → pending on unsendable/total-failure/crash paths (transactional, no-op unless still sending).
• sendOneIR56MDistribution — claim-gated: claim → resolve context → email → markDistributionSent (clears sendStartedAt); releases on not-sendable (voided), all-recipients-failed, and unexpected-throw paths.
• listDuePendingDistributions — also surfaces due sending docs past the reclaim threshold so the cron un-strands crashed sends; fresh claims stay invisible.
• Guards audited: updateDistributionSettings/cancelDistribution already gate on pending, so a mid-send doc can't be edited or cancelled (previously that was a race); edit modal shows a "being sent right now" read-only note for sending.
• Tests (8 new, in distributionSendClaim.test.ts): competing claims → exactly one wins; non-pending refusals; reclaim after-not-before the threshold; release semantics; due-listing incl. crashed-send recovery; two overlapping sendOneIR56MDistribution calls → exactly one email and a sent doc; total-failure → back to pending; already-sent refused without emailing. Full suite 629/629; unfiltered tsc clean.

2026-07-09 (later) — DEPLOYED · fix live in production

• ✅ Read AGENTS.md · tracking T-140 (close-out addendum).
• Source: Accounting (Diagnostics) · https://claude.ai/code/session_01G58Y71noihrYCDEDMexmea
• Shipped in PR #862 → main @ 571cf21d; Vercel production deployment dpl_bnjh66DTjs63DgxHnkZwQmbcjchH READY on that sha — the send-once claim is live. The double-send window is closed in production.
• Commit SHAs (append-only): 571cf21d (PR #862 — fix + tests), plus this close-out docs commit. (Diagnosis-only entries predate the fix; no earlier work commits exist.)
• Blast radius: IR56M distribution send path only (lib/taxHK/ir56m/distribution/*, edit modal note). New doc field sendStartedAt; new status value sending — any future reader of ir56mDistributions must treat sending as in-flight, not terminal.