From d46328084238378461aa6266bc335c937ecbd420 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:51:50 +0000 Subject: [PATCH 1/2] fix(cdp): report a skipped test email send as skipped A test send that the email worker declined, for example to a suppressed recipient or from a paused workflow, came back from the test invocation endpoint as a success. The endpoint now reports `skipped` when the email step was skipped, and keeps the next step for step-through. A recipient without an email address now records its reason in `emailSendResult` like the other skips. Refs Silthus/personal#204 Co-Authored-By: Claude Opus 5.5 --- nodejs/src/cdp/cdp-api.serial.test.ts | 8 ++++---- nodejs/src/cdp/cdp-api.ts | 3 ++- nodejs/src/cdp/services/messaging/email.service.test.ts | 4 ++++ nodejs/src/cdp/services/messaging/email.service.ts | 6 +++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/nodejs/src/cdp/cdp-api.serial.test.ts b/nodejs/src/cdp/cdp-api.serial.test.ts index 64baeb284314..b9f24aa082ea 100644 --- a/nodejs/src/cdp/cdp-api.serial.test.ts +++ b/nodejs/src/cdp/cdp-api.serial.test.ts @@ -2788,9 +2788,9 @@ describe('CDP API', () => { }) it.each([ - { status: 'accepted' }, - { status: 'skipped', reason: 'The recipient has no reachable mail servers.' }, - ])('returns the inline email outcome %j without routing to the email queue', async (emailSendResult) => { + [{ status: 'accepted' }, 'success'], + [{ status: 'skipped', reason: 'The recipient has no reachable mail servers.' }, 'skipped'], + ])('returns inline email outcome %j as %s, not via the email queue', async (emailSendResult, testStatus) => { emailSpy.mockImplementation((invocation: any) => Promise.resolve({ invocation, @@ -2811,7 +2811,7 @@ describe('CDP API', () => { }) expect(res.status).toBe(200) - expect(res.body.status).toBe('success') + expect(res.body.status).toBe(testStatus) expect(res.body.emailSendResult).toEqual(emailSendResult) expect(res.body.errors).toEqual([]) // EmailService was called inline — proving the test endpoint forced inline delivery diff --git a/nodejs/src/cdp/cdp-api.ts b/nodejs/src/cdp/cdp-api.ts index 649511b14f71..543a37137dfa 100644 --- a/nodejs/src/cdp/cdp-api.ts +++ b/nodejs/src/cdp/cdp-api.ts @@ -934,10 +934,11 @@ export class CdpApi { ) options.isTest = true const result = await this.hogFlowExecutor.executeCurrentAction(invocation, { hogExecutorOptions: options }) + const emailWasSkipped = result.emailSendResult?.status === 'skipped' res.json({ nextActionId: result.skipped ? null : result.invocation.state.currentAction?.id, - status: result.error ? 'error' : result.skipped ? 'skipped' : 'success', + status: result.error ? 'error' : result.skipped || emailWasSkipped ? 'skipped' : 'success', errors: result.error ? [result.error] : [], logs: [...result.logs, ...logs], variables: result.invocation.state.variables ?? {}, diff --git a/nodejs/src/cdp/services/messaging/email.service.test.ts b/nodejs/src/cdp/services/messaging/email.service.test.ts index 7c3cd05338a0..456ffcc500cb 100644 --- a/nodejs/src/cdp/services/messaging/email.service.test.ts +++ b/nodejs/src/cdp/services/messaging/email.service.test.ts @@ -1140,6 +1140,10 @@ describe('EmailService', () => { expect(result.error).toBeUndefined() expect(result.metrics.map((m) => m.metric_name)).toEqual(['email_missing_recipient']) expect(result.skipped).toBe(true) + expect(result.emailSendResult).toEqual({ + status: 'skipped', + reason: expect.stringContaining('the recipient has no email address'), + }) expect(invocation.state.vmState?.stack).toEqual([{ success: false }]) }) diff --git a/nodejs/src/cdp/services/messaging/email.service.ts b/nodejs/src/cdp/services/messaging/email.service.ts index ff83d8746de4..0f6f513a0572 100644 --- a/nodejs/src/cdp/services/messaging/email.service.ts +++ b/nodejs/src/cdp/services/messaging/email.service.ts @@ -492,10 +492,10 @@ export class EmailService { // so record a skip with its own metric instead of an `email_failed`. Otherwise one project // with bad data inflates the platform send failure rate. if (!params.to?.email?.trim()) { - addLog( - 'warn', + const skipReason = 'Skipping send: the recipient has no email address. Add a filter so only people with an email address reach this step.' - ) + addLog('warn', skipReason) + result.emailSendResult = { status: 'skipped', reason: skipReason } if (!isTest) { result.metrics.push({ team_id: invocation.teamId, From 11190d50c9c647a6c6ef2bb6eaa2999f172a7e1b Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:22:05 +0000 Subject: [PATCH 2/2] fix(cdp): keep the test endpoint's status for a skipped email The frontend reads the skip from emailSendResult (PostHog/posthog#123846), so the endpoint keeps reporting success for a send the worker skipped. Co-Authored-By: Claude Opus 5.5 --- nodejs/src/cdp/cdp-api.serial.test.ts | 8 ++++---- nodejs/src/cdp/cdp-api.ts | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/nodejs/src/cdp/cdp-api.serial.test.ts b/nodejs/src/cdp/cdp-api.serial.test.ts index b9f24aa082ea..64baeb284314 100644 --- a/nodejs/src/cdp/cdp-api.serial.test.ts +++ b/nodejs/src/cdp/cdp-api.serial.test.ts @@ -2788,9 +2788,9 @@ describe('CDP API', () => { }) it.each([ - [{ status: 'accepted' }, 'success'], - [{ status: 'skipped', reason: 'The recipient has no reachable mail servers.' }, 'skipped'], - ])('returns inline email outcome %j as %s, not via the email queue', async (emailSendResult, testStatus) => { + { status: 'accepted' }, + { status: 'skipped', reason: 'The recipient has no reachable mail servers.' }, + ])('returns the inline email outcome %j without routing to the email queue', async (emailSendResult) => { emailSpy.mockImplementation((invocation: any) => Promise.resolve({ invocation, @@ -2811,7 +2811,7 @@ describe('CDP API', () => { }) expect(res.status).toBe(200) - expect(res.body.status).toBe(testStatus) + expect(res.body.status).toBe('success') expect(res.body.emailSendResult).toEqual(emailSendResult) expect(res.body.errors).toEqual([]) // EmailService was called inline — proving the test endpoint forced inline delivery diff --git a/nodejs/src/cdp/cdp-api.ts b/nodejs/src/cdp/cdp-api.ts index 543a37137dfa..649511b14f71 100644 --- a/nodejs/src/cdp/cdp-api.ts +++ b/nodejs/src/cdp/cdp-api.ts @@ -934,11 +934,10 @@ export class CdpApi { ) options.isTest = true const result = await this.hogFlowExecutor.executeCurrentAction(invocation, { hogExecutorOptions: options }) - const emailWasSkipped = result.emailSendResult?.status === 'skipped' res.json({ nextActionId: result.skipped ? null : result.invocation.state.currentAction?.id, - status: result.error ? 'error' : result.skipped || emailWasSkipped ? 'skipped' : 'success', + status: result.error ? 'error' : result.skipped ? 'skipped' : 'success', errors: result.error ? [result.error] : [], logs: [...result.logs, ...logs], variables: result.invocation.state.variables ?? {},