fix(email): dry-run send no longer strands approved pipeline jobs

e2e/tests/pipeline-approve.spec.js

@@ -12,16 +12,16 @@
  *     → enqueues email.send on the in-process queue
  *   the email.send worker (server/email-jobs.js) finds no mail provider
  *   (RESEND_API_KEY / SMTP_* / Gmail OAuth all blank, no mailbox_accounts row)
- *   and takes its documented dev fallback at email-jobs.js:154-160:
+ *   and takes its dev fallback: it skips the provider call and nothing else.
  *     → contacts.status: 'pending' → 'sent', sent_at set, send_attempts = 1
- *     → returns { dryRun: true } *before* the provider-success block, so
- *       pipeline_jobs.stage stays 'send' (never reaches 'monitor'),
- *       email_sent_at / smtp_message_id stay NULL, and no email_replies or
- *       email_events rows are written.
+ *     → pipeline_jobs.stage: 'send' → 'monitor', email_sent_at set
+ *     → an outbound email_replies row and a 'sent' email_event (labelled
+ *       dryRun) are written; provider ids stay NULL because nothing was sent.
  *
- * That last point is a real quirk of the no-provider path, so it is asserted
- * explicitly rather than papered over — if the dry-run branch ever learns to
- * sync the pipeline row, this spec fails loudly and should be updated.
+ * This spec previously asserted the opposite and said it should be updated if
+ * the dry-run branch ever learned to sync the pipeline row. It has: the early
+ * return used to strand every approved job at 'send' on any provider-less
+ * deployment — which is what CI is, and what a fresh self-host is.
  */
 
 const { test, expect } = require('@playwright/test');
@@ -107,14 +107,14 @@ test('approving a review-stage job moves it out of review and marks the contact
   expect(job.email_approved).toBe(1);
   expect(job.email_to).toBe(fixture.emailTo);
   expect(job.error).toBeNull();
-  // Canary for the dry-run early-return described in the file header: with no
-  // mail provider the worker never reaches the pipeline_jobs sync, so the job
-  // stops at 'send' instead of advancing to 'monitor'.
-  expect(job.stage).toBe('send');
-  expect(job.email_sent_at).toBeNull();
+  // The no-provider path runs the same bookkeeping as a real send, minus the
+  // send: the job advances out of 'send', and the thread row and event exist
+  // so the UI isn't blank. Provider ids stay NULL because nothing was sent.
+  expect(job.stage).toBe('monitor');
+  expect(job.email_sent_at).toBeTruthy();
   expect(job.smtp_message_id).toBeNull();
-  expect(countRows('email_replies', 'contact_id = ?', [fixture.contactId])).toBe(0);
-  expect(countRows('email_events', 'contact_id = ?', [fixture.contactId])).toBe(0);
+  expect(countRows('email_replies', 'contact_id = ?', [fixture.contactId])).toBe(1);
+  expect(countRows('email_events', 'contact_id = ?', [fixture.contactId])).toBe(1);
 });
 
 test('approve is stage-gated: the action disappears and a second approve is rejected', async ({ page }) => {
@@ -137,5 +137,8 @@ test('approve is stage-gated: the action disappears and a second approve is reje
 
   const second = await approveOnce();
   expect(second.status()).toBe(400);
-  expect((await second.json()).error).toBe('Job is in stage "send", not "review"');
+  // Match the gate, not the stage the job happens to have moved on to — the
+  // worker races this assertion ('send' while queued, 'monitor' once the
+  // handler finishes), and which side wins is timing, not behavior.
+  expect((await second.json()).error).toMatch(/^Job is in stage "(send|monitor)", not "review"$/);
 });

server/__tests__/pipeline-send-integration.test.js

@@ -336,10 +336,13 @@ test('approve → send: contact reaches sent and the pipeline job advances to mo
   assert.equal(queue.getStats().failed, 0);
 });
 
