fix(mail): carry reply token in References + suppress self-echo (HT-49)

[zaridan]

Summary

Live production evidence (2026-07-17, first HT-44 run against real Gmail): Gmail's users.messages.send accepted the engine's verbatim Message-ID but replaced it on the wire with a Gmail-generated id. The customer's reply then carried In-Reply-To/References pointing at Gmail's id, with our signed reply token nowhere on the wire — so decideThreading correctly found no verified token and forked a new conversation instead of appending. This is the failure this PR fixes, proven against real Gmail, not simulated.

Fix (commit 1): sendReply now appends its own freshly-minted messageId as the FINAL entry of the outbound References chain, after any ancestor ids. Gmail does not rewrite References, and an RFC-5322-compliant reply's own References is built as {original References} + {original Message-ID} — so the token rides into the customer's reply one position before whatever foreign id the provider substituted, exactly where decideThreading's existing newest-first scan (src/mail/thread.ts, unmodified) finds it. In-Reply-To is untouched.

Review fix (commit 2): putting the token in every outbound reply's References has a consequence: Gmail also delivers the SENT message's own copy back into the mailbox it was sent from (the "self-echo" the reconcile pipeline ingests like any other message), and that self-echo now carries the token too. isOwnMessageReflection's existing loop guard only checks the message's OWN Message-ID, which Gmail rewrites — so the guard never fires, decideThreading finds the token in References, and the agent's own reply gets appended a second time as a phantom inbound message, reopening a closed conversation. Fixed one layer earlier, in the delivery ledger: sendReply captures EmailSendResult.providerMessageId (the same id reconcile later reports for this exact message) and, via an optional SelfEchoGuardDeps, pre-seeds (mailboxId, providerMessageId) as an already-suppressed row in the inbound delivery ledger right after a successful send. Reconcile's existing "terminal row, do not double-process" branch absorbs it — no change to decideThreading, no heuristic on message content, and the customer-autoresponder case (which legitimately carries our token in References too) is untouched because the correlation is providerMessageId, not the token.

Design decisions

• selfEchoGuard is optional and wired unconditionally only in the composition root (src/composition/root.ts), since every deployment that root builds is Gmail-backed. Absent everywhere else (API deps, delivery-worker deps) by default — a deployment with no self-reflecting transport behaves exactly as before this guard existed. Flagging for sign-off: confirm this is the right default posture if/when a non-Gmail transport is added — the guard would need explicit wiring there too, and nothing currently forces that.
• Known residual race, conceded rather than corrected: the ledger pre-seed happens after the send resolves. If reconcile's own claim() for the same provider id wins that race first (an unusually fast push-triggered reconcile), the pre-seed is a no-op (never overwrites an existing row) and the message ingests normally — reproducing the pre-HT-49 failure mode for that one send, not a new one. Documented in specs/mail/inbound-ingestion.md §5. Flagging in case the maintainer wants a stronger guarantee here rather than accepting the race.
• Correlation deliberately uses providerMessageId, not an extension of the Message-ID/References scan — chosen specifically so it can't misfire on a customer's legitimate autoresponder reply, which carries our token in the same References position.

Review — 5 adversarial findings (3 actionable), fixes applied

1. (actionable, fixed) Self-echo re-ingestion: outbound References token, once added, would cause Gmail's self-echo of the agent's own sent reply to be re-appended as a phantom inbound message, reopening closed conversations. Fixed via preSuppressOwnSend + SelfEchoGuardDeps (commit 2, full mechanism above).
2. (actionable, fixed) delivery-worker.ts's retry path (attemptDeliveryOfClaimedThread) shares the exact same self-echo exposure as sendReply's direct path but wasn't wired to the guard. Fixed: DeliveryWorkerDeps.selfEchoGuard threaded through runDeliveryWorker → attemptDeliveryOfClaimedThread.
3. (actionable, fixed) Formatting-only lint findings in send.test.ts (see Verification) — auto-fixed, not a logic change.
4. (non-actionable) Considered extending isOwnMessageReflection's Message-ID correlation to also scan References for self-echo detection — rejected because it would also misfire on a customer's legitimate autoresponder reply, which carries our token in the same position for a legitimate reason.
5. (non-actionable) Considered making the ledger pre-seed atomic with the send itself to close the residual race entirely — rejected as out of scope for this fix; documented as a known, conceded residual instead (see Design decisions).

Verification

Independent gate exit codes — ran for real, in this worktree, on this code:

