From e76533515432d79969a87bcec92de76496727d62 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 07:16:06 +0000 Subject: [PATCH] [core] Fix "event cursor missing after initial load" performance degradation due to clobbered cursor (#2056) Signed-off-by: Peter Wielander --- .changeset/fix-load-events-cursor-null.md | 6 + packages/core/src/runtime/helpers.test.ts | 134 +++++++++++++++++++++- packages/core/src/runtime/helpers.ts | 10 +- 3 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 .changeset/fix-load-events-cursor-null.md diff --git a/.changeset/fix-load-events-cursor-null.md b/.changeset/fix-load-events-cursor-null.md new file mode 100644 index 0000000000..5590941b79 --- /dev/null +++ b/.changeset/fix-load-events-cursor-null.md @@ -0,0 +1,6 @@ +--- +"@workflow/core": patch +"workflow": patch +--- + +Fix spurious "Event cursor missing after initial load" warning diff --git a/packages/core/src/runtime/helpers.test.ts b/packages/core/src/runtime/helpers.test.ts index 176340f9dc..4150e1510e 100644 --- a/packages/core/src/runtime/helpers.test.ts +++ b/packages/core/src/runtime/helpers.test.ts @@ -1,6 +1,10 @@ -import type { World } from '@workflow/world'; -import { describe, expect, it, vi } from 'vitest'; -import { getWorkflowQueueName, healthCheck } from './helpers.js'; +import type { Event, World } from '@workflow/world'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + getWorkflowQueueName, + getWorkflowRunEvents, + healthCheck, +} from './helpers.js'; // Mock the logger to suppress output during tests vi.mock('../logger.js', () => ({ @@ -12,6 +16,25 @@ vi.mock('../logger.js', () => ({ }, })); +const eventsListMock = vi.fn(); + +vi.mock('./world.js', () => ({ + getWorld: vi.fn(() => ({ + events: { + list: eventsListMock, + }, + })), +})); + +const makeEvent = (eventId: string): Event => + ({ + eventId, + runId: 'wrun_mockidnumber0001', + eventType: 'step_created', + correlationId: 'step_mock', + createdAt: new Date(), + }) as unknown as Event; + describe('getWorkflowQueueName', () => { it('should return a valid queue name for a simple workflow name', () => { expect(getWorkflowQueueName('myWorkflow')).toBe( @@ -137,3 +160,108 @@ describe('healthCheck', () => { ); }); }); + +describe('getWorkflowRunEvents', () => { + beforeEach(() => { + eventsListMock.mockReset(); + }); + + it('returns the cursor from the last page when pagination terminates normally', async () => { + const page1 = [makeEvent('evnt_a'), makeEvent('evnt_b')]; + eventsListMock.mockResolvedValueOnce({ + data: page1, + cursor: 'eid:evnt_b', + hasMore: false, + }); + + const result = await getWorkflowRunEvents('wrun_test'); + + expect(result.events).toHaveLength(2); + expect(result.cursor).toBe('eid:evnt_b'); + expect(eventsListMock).toHaveBeenCalledTimes(1); + }); + + // Regression test for the "Event cursor missing after initial load" warning. + // + // A World may legitimately return `{ data: [], cursor: null, hasMore: false }` + // on a trailing empty page — workflow-server does this whenever the previous + // page's DynamoDB query hit `Limit` exactly and DynamoDB returned a + // `LastEvaluatedKey` "just in case." If the pagination loop overwrites the + // cursor with `null` on that trailing page, the runtime's incremental-load + // path can't proceed and falls back to a full reload on every replay + // iteration, logging "Event cursor missing after initial load" each time. + it('preserves the cursor from the previous page when the final page is empty', async () => { + const page1 = [makeEvent('evnt_a'), makeEvent('evnt_b')]; + eventsListMock.mockResolvedValueOnce({ + data: page1, + cursor: 'eid:evnt_b', + hasMore: true, + }); + eventsListMock.mockResolvedValueOnce({ + data: [], + cursor: null, + hasMore: false, + }); + + const result = await getWorkflowRunEvents('wrun_test'); + + expect(result.events).toHaveLength(2); + expect(result.cursor).toBe('eid:evnt_b'); + expect(eventsListMock).toHaveBeenCalledTimes(2); + }); + + it('returns null cursor only when no events exist at all', async () => { + eventsListMock.mockResolvedValueOnce({ + data: [], + cursor: null, + hasMore: false, + }); + + const result = await getWorkflowRunEvents('wrun_test'); + + expect(result.events).toHaveLength(0); + expect(result.cursor).toBeNull(); + }); + + it('uses the latest cursor when paginating through multiple non-empty pages', async () => { + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_a')], + cursor: 'eid:evnt_a', + hasMore: true, + }); + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_b')], + cursor: 'eid:evnt_b', + hasMore: true, + }); + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_c')], + cursor: 'eid:evnt_c', + hasMore: false, + }); + + const result = await getWorkflowRunEvents('wrun_test'); + + expect(result.events.map((e) => e.eventId)).toEqual([ + 'evnt_a', + 'evnt_b', + 'evnt_c', + ]); + expect(result.cursor).toBe('eid:evnt_c'); + }); + + it('falls back to the afterCursor when an incremental load returns no events', async () => { + eventsListMock.mockResolvedValueOnce({ + data: [], + cursor: null, + hasMore: false, + }); + + const result = await getWorkflowRunEvents('wrun_test', 'eid:evnt_z'); + + expect(result.events).toHaveLength(0); + // Preserving the input cursor avoids the runtime treating "no new events + // since last poll" as "I have no idea where I am in the log." + expect(result.cursor).toBe('eid:evnt_z'); + }); +}); diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index c8376bc0b0..d606ab3d07 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -405,7 +405,15 @@ export async function getWorkflowRunEvents( allEvents.push(...response.data); hasMore = response.hasMore; - nextCursor = response.cursor; + // Preserve the last non-null cursor across pages. A World may + // legitimately return `{ data: [], cursor: null, hasMore: false }` + // on a trailing empty page — for example when the previous page's + // underlying DynamoDB query hit `Limit` exactly and returned a + // `LastEvaluatedKey` "just in case". Overwriting with that null + // would lose the position past the last real event we loaded and + // force the runtime into the "no cursor after initial load" full- + // reload fallback on every subsequent replay iteration. + nextCursor = response.cursor ?? nextCursor; pagesLoaded++; runtimeLogger.debug('Loaded event page', {