From 9659625c7d39d409c837bd1ba7e8d9dd2b1734a3 Mon Sep 17 00:00:00 2001 From: Danilo Leone Date: Wed, 24 Jun 2026 08:42:20 -0300 Subject: [PATCH] fix(journey): fall back to session journeyId so journey-default variables interpolate at every node (EVO-1885) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most node dispatch sites in journey-execution.workflow.ts omit journeyId, so BaseNode.interpolateNodeData found no journey and left journey-default {{variables}} as literal tokens at runtime (silently — the try/catch swallows it). EVO-1882 fixed only send-webhook; the omission is systemic across the 17 interpolating nodes. Single-point fallback: resolve the journey id as input.journeyId ?? session.journeyId. session.journeyId is a scalar column present on both the cache (CachedJourneySession) and DB session shapes, so one change covers all current and future nodes (including conditional via selectiveInterpolateNodeData). Guard the query on a resolved id — findOne({ where: { id: undefined } }) drops the condition and would return an arbitrary journey's defaults. New base.node.spec.ts covers fallback, input precedence, the no-id guard, and session-variable no-regression. --- .../activities/nodes/base.node.spec.ts | 109 ++++++++++++++++++ .../temporal/activities/nodes/base.node.ts | 13 ++- 2 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 src/modules/temporal/activities/nodes/base.node.spec.ts diff --git a/src/modules/temporal/activities/nodes/base.node.spec.ts b/src/modules/temporal/activities/nodes/base.node.spec.ts new file mode 100644 index 0000000..4f249eb --- /dev/null +++ b/src/modules/temporal/activities/nodes/base.node.spec.ts @@ -0,0 +1,109 @@ +import { BaseNode, NodeExecutionResult } from './base.node'; + +// interpolateNodeData dynamically imports the activities module to read the +// session from cache; stub it so the test drives the session shape directly. +const mockGetSessionFromCache = jest.fn(); +jest.mock('../journey-execution.activities', () => ({ + journeyExecutionActivities: { + getSessionFromCache: (...args: any[]) => mockGetSessionFromCache(...args), + }, +})); + +class TestNode extends BaseNode { + constructor() { + super('test-node'); + } + + async execute(): Promise { + return { success: true }; + } + + // Expose the protected method under test. + public interpolate(input: any, nodeData: any): Promise { + return this.interpolateNodeData(input, nodeData); + } +} + +describe('BaseNode.interpolateNodeData — journeyId fallback (EVO-1885)', () => { + let node: TestNode; + let findOne: jest.Mock; + + beforeEach(() => { + node = new TestNode(); + findOne = jest.fn(); + // initializeDatabase is the only DB seam; stub it so getRepository(Journey) + // yields our findOne spy and no real connection is opened. + jest + .spyOn(node as any, 'initializeDatabase') + .mockResolvedValue({ getRepository: () => ({ findOne }) }); + mockGetSessionFromCache.mockReset(); + findOne.mockReset(); + }); + + it('AC1: falls back to session.journeyId when input omits journeyId, resolving journey-default variables', async () => { + mockGetSessionFromCache.mockResolvedValue({ + id: 's1', + journeyId: 'journey-from-session', + variables: {}, + }); + findOne.mockResolvedValue({ + variables: [{ name: 'greeting', defaultValue: 'Hi' }], + }); + + const result = await node.interpolate( + { sessionId: 's1' }, // no journeyId on input + { message: '{{greeting}}' }, + ); + + expect(findOne).toHaveBeenCalledWith({ where: { id: 'journey-from-session' } }); + expect(result.message).toBe('Hi'); + }); + + it('AC2: prefers input.journeyId over session.journeyId when both are present', async () => { + mockGetSessionFromCache.mockResolvedValue({ + id: 's1', + journeyId: 'journey-from-session', + variables: {}, + }); + findOne.mockResolvedValue({ variables: [] }); + + await node.interpolate( + { sessionId: 's1', journeyId: 'journey-from-input' }, + { message: '{{greeting}}' }, + ); + + expect(findOne).toHaveBeenCalledWith({ where: { id: 'journey-from-input' } }); + }); + + it('AC3: skips the journey query when no id resolves, leaving journey-default tokens literal', async () => { + mockGetSessionFromCache.mockResolvedValue({ + id: 's1', + // no journeyId + variables: {}, + }); + + const result = await node.interpolate( + { sessionId: 's1' }, // no journeyId either + { message: '{{greeting}}' }, + ); + + expect(findOne).not.toHaveBeenCalled(); + expect(result.message).toBe('{{greeting}}'); + }); + + it('still resolves session variables regardless of journeyId (no regression)', async () => { + mockGetSessionFromCache.mockResolvedValue({ + id: 's1', + // no journeyId + variables: { name: 'Ada' }, + }); + + const result = await node.interpolate( + { sessionId: 's1' }, + { message: 'Hello {{name}}' }, + ); + + expect(findOne).not.toHaveBeenCalled(); + expect(result.message).toBe('Hello Ada'); + }); +}); diff --git a/src/modules/temporal/activities/nodes/base.node.ts b/src/modules/temporal/activities/nodes/base.node.ts index eb8abc7..3ec5038 100644 --- a/src/modules/temporal/activities/nodes/base.node.ts +++ b/src/modules/temporal/activities/nodes/base.node.ts @@ -230,9 +230,16 @@ export abstract class BaseNode { '../../../journeys/entities/journey.entity' ); const journeyRepository = dataSource.getRepository(Journey); - const journey = await journeyRepository.findOne({ - where: { id: input.journeyId }, - }); + // Most dispatch sites omit input.journeyId (EVO-1885); fall back to the + // session's own journeyId (a scalar column present on both the cache and + // DB session shapes) so journey-default {{variables}} resolve at every + // node, not just the few that thread journeyId explicitly. Guard the query + // on a resolved id: findOne({ where: { id: undefined } }) drops the + // condition and would return an arbitrary journey's defaults. + const journeyId = input.journeyId ?? session.journeyId; + const journey = journeyId + ? await journeyRepository.findOne({ where: { id: journeyId } }) + : null; const context: VariableContext = { sessionVariables: session.variables || {},