Helpthread · zaridan · Jul 10, 2026 · Jul 10, 2026 · Jul 10, 2026
@@ -0,0 +1,103 @@
+# Outbound sending & the reply-token lifecycle
+
+Status: accepted (HT-15). Companion to [threading.md](./threading.md) — that spec
+decides which conversation an *inbound* message joins; this one covers how an
+*outbound* reply is minted, persisted, and sent, and is where the threading
+model's authority actually originates.
+
+## 1. Why sending is the load-bearing step
+
+Threading is *outbound-anchored* (threading.md §2): an inbound reply is threaded
+**only** on a signed reply token the engine minted into one of its own outbound
+`Message-ID`s. Nothing about inbound `In-Reply-To`/`References` is trusted on its
+own. That means every outbound message is a promise: the token it carries is the
+sole future handle on this conversation. If sending mints a token that doesn't
+match what's stored, or stores a token for a message that never went out, the
+thread breaks. So sending is held to the same "correctness outranks velocity"
+bar as the threading decision itself (CHARTER.md invariant #3).
+
+## 2. The id/token knot, and its resolution
+
+The outbound `Message-ID` must embed a token over `{conversationId, threadId}`
+(threading.md §2). But a thread's `threadId` is its storage primary key, and the
+`Message-ID` is a column stored on that same row — so the id must exist *before*
+the row is inserted. The database generating the id at insert time is circular.
+
+**Resolution (option A):** the application generates the outbound thread's UUID
+(`crypto.randomUUID()` — a CSPRNG) *before* persistence, mints the token from it,
+and inserts the row with `id` **and** `message_id` set together in one write.
+
+- **`threadId` in the token identifies the outbound thread that carries it** —
+  the specific outbound message. A later verified inbound reply therefore names
+  the exact message it is answering (useful for lineage, audit, and future
+  per-thread routing), even though `decideThreading` today routes on
+  `conversationId` alone.
+- **App-generated ids are safe in the HMAC.** The token's integrity is the
+  signature, never the unguessability of the id (threading.md §2). A v4 UUID is
+  a perfectly good identifier here; a DB-generated one would be no safer.
+- **UUIDs are token-safe.** `reply-token.ts`'s id charset is `[A-Za-z0-9_-]`,
+  which admits UUID hex-and-hyphens; UUIDs contain no `.`/`@`, the token's
+  structural delimiters. So a real store UUID mints and verifies unchanged.
+
+## 3. Outbound threads are outbox items
+
+An outbound thread carries an explicit **delivery status**: `pending`, `sent`,
+or `failed`. (Inbound threads have no delivery status — the column is `NULL`
+for them.) This makes "persisted" and "delivered" distinct facts, which is what
+keeps a mid-flight failure from lying.
+
+**Ordering — persist, then send, then mark:**
+
+1. Generate `threadId`; mint the token → `messageId`.
+2. Persist the outbound thread with `delivery_status = 'pending'` and
+   `message_id = messageId`.
+3. Call the sender provider (§4).
+4. On success → `delivery_status = 'sent'`; on failure → `'failed'`.
+
+A crash at any point leaves a truthful record: a thread stuck at `pending` means
+"we may or may not have delivered it," never a false `sent`. Send-*then*-persist
+is rejected — a crash after a successful send would lose the outbound message
+from the conversation entirely.
+
+**Retries reuse, never re-mint.** A `failed` (or orphaned `pending`) outbound
+thread is re-attempted with the **same** `threadId` and the **same**
+`Message-ID`. Minting a fresh token per attempt would spray multiple valid
+threading handles for one logical message and risk double-sends. The stable
+`Message-ID` is the idempotency anchor: a provider that de-dupes on `Message-ID`
+will not double-deliver a retried send.
+
+## 4. What a sender provider must guarantee
+
+The `EmailSender` provider (`src/providers/`) is handed a fully-formed outbound
+message and MUST transmit the engine-supplied `Message-ID` **verbatim** as the
+RFC 5322 `Message-ID` header — not generate or overwrite its own. Threading
+depends on it; a provider that cannot set `Message-ID` is unusable for
+Helpthread. `In-Reply-To` and `References` are likewise engine-set and must be
+transmitted as given.
+
+The interface can only state this contract; it cannot enforce it. Therefore
+**every real `EmailSender` adapter MUST ship with a wire-level contract test**
+asserting the exact `Message-ID`/`In-Reply-To`/`References` it emits (against
+the raw MIME or provider-API payload it produces), because an adapter whose SDK
+silently rewrites `Message-ID` would pass `sendReply` (the thread is marked
+`sent`) while every future reply fails to thread. Prefer provider APIs that
+accept raw MIME; reject any that will not carry `Message-ID` unaltered. The
+in-repo fake used by the engine tests proves only that `sendReply` *passes* the
+value to the seam — not that any given adapter preserves it on the wire.
+
+## 5. Scope of the first increment (HT-15)
+
+Deliberately narrow; each deferral below has a named later home:
+
+- **Synchronous send only** — the persist→send→mark flow runs inline. No queue
+  or retry worker yet; the `failed` status plus the stable id/`Message-ID` are
+  the seam a later delivery worker (queue provider, already interfaced) picks up.
+- **Reply to an existing conversation only.** Agent-*initiated* brand-new
+  conversations are a separate later flow.
+- **`In-Reply-To`/`References` are caller-supplied** (from the inbound message
+  being answered). Deriving the full `References` chain from stored threads is a
+  later refinement.
+- **A missing or deleted conversation is refused** — the token is minted first
+  (before `appendThread` resolves) and then discarded on refusal; only
+  persistence and sending are skipped, and the sender is never called (mirrors
+  the store's `appendThread` policy; threading.md §5).
@@ -32,14 +32,17 @@ describe('migrate', () => {
     expect(thread.id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/)
   })
 
-  it('records exactly one _migrations row for migration 001', async () => {
+  it('records exactly one _migrations row per migration', async () => {
     db = await createPgliteDb()
     await migrate(db)
 
     const rows = await db.query<{ id: number; name: string }>(
       'SELECT id, name FROM _migrations ORDER BY id',
     )
-    expect(rows).toEqual([{ id: 1, name: 'conversations_and_threads' }])
+    expect(rows).toEqual([
+      { id: 1, name: 'conversations_and_threads' },
+      { id: 2, name: 'add_thread_delivery_status' },
+    ])
   })
 
   it('is idempotent: a second call is a clean no-op', async () => {
@@ -48,6 +51,95 @@ describe('migrate', () => {
     await migrate(db) // must not throw (e.g. "relation already exists")
 
     const rows = await db.query<{ id: number }>('SELECT id FROM _migrations ORDER BY id')
-    expect(rows).toEqual([{ id: 1 }])
+    expect(rows).toEqual([{ id: 1 }, { id: 2 }])
+  })
+
+  it('migration 002 ties delivery_status to direction: inbound must be NULL, outbound must be pending/sent/failed', async () => {
+    db = await createPgliteDb()
+    await migrate(db)
+
+    const [conversation] = await db.query<{ id: string }>(
+      'INSERT INTO conversations (customer_email) VALUES ($1) RETURNING id',
+      ['customer@example.test'],
+    )
+
+    // Inbound → NULL is the only legal value.
+    const [nullRow] = await db.query<{ delivery_status: string | null }>(
+      `INSERT INTO threads (conversation_id, direction, from_address)
+       VALUES ($1, 'inbound', $2) RETURNING delivery_status`,
+      [conversation.id, 'customer@example.test'],
+    )
+    expect(nullRow.delivery_status).toBeNull()
+
+    // Outbound → one of the three outbox states.
+    const [pendingRow] = await db.query<{ delivery_status: string | null }>(
+      `INSERT INTO threads (conversation_id, direction, from_address, delivery_status)
+       VALUES ($1, 'outbound', $2, 'pending') RETURNING delivery_status`,
+      [conversation.id, 'support@example.test'],
+    )
+    expect(pendingRow.delivery_status).toBe('pending')
+
+    // Outbound with an out-of-domain value → rejected.
+    await expect(
+      db.query(
+        `INSERT INTO threads (conversation_id, direction, from_address, delivery_status)
+         VALUES ($1, 'outbound', $2, 'bogus')`,
+        [conversation.id, 'support@example.test'],
+      ),
+    ).rejects.toThrow()
+
+    // Cross-column invariant: an INBOUND thread may NOT carry a status...
+    await expect(
+      db.query(
+        `INSERT INTO threads (conversation_id, direction, from_address, delivery_status)
+         VALUES ($1, 'inbound', $2, 'sent')`,
+        [conversation.id, 'customer@example.test'],
+      ),
+    ).rejects.toThrow()
+
+    // ...and an OUTBOUND thread may NOT be left NULL (invisible to a delivery worker).
+    await expect(
+      db.query(
+        `INSERT INTO threads (conversation_id, direction, from_address, delivery_status)
+         VALUES ($1, 'outbound', $2, NULL)`,
+        [conversation.id, 'support@example.test'],
+      ),
+    ).rejects.toThrow()
+  })
+
+  it('migration 002 upgrades a NON-fresh 001 database with preexisting outbound rows (backfills, does not fail)', async () => {
+    db = await createPgliteDb()
+
+    // Apply ONLY migration 001, then write an outbound thread the way a
+    // pre-002 deployment would have — no delivery_status column yet.
+    await migrate(db, { throughId: 1 })
+    const [conversation] = await db.query<{ id: string }>(
+      'INSERT INTO conversations (customer_email) VALUES ($1) RETURNING id',
+      ['customer@example.test'],
+    )
+    const [outbound] = await db.query<{ id: string }>(
+      `INSERT INTO threads (conversation_id, direction, from_address)
+       VALUES ($1, 'outbound', $2) RETURNING id`,
+      [conversation.id, 'support@example.test'],
+    )
+
+    // Now apply 002 over that existing data. Without the backfill this throws
+    // (the preexisting outbound row is NULL and violates the new CHECK).
+    await expect(migrate(db)).resolves.toBeUndefined()
+
+    // The preexisting outbound row was backfilled to 'pending', and the
+    // constraint is now live (a fresh NULL outbound insert is rejected).
+    const [row] = await db.query<{ delivery_status: string | null }>(
+      'SELECT delivery_status FROM threads WHERE id = $1',
+      [outbound.id],
+    )
+    expect(row.delivery_status).toBe('pending')
+    await expect(
+      db.query(
+        `INSERT INTO threads (conversation_id, direction, from_address, delivery_status)
+         VALUES ($1, 'outbound', $2, NULL)`,
+        [conversation.id, 'support@example.test'],
+      ),
+    ).rejects.toThrow()
   })
 })
@@ -61,13 +61,59 @@ CREATE TABLE threads (
 CREATE INDEX threads_conversation_id_idx ON threads (conversation_id);
 `
 
+/**
+ * Migration 002 — outbound delivery status (specs/mail/sending.md §3).
+ *
+ * An outbound thread is an outbox item: it carries `pending`/`sent`/`failed`
+ * to make "persisted" and "delivered" distinct facts (a crash mid-send must
+ * never be misreported as delivered). Inbound threads have no delivery
+ * concept, so the column stays `NULL` for them.
+ *
+ * The constraint is a CROSS-COLUMN (table-level) invariant tying status to
+ * direction, not a value-only check: an inbound row MUST be `NULL` and an
+ * outbound row MUST be one of the three states. This makes the illegal
+ * states — an inbound thread marked `'sent'`, or an outbound thread with a
+ * `NULL` status invisible to a future delivery worker — unrepresentable at
+ * the database level, not merely discouraged in application code (a
+ * table-level constraint is added with a separate `ADD CONSTRAINT` because an
+ * inline `ADD COLUMN ... CHECK` may only reference its own column).
+ */
+// NOTE on the explicit \`delivery_status IS NOT NULL\` in the outbound branch:
+// a CHECK constraint passes on TRUE *or* NULL (unknown) and only fails on
+// FALSE. Without the IS-NOT-NULL guard, an outbound row with a NULL status
+// makes \`delivery_status IN (...)\` evaluate to NULL, so the whole CHECK is
+// NULL and the row is (wrongly) ACCEPTED — the exact "outbound with no status,
+// invisible to the delivery worker" state this constraint exists to forbid.
+// The guard forces that case to FALSE so it is rejected.
+// The BACKFILL between ADD COLUMN and ADD CONSTRAINT is load-bearing, not
+// cosmetic: on a database that already ran migration 001 and stored outbound
+// threads, ADD COLUMN gives those rows a NULL delivery_status, which the new
+// direction-tied CHECK (with its IS NOT NULL guard) would then REJECT —
+// failing the whole migration on any non-fresh database. Backfilling existing
+// outbound rows to 'pending' (a truthful "delivery state unknown/unconfirmed"
+// for rows that predate delivery tracking) makes them satisfy the constraint
+// before it is added. Inbound rows correctly stay NULL.
+const MIGRATION_002_ADD_THREAD_DELIVERY_STATUS = `
+ALTER TABLE threads ADD COLUMN delivery_status text;
+UPDATE threads SET delivery_status = 'pending' WHERE direction = 'outbound' AND delivery_status IS NULL;
+ALTER TABLE threads ADD CONSTRAINT threads_delivery_status_by_direction CHECK (
+  (direction = 'inbound' AND delivery_status IS NULL)
+  OR (direction = 'outbound' AND delivery_status IS NOT NULL AND delivery_status IN ('pending','sent','failed'))
+);
+`
+
 /**
  * Every migration, in the order they must apply. `id` is the sole ordering
  * key (ascending) — array position is not relied upon, so re-sorting this
  * array by accident is harmless.
  */
 const MIGRATIONS: Migration[] = [
   { id: 1, name: 'conversations_and_threads', sql: MIGRATION_001_CONVERSATIONS_AND_THREADS },
+  {
+    id: 2,
+    name: 'add_thread_delivery_status',
+    sql: MIGRATION_002_ADD_THREAD_DELIVERY_STATUS,
+  },
 ]
 
 /**
@@ -138,8 +184,18 @@ const MIGRATION_ADVISORY_LOCK_KEY = 4_137_231_984
  * only reproducible against a real multi-connection server, so it is not
  * unit-testable here. The idempotency test covers the apply-once bookkeeping;
  * true concurrent-migrate coverage waits for the Supabase-backed `Db`.)
+ *
+ * ## `throughId`
+ *
+ * `options.throughId` applies only migrations with `id <= throughId`, leaving
+ * later ones pending. Its main use is staged rollouts and testing forward
+ * UPGRADE paths — applying an earlier schema, writing data against it, then
+ * applying the next migration over that data (exactly what a real deploy does,
+ * and what a fresh-only test never exercises). Omitted, every pending
+ * migration is applied.
  */
-export async function migrate(db: Db): Promise<void> {
+export async function migrate(db: Db, options?: { throughId?: number }): Promise<void> {
+  const throughId = options?.throughId
   await db.transaction(async (tx) => {
     // Serialize concurrent migrate() runs before touching any state. A bare
     // integer key needs no table, so this is safe to take before `_migrations`
@@ -157,9 +213,10 @@ export async function migrate(db: Db): Promise<void> {
     const applied = await tx.query<{ id: number }>('SELECT id FROM _migrations')
     const appliedIds = new Set(applied.map((row) => row.id))
 
-    const pending = MIGRATIONS.filter((migration) => !appliedIds.has(migration.id)).sort(
-      (a, b) => a.id - b.id,
-    )
+    const pending = MIGRATIONS.filter(
+      (migration) =>
+        !appliedIds.has(migration.id) && (throughId === undefined || migration.id <= throughId),
+    ).sort((a, b) => a.id - b.id)
 
     for (const migration of pending) {
       for (const statement of splitStatements(migration.sql)) {