• npm run typecheck — exit 0, clean, no errors.
• npm run lint (biome check .) — clean after one auto-fix pass (npx biome check --write on send.test.ts, formatting only); re-ran lint after, exit 0, clean.
• Targeted vitest run (src/mail/send.test.ts, src/api/index.ts-adjacent, src/mail/ingest.test.ts, src/store/inbound-deliveries.test.ts) — exit 0, all passing, including the new self-echo suppression fixture and the preSuppressOwnSend race-concession case.
• git status — clean tree, nothing uncommitted.

Link: https://resonantiq.atlassian.net/browse/HT-49

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved email threading for providers that rewrite Message-ID by ensuring the reply’s minted message id is always the final References entry.
  • Prevented self-echoed sent messages from being re-ingested as phantom inbound messages via an optional self-echo suppression guard.
  • Strengthened handling for edge cases like missing provider ids and concurrent delivery/claim races.

• Documentation

  • Updated Agent Inbox and mail sending/threading/ingestion specifications to reflect the revised References and self-echo behavior.

• Tests

  • Added regression coverage for HT-49 threading and suppression scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4eaf58c7-ed33-471b-b7a4-e7e0feb1d6e3

📥 Commits

Reviewing files that changed from the base of the PR and between 27caca7 and 92f18d1.

📒 Files selected for processing (3)

• specs/api/agent-inbox-v1.md
• specs/mail/sending.md
• src/api/index.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• src/api/index.ts
• specs/api/agent-inbox-v1.md
• specs/mail/sending.md

---

📝 Walkthrough

Walkthrough

HT-49 updates reply References generation to append the minted message ID, adds optional provider self-echo suppression through the inbound delivery ledger, wires the guard through API and delivery paths, and adds specification, MIME, send, ingest, and storage regression coverage.

Changes

Reply threading and self-echo suppression

