diff --git a/.changeset/spotty-pagination-progress.md b/.changeset/spotty-pagination-progress.md new file mode 100644 index 0000000000..eed301c263 --- /dev/null +++ b/.changeset/spotty-pagination-progress.md @@ -0,0 +1,5 @@ +--- +'@workflow/core': patch +--- + +Further harden event pagination code, failing cleanly when a page reports more results but adds no new events and would get stuck in a loop diff --git a/packages/core/src/runtime/helpers.test.ts b/packages/core/src/runtime/helpers.test.ts index 0b1e6b8f58..7600455e39 100644 --- a/packages/core/src/runtime/helpers.test.ts +++ b/packages/core/src/runtime/helpers.test.ts @@ -283,4 +283,60 @@ describe('loadWorkflowRunEvents', () => { }); expect(eventsListMock).toHaveBeenCalledTimes(1); }); + + it('fails when a page advances the cursor but adds no new events', async () => { + // A backend that keeps returning fresh cursors with empty pages would + // never repeat a cursor, so the cursor-based guards never trip. Zero + // forward progress must still bound the loop on the first such page. + eventsListMock.mockResolvedValue({ + data: [], + cursor: 'eid:evnt_a', + hasMore: true, + }); + + await expect(loadWorkflowRunEvents('wrun_test')).rejects.toMatchObject({ + code: 'WORLD_CONTRACT_ERROR', + }); + expect(eventsListMock).toHaveBeenCalledTimes(1); + }); + + it('fails when a non-final page is entirely overlapping events', async () => { + // Real events on page 1, then a page that re-serves only already-seen + // events while still claiming `hasMore` with a brand-new cursor. + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_a'), makeEvent('evnt_b')], + cursor: 'eid:evnt_b', + hasMore: true, + }); + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_a'), makeEvent('evnt_b')], + cursor: 'eid:evnt_b_again', + hasMore: true, + }); + + await expect(loadWorkflowRunEvents('wrun_test')).rejects.toMatchObject({ + code: 'WORLD_CONTRACT_ERROR', + }); + expect(eventsListMock).toHaveBeenCalledTimes(2); + }); + + it('still completes when a trailing empty page closes pagination', async () => { + // Regression guard: an empty final page is legitimate as long as it does + // not claim `hasMore`, and must not be mistaken for non-progression. + eventsListMock.mockResolvedValueOnce({ + data: [makeEvent('evnt_a')], + cursor: 'eid:evnt_a', + hasMore: true, + }); + eventsListMock.mockResolvedValueOnce({ + data: [], + cursor: null, + hasMore: false, + }); + + const result = await loadWorkflowRunEvents('wrun_test'); + + expect(result.events.map((event) => event.eventId)).toEqual(['evnt_a']); + expect(eventsListMock).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index 91bdd15056..1c3380ee1d 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -348,7 +348,8 @@ function assertEventPaginationProgress( runId: string, hasMore: boolean, cursor: string | null, - requestedCursors: Set + requestedCursors: Set, + addedNewEvents: boolean ): void { if (!hasMore) { return; @@ -362,6 +363,19 @@ function assertEventPaginationProgress( if (requestedCursors.has(cursor)) { throw eventPaginationContractError(runId, 'repeated a cursor'); } + // A non-final page over an ascending cursor must surface at least one event + // we have not already seen. The existing guards above only catch a cursor + // that repeats a value we requested before; a backend that keeps handing + // back fresh cursors with empty or fully-overlapping pages would otherwise + // spin `while (hasMore)` forever while `appendUniqueEvents` keeps the result + // flat. Treating zero forward progress as a contract violation bounds the + // loop without an arbitrary page cap. + if (!addedNewEvents) { + throw eventPaginationContractError( + runId, + 'reported more pages without making progress' + ); + } } function shouldRetryWithoutEventCursor( @@ -449,13 +463,15 @@ export async function loadWorkflowRunEvents( throw error; } + const eventsBeforeAppend = loadedEvents.length; appendUniqueEvents(loadedEvents, loadedEventIds, response.data); hasMore = response.hasMore; assertEventPaginationProgress( runId, hasMore, response.cursor, - requestedCursors + requestedCursors, + loadedEvents.length > eventsBeforeAppend ); // Preserve the last non-null cursor across pages. A World may // legitimately return `{ data: [], cursor: null, hasMore: false }`