From 37c1dbe093260274fde6b8bc60ec3380b258728a Mon Sep 17 00:00:00 2001 From: Danilo Leone Date: Thu, 25 Jun 2026 07:32:21 -0300 Subject: [PATCH 1/2] fix(journeys): lazy-upsert minimal contact row so session persistence never violates contact FK (EVO-1929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit journey_sessions.contact_id carries FK_journey_sessions_contact_id to evo_campaign.contacts (ON DELETE CASCADE), but the evo-flow community surface never populates that table (ContactsService/ContactsClientService are HTTP proxies to the CRM). EVO-1892 made only the journey START path swallow the FK violation (best-effort persist); the per-node runtime writes from the Temporal activity still go through the durable (throwing) persist path, so the first per-node session save aborts the journey at node 1 for any contact not pre-seeded into evo_campaign.contacts. Fix (option b — lazy upsert): persistToDatabase ensures a minimal contacts row (id only) via INSERT ... ON CONFLICT (id) DO NOTHING before saving the session. The contacts table requires only id (every other column has a DB default or is nullable), so an id-only row is valid; the ON CONFLICT keeps it idempotent and never clobbers a real CRM-synced contact. This resolves the root cause on ALL write paths (start and runtime) while preserving referential integrity, instead of relaxing the FK. --- .../journey-session-cache.service.spec.ts | 73 +++++++++++++++++-- .../services/journey-session-cache.service.ts | 43 +++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/src/modules/cache/services/journey-session-cache.service.spec.ts b/src/modules/cache/services/journey-session-cache.service.spec.ts index 845cb2e..0ae517d 100644 --- a/src/modules/cache/services/journey-session-cache.service.spec.ts +++ b/src/modules/cache/services/journey-session-cache.service.spec.ts @@ -93,16 +93,18 @@ jest.mock('./redis-singleton.service', () => ({ import { JourneySessionCacheService } from './journey-session-cache.service'; -function makeRepository(): jest.Mocked< +type MockRepository = jest.Mocked< Pick, 'findOne' | 'find' | 'save'> -> { +> & { manager: { query: jest.Mock } }; + +function makeRepository(): MockRepository { return { findOne: jest.fn().mockResolvedValue(null), find: jest.fn().mockResolvedValue([]), save: jest.fn().mockImplementation((e) => Promise.resolve(e)), - } as unknown as jest.Mocked< - Pick, 'findOne' | 'find' | 'save'> - >; + // EVO-1929: the lazy contact upsert runs through the entity manager. + manager: { query: jest.fn().mockResolvedValue([]) }, + } as unknown as MockRepository; } function makeSession(id: string): JourneySession { @@ -293,3 +295,64 @@ describe('JourneySessionCacheService — trigger idempotency (EVO-1896)', () => ).resolves.toBe(true); }); }); + +describe('JourneySessionCacheService — lazy contact upsert satisfies FK (EVO-1929)', () => { + beforeEach(() => { + mockKv.clear(); + mockSets.clear(); + jest.clearAllMocks(); + }); + + it('upserts a minimal contacts row before saving a session for a CRM-only contact', async () => { + const repo = makeRepository(); + const service = makeService(repo); + + const session = makeSession('sess-fk'); + (session as unknown as { contactId: string }).contactId = 'crm-only-contact'; + + await service.set(session); + + // The contact row is ensured BEFORE the session is persisted, so the + // FK_journey_sessions_contact_id constraint is always satisfied. + expect(repo.manager.query).toHaveBeenCalledWith( + 'INSERT INTO contacts (id) VALUES ($1) ON CONFLICT (id) DO NOTHING', + ['crm-only-contact'], + ); + const upsertOrder = repo.manager.query.mock.invocationCallOrder[0]; + const saveOrder = repo.save.mock.invocationCallOrder[0]; + expect(upsertOrder).toBeLessThan(saveOrder); + expect(repo.save).toHaveBeenCalledTimes(1); + }); + + it('runs the idempotent upsert on per-node runtime writes (not just start)', async () => { + const repo = makeRepository(); + const service = makeService(repo); + + // First write creates the session (and ensures the contact)... + await service.set(makeSession('sess-runtime')); + repo.manager.query.mockClear(); + + // ...a subsequent per-node status transition (the runtime path that used + // to FK-fail) also runs the idempotent upsert. + await service.updateSessionStatus('sess-runtime', 'completed', { + completedAt: new Date('2026-06-03T00:00:00Z'), + }); + + expect(repo.manager.query).toHaveBeenCalledWith( + 'INSERT INTO contacts (id) VALUES ($1) ON CONFLICT (id) DO NOTHING', + ['contact-1'], + ); + }); + + it('skips the upsert when the session has no contact id', async () => { + const repo = makeRepository(); + const service = makeService(repo); + + const session = makeSession('sess-no-contact'); + (session as unknown as { contactId?: string }).contactId = undefined; + + await service.set(session); + + expect(repo.manager.query).not.toHaveBeenCalled(); + }); +}); diff --git a/src/modules/cache/services/journey-session-cache.service.ts b/src/modules/cache/services/journey-session-cache.service.ts index d0e4006..ca2c65e 100644 --- a/src/modules/cache/services/journey-session-cache.service.ts +++ b/src/modules/cache/services/journey-session-cache.service.ts @@ -89,6 +89,27 @@ export class JourneySessionCacheService extends BaseCacheService< private async persistToDatabase(value: JourneySession): Promise { const v = value as unknown as CachedJourneySession; try { + // EVO-1929: `journey_sessions.contact_id` carries a FK to + // `evo_campaign.contacts` (FK_journey_sessions_contact_id, ON DELETE + // CASCADE), but the evo-flow community surface never populates that table + // — `ContactsService`/`ContactsClientService` are HTTP proxies to the CRM + // (Rails), so a contact that exists only in the CRM has no row here. + // EVO-1892 made the journey *start* path swallow the resulting FK + // violation (best-effort), but the per-node runtime writes from the + // Temporal activity still go through this durable (throwing) path, so the + // first per-node session persistence aborts the journey at node 1 for any + // contact not pre-seeded into evo_campaign.contacts. + // + // Fix (option b — lazy upsert): guarantee a minimal `contacts` row exists + // before saving the session. The contacts table requires only `id`; every + // other column has a DB default ('', false, '{}'::jsonb) or is nullable + // (see init-base-tables migration), so an id-only insert yields a valid + // row. `ON CONFLICT (id) DO NOTHING` keeps it idempotent and never + // clobbers a real CRM-synced contact. This resolves the root cause on ALL + // write paths while preserving referential integrity (vs. relaxing the + // FK). + await this.ensureContactRow(v.contactId); + await this.repository.save({ id: v.id, journeyId: v.journeyId, @@ -116,6 +137,28 @@ export class JourneySessionCacheService extends BaseCacheService< } } + /** + * EVO-1929: idempotently ensure a minimal `contacts` row exists so the + * `journey_sessions.contact_id` FK is satisfied before persisting a session. + * + * The contacts table is owned by the CRM (Rails) and only the `id` column is + * mandatory — all other columns have DB-level defaults or are nullable — so + * an id-only insert is a valid, minimal row. `ON CONFLICT (id) DO NOTHING` + * makes this a no-op when the contact already exists (CRM-synced or seeded), + * so it never overwrites real contact data and is safe to call on every + * session write. A missing/non-uuid id is skipped (the save itself will then + * fail loudly, preserving the existing error contract). + */ + private async ensureContactRow(contactId?: string): Promise { + if (!contactId) { + return; + } + await this.repository.manager.query( + 'INSERT INTO contacts (id) VALUES ($1) ON CONFLICT (id) DO NOTHING', + [contactId], + ); + } + async getActiveSessionsByJourney( journeyId: string, ): Promise { From ed459f3761a890fb6a8403172c82503b3ef91db9 Mon Sep 17 00:00:00 2001 From: Danilo Leone Date: Fri, 26 Jun 2026 06:05:57 -0300 Subject: [PATCH 2/2] docs(journeys): corrige JSDoc do ensureContactRow + caveat de deploy e trava contrato non-uuid (EVO-1929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups do review (PR #103): - A JSDoc dizia que a contacts table era "owned by the CRM (Rails)"; na verdade o alvo da FK e a tabela propria do evo-flow em evo_campaign (init-base-tables) — FK no Postgres nao cruza database. Corrigido + adicionado caveat de deploy (se evo-flow apontar pro Postgres do CRM/evo_community, a contacts do Rails tem created_at/updated_at NOT NULL sem default e o id-only insert quebra). - A JSDoc dizia "missing/non-uuid id is skipped"; so um id falsy e pulado. Um id non-uuid presente NAO e pulado: vai pro INSERT e falha alto no DB. Texto corrigido + teste travando o contrato. --- .../journey-session-cache.service.spec.ts | 19 +++++++++++++++ .../services/journey-session-cache.service.ts | 24 +++++++++++++------ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/modules/cache/services/journey-session-cache.service.spec.ts b/src/modules/cache/services/journey-session-cache.service.spec.ts index 0ae517d..8a51bf0 100644 --- a/src/modules/cache/services/journey-session-cache.service.spec.ts +++ b/src/modules/cache/services/journey-session-cache.service.spec.ts @@ -355,4 +355,23 @@ describe('JourneySessionCacheService — lazy contact upsert satisfies FK (EVO-1 expect(repo.manager.query).not.toHaveBeenCalled(); }); + + it('does NOT skip a present-but-non-uuid contact id (it reaches the INSERT and fails loud at the DB)', async () => { + // Contract guard for the JSDoc: only a falsy id is short-circuited. A + // present-but-non-uuid id is passed straight to the INSERT — here the mock + // accepts it, but against a real DB the uuid cast fails loudly, preserving + // the existing error contract (it is NOT silently skipped). + const repo = makeRepository(); + const service = makeService(repo); + + const session = makeSession('sess-bad-id'); + (session as unknown as { contactId: string }).contactId = 'not-a-uuid'; + + await service.set(session); + + expect(repo.manager.query).toHaveBeenCalledWith( + 'INSERT INTO contacts (id) VALUES ($1) ON CONFLICT (id) DO NOTHING', + ['not-a-uuid'], + ); + }); }); diff --git a/src/modules/cache/services/journey-session-cache.service.ts b/src/modules/cache/services/journey-session-cache.service.ts index ca2c65e..ee0f94f 100644 --- a/src/modules/cache/services/journey-session-cache.service.ts +++ b/src/modules/cache/services/journey-session-cache.service.ts @@ -141,13 +141,23 @@ export class JourneySessionCacheService extends BaseCacheService< * EVO-1929: idempotently ensure a minimal `contacts` row exists so the * `journey_sessions.contact_id` FK is satisfied before persisting a session. * - * The contacts table is owned by the CRM (Rails) and only the `id` column is - * mandatory — all other columns have DB-level defaults or are nullable — so - * an id-only insert is a valid, minimal row. `ON CONFLICT (id) DO NOTHING` - * makes this a no-op when the contact already exists (CRM-synced or seeded), - * so it never overwrites real contact data and is safe to call on every - * session write. A missing/non-uuid id is skipped (the save itself will then - * fail loudly, preserving the existing error contract). + * The `contacts` table targeted here is evo-flow's OWN table in + * `evo_campaign` (created by the init-base-tables migration), NOT the CRM's + * Rails `contacts` — a Postgres FK never crosses databases. In that table + * only `id` is mandatory; every other column has a DB-level default + * ('', false, '{}'::jsonb) or is nullable, so an id-only insert is a valid, + * minimal row. `ON CONFLICT (id) DO NOTHING` makes this a no-op when the + * contact already exists (CRM-synced or seeded), so it never overwrites real + * contact data and is safe to call on every session write. + * + * Deploy caveat: this relies on evo-flow pointing at its own `evo_campaign` + * schema. If evo-flow is ever repointed at the CRM's Postgres (`evo_community`), + * the Rails `contacts` has `created_at`/`updated_at` NOT NULL WITHOUT a + * default, so the id-only insert would break — revisit this then. + * + * Only a missing/falsy id is skipped here; a present-but-non-uuid id is NOT + * skipped — it is passed straight to the INSERT and fails loudly there, + * preserving the existing error contract. */ private async ensureContactRow(contactId?: string): Promise { if (!contactId) {