-test('no mail provider configured: the dry-run branch marks the contact sent but leaves the job at send', async () => {
-  // Pins the behavior the Playwright suite observes in CI, where no
-  // RESEND_API_KEY exists: email-jobs.js:154-160 returns { dryRun: true }
-  // *before* the pipeline_jobs sync, so the job never reaches 'monitor'.
+test('no mail provider configured: dry-run completes the whole flow without calling out', async () => {
+  // This test used to pin the opposite: the dry-run branch returned early,
+  // *before* the pipeline_jobs sync, so a provider-less deployment (which is
+  // what CI is, and what a fresh self-host is) left every approved job
+  // stranded at stage='send' — the contact said "sent" while the Pipeline
+  // page showed it stuck mid-flight forever. The branch now skips only the
+  // provider call; all bookkeeping below it runs.
   const f = await driveToReview('dryrun');
   const mailAgent = makeMailAgent({ configured: false });
   const queue = makeQueue(mailAgent);
@@ -350,16 +353,29 @@ test('no mail provider configured: the dry-run branch marks the contact sent but
   const contact = await getContact(contactId);
   assert.equal(contact.status, 'sent');
   assert.ok(contact.sent_at);
-  assert.equal(contact.provider_message_id, null);
+  assert.equal(contact.provider_message_id, null, 'nothing was sent, so there is no provider id');
 
   const job = await getJob(f.pipelineJobId);
-  assert.equal(job.stage, 'send');
-  assert.equal(job.email_sent_at, null);
+  assert.equal(job.stage, 'monitor', 'the job must not be stranded at send');
+  assert.ok(job.email_sent_at, 'pipeline row records when the flow completed');
   assert.equal(job.smtp_message_id, null);
 
   assert.equal(mailAgent.sent.length, 0, 'no provider call may happen in dry-run mode');
-  const events = await query('SELECT event_type FROM email_events WHERE contact_id = ?', [contactId]);
-  assert.equal(events.rows.length, 0);
+
+  // The thread row and the event exist so the UI isn't blank, and the event
+  // is labelled so nobody mistakes a dry run for a real delivery.
+  const thread = await query(
+    "SELECT direction, to_email FROM email_replies WHERE contact_id = ?", [contactId]
+  );
+  assert.equal(thread.rows.length, 1);
+  assert.equal(thread.rows[0].direction, 'outbound');
+
+  const events = await query(
+    'SELECT event_type, payload FROM email_events WHERE contact_id = ?', [contactId]
+  );
+  assert.equal(events.rows.length, 1);
+  assert.equal(events.rows[0].event_type, 'sent');
+  assert.equal(JSON.parse(events.rows[0].payload || '{}').dryRun, true);
 });
 
 test('terminal provider failure: contact fails and the pipeline job bounces back to review', async () => {

server/email-jobs.js

@@ -150,38 +150,49 @@ function register({ jobQueue, query, queryOne, exec, mailAgent }) {
 
     const mailbox = await resolveMailbox(contact);
 
-    // Dev fallback: if no provider config, mark sent (keeps existing behavior).
-    if (!mailAgent.isConfigured() && !mailbox) {
-      await exec(
-        `UPDATE contacts SET status='sent', sent_at=CURRENT_TIMESTAMP, send_error=NULL, last_send_attempt_at=CURRENT_TIMESTAMP, send_attempts=COALESCE(send_attempts,0)+1 WHERE id=?`,
-        [contactId]
+    // Dev fallback: with no provider configured we don't call out to anyone,
+    // but everything downstream still runs. This used to `return` early, which
+    // skipped the pipeline_jobs sync below — so on every provider-less
+    // deployment an approved job reached contacts.status='sent' while
+    // pipeline_jobs.stage stayed at 'send' forever, and the Pipeline page
+    // showed it stuck mid-flight with no thread row and no event.
+    const dryRun = !mailAgent.isConfigured() && !mailbox;
+    if (dryRun) {
+      // Everything below records a fully successful send, so this line is the
+      // only operator-facing signal that nothing actually went out. Nothing
+      // fails fast at boot without a mail provider (email.js isConfigured() is
+      // just a boolean), so this path is reachable in production — where it
+      // means outreach is silently going nowhere. Warn, don't swallow.
+      log.warn(
+        `[email-jobs] dry-run: no mail provider configured (RESEND_API_KEY / SMTP_* / mailbox account all absent) — contact ${contactId} will be marked sent without sending`
       );
-      return { dryRun: true };
     }
 
     await exec(
       `UPDATE contacts SET send_attempts = COALESCE(send_attempts, 0) + 1, last_send_attempt_at = CURRENT_TIMESTAMP WHERE id = ?`,
       [contactId]
     );
 
-    const result = await mailAgent.sendEmail({
-      to: emailTo,
-      subject,
-      body,
-      mailboxAccount: mailbox,
-      // Persist refreshed Gmail tokens so the next send uses the new access_token
-      onCredsRefreshed: async (fresh) => {
-        if (!mailbox?.id) return;
-        try {
-          await exec(
-            'UPDATE mailbox_accounts SET credentials_encrypted = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?',
-            [secrets.encrypt(fresh), mailbox.id]
-          );
-        } catch (e) {
-          log.warn('[email-jobs] failed to persist refreshed creds:', e.message);
-        }
-      },
-    });
+    const result = dryRun
+      ? { success: true, messageId: null, provider: 'dry-run' }
+      : await mailAgent.sendEmail({
+        to: emailTo,
+        subject,
+        body,
+        mailboxAccount: mailbox,
+        // Persist refreshed Gmail tokens so the next send uses the new access_token
+        onCredsRefreshed: async (fresh) => {
+          if (!mailbox?.id) return;
+          try {
+            await exec(
+              'UPDATE mailbox_accounts SET credentials_encrypted = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?',
+              [secrets.encrypt(fresh), mailbox.id]
+            );
+          } catch (e) {
+            log.warn('[email-jobs] failed to persist refreshed creds:', e.message);
+          }
+        },
+      });
 
     if (!result.success) {
       // Transient vs terminal distinction: network-ish errors -> throw to retry.
@@ -255,10 +266,10 @@ function register({ jobQueue, query, queryOne, exec, mailAgent }) {
       contactId,
       providerMessageId: result.messageId,
       eventType: 'sent',
-      payload: { provider: result.provider, to: emailTo },
+      payload: { provider: result.provider, to: emailTo, dryRun: dryRun || undefined },
     });
 
-    return { success: true, messageId: result.messageId };
+    return { success: true, messageId: result.messageId, dryRun: dryRun || undefined };
   });
 
   // ---- email.batch_send ----