feat(mail): couple the delivery lease to the sender's enforced timeout (HT-22)

[zaridan]

Jira: HT-22 — follow-up to #21's adversarial-review finding B.

Problem

HT-16 documented the invariant that the delivery lease (DEFAULT_LEASE_MS = 120_000, src/mail/send.ts) must strictly exceed the worst-case EmailSender.send() duration — otherwise a re-claimed retry can race a still-in-flight send into a concurrent double-send (specs/mail/sending.md §3a, §4). The Gmail adapter's 30 s AbortSignal.timeout sits safely under it today, but nothing tied the two constants together: a future adapter with no/looser timeout, or a raised Gmail timeoutMs, would violate the invariant silently.

Mechanical coupling

1. EmailSender requires readonly maxSendMs: number — the bound the implementation itself enforces on one send() call (a real mechanical timeout, not an estimate). Required, so TypeScript rejects any future adapter that doesn't declare a bound.
2. Gmail adapter declares maxSendMs: timeoutMs — the same variable that feeds AbortSignal.timeout, so the declared bound and the enforced one cannot drift apart inside the adapter. The existing abort test proves the timeout is real; a new test pins the declaration to it (default and custom).
3. assertLeaseExceedsSenderBound(sender, leaseMs) runs before every claimThreadForDelivery — in sendReply's keyed retry path and at the top of runDeliveryWorker. Any violating combination (maxSendMs >= leaseMs; equality is a violation, the invariant says strictly) throws up front, before anything is listed, claimed, or sent. A violation is a wiring bug, so it throws rather than returning a result — matching sendReply's "only throw on genuinely unexpected faults" contract.
4. Spec: sending.md §3a/§4 updated from "must by convention" to "enforced mechanically".

Rejected alternative: passing a lease-derived AbortSignal into send() — a larger contract change that guarantees nothing (an adapter can ignore the signal, and an aborted fetch may still be delivered server-side). The declared-bound + assert makes the invariant checkable exactly where lease and sender meet.

Tests

• Gmail adapter: maxSendMs equals the enforced timeout, default (30 000) and custom.
• Keyed sendReply with a violating sender throws before claim/send (both spies uncalled); equality pinned as a violation.
• No-key path regression-pinned as unaffected — it never claims a lease, so there is no invariant to violate there.
• runDeliveryWorker with a violating leaseMs throws before listDeliverableThreads is even called.

Verification

On this branch (post-#21 main, including ccc9be2's claim re-check): tsc exit 0, biome check clean, full vitest suite 331/331.

Noted, out of scope (in HT-22)

staleAfterMs (5 min default) has an analogous relationship to no-key sends, which never take a lease. With defaults it's transitively safe (maxSendMs < leaseMs < staleAfterMs); a separate ticket could assert maxSendMs < staleAfterMs if that window is ever tuned aggressively low.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added explicit email-sender timeout bounds to improve delivery reliability.
  • Delivery attempts now fail early when the configured lease is not longer than the sender’s maximum send duration.
  • Gmail sender settings now automatically report their configured timeout bound.

• Documentation

  • Clarified sender timeout and delivery lease requirements to help prevent duplicate or overlapping retries.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds an enforced EmailSender.maxSendMs contract, exposes the Gmail timeout through that property, and validates that delivery leases strictly exceed the sender bound before keyed claims or worker processing. Tests cover adapter values, keyed and unkeyed sends, worker startup, and sender fakes.

Changes

Lease-bound email delivery

Layer / File(s)	Summary
Sender timeout contract src/providers/email-sender.ts, specs/mail/sending.md	EmailSender.maxSendMs is documented as an enforced per-send settlement bound, and retry leases must strictly exceed it.
Gmail timeout declaration src/providers/adapters/gmail/sender.ts, src/providers/adapters/gmail/sender.test.ts	The Gmail sender sets maxSendMs from timeoutMs, with tests covering default and custom values.
Keyed retry validation src/mail/send.ts, src/mail/send.test.ts, src/api/index.test.ts	Keyed sendReply validates the sender bound before claiming; tests cover rejection, no-claim/no-send behavior, unkeyed sends, and updated fake senders.
Worker startup validation src/mail/delivery-worker.ts, src/mail/delivery-worker.test.ts	runDeliveryWorker validates lease configuration before listing or claiming threads, with tests covering invalid leases and worker sender configurations.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DeliveryWorker
  participant LeaseGuard
  participant DeliveryStore
  participant EmailSender
  DeliveryWorker->>LeaseGuard: validate leaseMs > sender.maxSendMs
  LeaseGuard-->>DeliveryWorker: allow or throw
  DeliveryWorker->>DeliveryStore: listDeliverableThreads
  DeliveryWorker->>DeliveryStore: claimThreadForDelivery
  DeliveryWorker->>EmailSender: send email

Loading

Possibly related PRs

• Helpthread/helpthread#15: Both modify sendReply result and delivery behavior.
• Helpthread/helpthread#17: Both modify the Gmail EmailSender adapter and timeout handling.

🚥 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 summarizes the main change: enforcing that the delivery lease is coupled to the sender's enforced timeout.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% 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 feat/ht-22-lease-timeout-coupling

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/providers/adapters/gmail/sender.ts (1)

121-150: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

maxSendMs still excludes token acquisition. getAccessToken() runs before AbortSignal.timeout(timeoutMs), so a slow refresh can make send() exceed the declared bound and let a retry race a still-in-flight send. Fold token acquisition into the same timeout budget, or maxSendMs no longer matches the worst-case send() time.

🤖 Prompt for 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.

In `@src/providers/adapters/gmail/sender.ts` around lines 121 - 150, Update
createGmailEmailSender’s send method so getAccessToken() is governed by the same
timeout budget as the Gmail request, ensuring the complete send operation cannot
exceed maxSendMs. Create and reuse a single timeout signal before token
acquisition, pass it to fetchImpl, and preserve the existing timeout behavior
for the HTTP exchange.

🤖 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.

Outside diff comments:
In `@src/providers/adapters/gmail/sender.ts`:
- Around line 121-150: Update createGmailEmailSender’s send method so
getAccessToken() is governed by the same timeout budget as the Gmail request,
ensuring the complete send operation cannot exceed maxSendMs. Create and reuse a
single timeout signal before token acquisition, pass it to fetchImpl, and
preserve the existing timeout behavior for the HTTP exchange.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2dc8c2e6-59fb-4850-8a78-4159518fb5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 063f0f5 and e322d92.

📒 Files selected for processing (9)

• specs/mail/sending.md
• src/api/index.test.ts
• src/mail/delivery-worker.test.ts
• src/mail/delivery-worker.ts
• src/mail/send.test.ts
• src/mail/send.ts
• src/providers/adapters/gmail/sender.test.ts
• src/providers/adapters/gmail/sender.ts
• src/providers/email-sender.ts