Layer / File(s)	Summary
Threading and delivery contracts specs/api/agent-inbox-v1.md, specs/mail/*.md	Specifications define the minted reply ID as the final References entry and document optional self-echo suppression when providers rewrite Message-ID.
Pre-seeded delivery suppression src/store/inbound-deliveries.ts, src/store/inbound-deliveries.test.ts, src/mail/ingest.test.ts	The delivery ledger adds idempotent preSuppressOwnSend; ingestion tests cover suppressed self-echoes, races, and rewritten provider IDs.
Reply metadata and send suppression src/mail/send.ts, src/providers/email-sender.ts, src/providers/adapters/gmail/mime.test.ts, src/mail/send.test.ts	sendReply appends and persists the minted reply ID, while successful sends optionally pre-suppress returned provider message IDs and validate both paths.
API and worker dependency wiring src/api/*.ts, src/composition/root.ts, src/mail/delivery-worker.ts, src/api/index.test.ts	The optional selfEchoGuard dependency is exposed, constructed, forwarded, and reflected in API reply expectations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AgentInboxAPI
  participant sendReply
  participant EmailSender
  participant InboundDeliveryStore
  participant ingestInboundMessage
  AgentInboxAPI->>sendReply: submit reply with ancestor References
  sendReply->>EmailSender: send with minted ID as final References entry
  EmailSender-->>sendReply: providerMessageId
  sendReply->>InboundDeliveryStore: preSuppressOwnSend(mailboxId, providerMessageId)
  ingestInboundMessage->>InboundDeliveryStore: claim echoed provider message
  InboundDeliveryStore-->>ingestInboundMessage: suppressed outcome

Loading

Possibly related PRs

• Helpthread/helpthread#5: Defines the mail-threading rules extended by the provider Message-ID rewrite handling.
• Helpthread/helpthread#12: Established the outbound reply path extended with minted-token References handling.
• Helpthread/helpthread#38: Established the inbound delivery-ledger pipeline extended with pre-seeded suppression.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main changes: reply token propagation in References and self-echo suppression for HT-49.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ht-49-references-reply-token

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/api/agent-inbox-v1.md`:
- Around line 400-407: Revise the v1.1 description of InboxApiDeps.selfEchoGuard
to characterize suppression as best-effort rather than guaranteed. State that it
depends on a reported provider ID and mailbox match, and that reconciliation may
win the race as documented in inbound-ingestion.md §5; preserve the
absent-by-default and unchanged-deployment behavior.

In `@specs/mail/sending.md`:
- Around line 185-190: Update the “transmit References verbatim” contract in the
sendReply documentation to qualify that safe message-ID atoms preserve their
original order and values, while adapters may sanitize unsafe attacker-derived
ancestor IDs. Explicitly require the engine-minted final messageId token to
remain intact and preserve the existing Gmail MIME adapter security behavior.

In `@src/api/index.ts`:
- Around line 151-161: Update the documentation comment for selfEchoGuard in
sendReply to replace the absolute “never” claim with wording that normally
suppresses the sent-message echo while acknowledging the narrow race where
reconciliation claims the message before preSuppressOwnSend. Keep the existing
behavior and references to the self-echo guard unchanged.

In `@src/mail/send.ts`:
- Around line 103-171: Update the successful-send persistence flow around
setThreadDeliveryStatus and releaseThreadLease to durably mark the delivery as
completed using the provider receipt before any failure can leave it retryable.
Ensure persistence or lease-release errors cannot allow the delivery worker or
keyed replay to reclaim and resend an already-accepted message, while preserving
the original successful send outcome.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ef4b5c23-e276-4d3f-82d4-49eafbc4197d

📥 Commits

Reviewing files that changed from the base of the PR and between e5c9ccd and 27caca7.

📒 Files selected for processing (16)

• specs/api/agent-inbox-v1.md
• specs/mail/inbound-ingestion.md
• specs/mail/sending.md
• specs/mail/threading.md
• src/api/conversations.ts
• src/api/index.test.ts
• src/api/index.ts
• src/composition/root.ts
• src/mail/delivery-worker.ts
• src/mail/ingest.test.ts
• src/mail/send.test.ts
• src/mail/send.ts
• src/providers/adapters/gmail/mime.test.ts
• src/providers/email-sender.ts
• src/store/inbound-deliveries.test.ts
• src/store/inbound-deliveries.ts

[zaridan]

CodeRabbit review round resolved in 92f18d1 (docs/spec-only; no code behavior changed). Per-finding disposition:

1. selfEchoGuard best-effort wording (Minor, specs/api/agent-inbox-v1.md) — Fixed. The changelog entry now conditions suppression on the sender reporting a provider message id for a resolvable outbound mailbox, calls the pre-seed best-effort, and states that reconcile can win the documented race, pointing at inbound-ingestion.md §5's "Known residual."

2. "Transmit References verbatim" contract (Major, specs/mail/sending.md) — Fixed. The HT-49 paragraph now states the actual adapter contract the shipped Gmail adapter implements: every transmitted atom goes out verbatim and in order (never rewritten/reordered/substituted), but adapters MAY sanitize by dropping an unsafe attacker-derived ancestor atom (isSafeMsgId, src/providers/adapters/gmail/mime.ts — the header-injection/DoS defense), while the engine-minted final token — which passes any such filter by construction (reply-token.ts's bounded [A-Za-z0-9_-]/./@ charset) — MUST reach the wire intact.

3. "Never" overclaim (Major, src/api/index.ts) — Fixed. Verified the actual interleaving in code: preSuppressOwnSend is INSERT … ON CONFLICT DO NOTHING and never overwrites an existing row, so when a fast reconcile's claim() wins the race, the echo ingests normally and is stored as a visible phantom direction: 'inbound' message in its own conversation — the pre-guard failure mode for that single send, nothing new. No downstream filtering of such messages exists on this branch. The doc comment now says exactly that instead of "never," with a pointer to inbound-ingestion.md §5's conceded race.

4. Resend after send-succeeds-but-mark-fails (Major, src/mail/send.ts) — Rebutted, no change (full reasoning in the inline reply on that thread). Summary: the proposed "durable success marker" is the very write that failed — releaseThreadLease/setThreadDeliveryStatus('sent') is written immediately on provider receipt, under the still-held lease; any substitute marker to the same store fails identically, and no ordering closes the window. Blocking retries instead would trade "occasionally delivered twice, byte-identical Message-ID" for "sometimes never delivered." This is HT-16's explicitly specced at-least-once contract (sending.md §3a) with its mitigations (envelope snapshot, stable Message-ID as idempotency anchor, provider-side dedup recommendation) already in place and fixture-pinned in send.test.ts — and the machinery predates this PR, which only appended the suppressSelfEcho call after the existing marks.

Verification: npm run typecheck exit 0, npm run lint exit 0, npx vitest run src/api/index.test.ts 104/104 passed (the only touched code file; the other two changes are spec text).

🤖 Generated with Claude Code