Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/spotty-pagination-progress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@workflow/core': patch
---

Fail event pagination cleanly when a page reports more results but adds no new events, bounding the load loop against non-progressing cursor responses.
Comment thread
VaguelySerious marked this conversation as resolved.
Outdated
56 changes: 56 additions & 0 deletions packages/core/src/runtime/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
20 changes: 18 additions & 2 deletions packages/core/src/runtime/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ function assertEventPaginationProgress(
runId: string,
hasMore: boolean,
cursor: string | null,
requestedCursors: Set<string>
requestedCursors: Set<string>,
addedNewEvents: boolean
): void {
if (!hasMore) {
return;
Expand All @@ -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(
Expand Down Expand Up @@ -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 }`
Expand Down
Loading